fix(ui): one transport for every control, and a queue panel that fits
The queue panel drove the playback store directly, so its buttons played locally while another device held the audio. Both control sets now go through one transport that forwards a press when the audio is elsewhere. The panel's artwork is capped against viewport height too: at full width on a phone the square alone pushed Up Next off the screen. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -7,6 +7,7 @@ import { Artwork } from './Artwork';
|
||||
import { ArtistLinks } from './ArtistLinks';
|
||||
import { TrackRow, formatDuration } from './TrackRow';
|
||||
import { albumService } from '../services/albumService';
|
||||
import { useTransport } from '../hooks/useTransport';
|
||||
|
||||
export function NowPlayingPanel({ onClose }: { onClose: () => void }) {
|
||||
const panelRef = useRef<HTMLElement>(null);
|
||||
@@ -34,7 +35,8 @@ export function NowPlayingPanel({ onClose }: { onClose: () => void }) {
|
||||
previous?.focus();
|
||||
};
|
||||
}, []);
|
||||
const { currentTrack, queue, isPlaying, position, duration, play, pause, next, prev, setPosition } = usePlaybackStore();
|
||||
const { currentTrack, queue, isPlaying, position, duration } = usePlaybackStore();
|
||||
const transport = useTransport();
|
||||
|
||||
const currentIdx = currentTrack ? queue.findIndex((t) => t.id === currentTrack.id) : -1;
|
||||
const upNext = currentIdx >= 0 ? queue.slice(currentIdx + 1) : queue;
|
||||
@@ -56,17 +58,21 @@ export function NowPlayingPanel({ onClose }: { onClose: () => void }) {
|
||||
</button>
|
||||
</div>
|
||||
|
||||
{/* The artwork below is capped against viewport height, not just width.
|
||||
At full width on a phone the square alone is taller than the space
|
||||
between the top bar and the transport, which pushed Up Next — the
|
||||
reason the panel opens — entirely off the screen. */}
|
||||
<div className="p-3 space-y-3 sm:p-4 sm:space-y-4">
|
||||
{currentTrack?.album_id ? (
|
||||
<Link to="/albums/$albumId" params={{ albumId: currentTrack.album_id }}
|
||||
className="group mx-auto block aspect-square w-full max-w-sm rounded-xl overflow-hidden relative shadow-lg shadow-black/40" title="Go to album">
|
||||
className="group mx-auto block aspect-square w-full max-w-[min(100%,34vh)] rounded-xl overflow-hidden relative shadow-lg shadow-black/40" title="Go to album">
|
||||
<Artwork seed={`${currentTrack.title} ${currentTrack.artist}`} src={artwork} className="w-full h-full transition-transform group-hover:scale-105" rounded="xl" />
|
||||
<div className="absolute inset-0 flex items-center justify-center bg-black/0 group-hover:bg-black/40 transition-colors">
|
||||
<Disc3 size={28} className="text-on-accent opacity-0 group-hover:opacity-100 transition-opacity" />
|
||||
</div>
|
||||
</Link>
|
||||
) : (
|
||||
<div className="mx-auto aspect-square w-full max-w-sm rounded-xl overflow-hidden shadow-lg shadow-black/40">
|
||||
<div className="mx-auto aspect-square w-full max-w-[min(100%,34vh)] rounded-xl overflow-hidden shadow-lg shadow-black/40">
|
||||
<Artwork seed={currentTrack ? `${currentTrack.title} ${currentTrack.artist}` : 'empty'} src={artwork} className="w-full h-full" rounded="xl" />
|
||||
</div>
|
||||
)}
|
||||
@@ -90,7 +96,7 @@ export function NowPlayingPanel({ onClose }: { onClose: () => void }) {
|
||||
<input
|
||||
type="range" min={0} max={Math.max(duration, 0.1)} step={0.1}
|
||||
value={Math.min(position, duration || 0)}
|
||||
onChange={(e) => setPosition(Number(e.target.value))}
|
||||
onChange={(e) => transport.seek(Number(e.target.value))}
|
||||
disabled={!currentTrack || duration <= 0}
|
||||
className="w-full cursor-pointer"
|
||||
/>
|
||||
@@ -101,15 +107,16 @@ export function NowPlayingPanel({ onClose }: { onClose: () => void }) {
|
||||
</div>
|
||||
|
||||
<div className="flex items-center justify-center gap-4 sm:gap-6">
|
||||
<button onClick={prev} aria-label="Previous" className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0"><SkipBack size={20} /></button>
|
||||
<button onClick={transport.prev} aria-label="Previous" className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0"><SkipBack size={20} /></button>
|
||||
<button
|
||||
onClick={() => isPlaying ? pause() : play()}
|
||||
onClick={transport.toggle}
|
||||
aria-label={isPlaying ? 'Pause' : 'Play'}
|
||||
disabled={!currentTrack}
|
||||
className="transport-btn"
|
||||
>
|
||||
{isPlaying ? <Pause size={18} fill="currentColor" /> : <Play size={18} fill="currentColor" />}
|
||||
</button>
|
||||
<button onClick={next} aria-label="Next" className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0"><SkipForward size={20} /></button>
|
||||
<button onClick={transport.next} aria-label="Next" className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0"><SkipForward size={20} /></button>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -6,7 +6,7 @@ import { Artwork } from './Artwork';
|
||||
import { ArtistLinks } from './ArtistLinks';
|
||||
import { formatDuration } from './TrackRow';
|
||||
import { DevicePicker } from './DevicePicker';
|
||||
import { usePlaybackSyncContext } from './PlaybackSyncProvider';
|
||||
import { useTransport } from '../hooks/useTransport';
|
||||
|
||||
interface PlaybackBarProps {
|
||||
queueOpen: boolean;
|
||||
@@ -16,20 +16,9 @@ interface PlaybackBarProps {
|
||||
}
|
||||
|
||||
export function PlaybackBar({ queueOpen, lyricsOpen, onToggleQueue, onToggleLyrics }: PlaybackBarProps) {
|
||||
const { currentTrack, isPlaying, position, duration, volume, shuffle, repeat, play, pause, next, prev, setPosition, setVolume, toggleShuffle, cycleRepeat } = usePlaybackStore();
|
||||
const { currentTrack, isPlaying, position, duration, volume, shuffle, repeat, setVolume, toggleShuffle, cycleRepeat } = usePlaybackStore();
|
||||
const dislikeTrack = useDislikeTrack();
|
||||
const { hasRemoteOwner, sendCommand } = usePlaybackSyncContext();
|
||||
|
||||
// While another device holds the audio, the transport is a remote: the press
|
||||
// travels to that device instead of starting a second stream here.
|
||||
const remote = {
|
||||
play: () => (hasRemoteOwner ? void sendCommand({ type: 'play' }) : play()),
|
||||
pause: () => (hasRemoteOwner ? void sendCommand({ type: 'pause' }) : pause()),
|
||||
next: () => (hasRemoteOwner ? void sendCommand({ type: 'next' }) : next()),
|
||||
prev: () => (hasRemoteOwner ? void sendCommand({ type: 'prev' }) : prev()),
|
||||
seek: (seconds: number) =>
|
||||
hasRemoteOwner ? void sendCommand({ type: 'seek', position: seconds }) : setPosition(seconds),
|
||||
};
|
||||
const transport = useTransport();
|
||||
|
||||
const handleDislike = () => {
|
||||
if (!currentTrack) return;
|
||||
@@ -94,18 +83,18 @@ export function PlaybackBar({ queueOpen, lyricsOpen, onToggleQueue, onToggleLyri
|
||||
>
|
||||
<Shuffle size={18} />
|
||||
</button>
|
||||
<button onClick={remote.prev} className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0" aria-label="Previous">
|
||||
<button onClick={transport.prev} className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0" aria-label="Previous">
|
||||
<SkipBack size={20} />
|
||||
</button>
|
||||
<button
|
||||
onClick={() => (isPlaying ? remote.pause() : remote.play())}
|
||||
onClick={transport.toggle}
|
||||
disabled={!currentTrack}
|
||||
className="transport-btn"
|
||||
aria-label={isPlaying ? 'Pause' : 'Play'}
|
||||
>
|
||||
{isPlaying ? <Pause size={18} fill="currentColor" /> : <Play size={18} fill="currentColor" />}
|
||||
</button>
|
||||
<button onClick={remote.next} className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0" aria-label="Next">
|
||||
<button onClick={transport.next} className="rounded-md p-2 text-muted hover:text-text hover:bg-surface0" aria-label="Next">
|
||||
<SkipForward size={20} />
|
||||
</button>
|
||||
<button
|
||||
@@ -122,7 +111,7 @@ export function PlaybackBar({ queueOpen, lyricsOpen, onToggleQueue, onToggleLyri
|
||||
<input
|
||||
type="range" min={0} max={Math.max(duration, 0.1)} step={0.1}
|
||||
value={Math.min(position, duration || 0)}
|
||||
onChange={(e) => remote.seek(Number(e.target.value))}
|
||||
onChange={(e) => transport.seek(Number(e.target.value))}
|
||||
disabled={!currentTrack || duration <= 0}
|
||||
className="flex-1 h-1 cursor-pointer"
|
||||
aria-label="Seek"
|
||||
|
||||
@@ -6,7 +6,7 @@ import { usePlaybackSync, type PlaybackSyncApi } from '../hooks/usePlaybackSync'
|
||||
* control. Mounting the hook twice would open two streams and register the same
|
||||
* browser as two devices.
|
||||
*/
|
||||
const PlaybackSyncContext = createContext<PlaybackSyncApi | null>(null);
|
||||
export const PlaybackSyncContext = createContext<PlaybackSyncApi | null>(null);
|
||||
|
||||
export function PlaybackSyncProvider({ children }: { children: ReactNode }) {
|
||||
const sync = usePlaybackSync();
|
||||
@@ -18,3 +18,12 @@ export function usePlaybackSyncContext(): PlaybackSyncApi {
|
||||
if (!value) throw new Error('usePlaybackSyncContext must be used inside PlaybackSyncProvider');
|
||||
return value;
|
||||
}
|
||||
|
||||
/**
|
||||
* The same session, or nothing when the caller sits outside the provider — a
|
||||
* test rendering one control, for instance. Callers that only need to know
|
||||
* whether the audio is elsewhere use this and treat absence as "play here".
|
||||
*/
|
||||
export function useOptionalPlaybackSync(): PlaybackSyncApi | null {
|
||||
return useContext(PlaybackSyncContext);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,68 @@
|
||||
import { renderHook, act } from '@testing-library/react';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import type { ReactNode } from 'react';
|
||||
import { PlaybackSyncContext } from '../components/PlaybackSyncProvider';
|
||||
import type { PlaybackSyncApi } from './usePlaybackSync';
|
||||
import { usePlaybackStore } from '../store/usePlaybackStore';
|
||||
import { useTransport } from './useTransport';
|
||||
|
||||
function wrapperFor(api: Partial<PlaybackSyncApi>) {
|
||||
const value = {
|
||||
deviceId: 'this-device',
|
||||
devices: [],
|
||||
isOwner: false,
|
||||
hasRemoteOwner: false,
|
||||
transferTo: async () => undefined,
|
||||
sendCommand: async () => undefined,
|
||||
...api,
|
||||
} as PlaybackSyncApi;
|
||||
return ({ children }: { children: ReactNode }) => (
|
||||
<PlaybackSyncContext.Provider value={value}>{children}</PlaybackSyncContext.Provider>
|
||||
);
|
||||
}
|
||||
|
||||
describe('useTransport', () => {
|
||||
it('plays here when this device holds the audio', () => {
|
||||
usePlaybackStore.setState({ isPlaying: false });
|
||||
const sendCommand = vi.fn();
|
||||
const { result } = renderHook(() => useTransport(), {
|
||||
wrapper: wrapperFor({ isOwner: true, hasRemoteOwner: false, sendCommand }),
|
||||
});
|
||||
|
||||
act(() => result.current.toggle());
|
||||
|
||||
expect(sendCommand).not.toHaveBeenCalled();
|
||||
expect(usePlaybackStore.getState().isPlaying).toBe(true);
|
||||
});
|
||||
|
||||
it('forwards the press when another device holds the audio', () => {
|
||||
usePlaybackStore.setState({ isPlaying: true });
|
||||
const sendCommand = vi.fn();
|
||||
const { result } = renderHook(() => useTransport(), {
|
||||
wrapper: wrapperFor({ hasRemoteOwner: true, sendCommand }),
|
||||
});
|
||||
|
||||
act(() => {
|
||||
result.current.toggle();
|
||||
result.current.next();
|
||||
result.current.seek(30);
|
||||
});
|
||||
|
||||
expect(sendCommand.mock.calls.map(([command]) => command)).toEqual([
|
||||
{ type: 'pause' },
|
||||
{ type: 'next' },
|
||||
{ type: 'seek', position: 30 },
|
||||
]);
|
||||
// The remote device is the one that stops; this one never started.
|
||||
expect(usePlaybackStore.getState().isPlaying).toBe(true);
|
||||
});
|
||||
|
||||
it('plays here when there is no sync session at all', () => {
|
||||
usePlaybackStore.setState({ isPlaying: true });
|
||||
const { result } = renderHook(() => useTransport());
|
||||
|
||||
act(() => result.current.pause());
|
||||
|
||||
expect(usePlaybackStore.getState().isPlaying).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,48 @@
|
||||
import { useMemo } from 'react';
|
||||
import { usePlaybackStore } from '../store/usePlaybackStore';
|
||||
import { useOptionalPlaybackSync } from '../components/PlaybackSyncProvider';
|
||||
|
||||
/**
|
||||
* The transport every control set should call.
|
||||
*
|
||||
* While another device holds the audio, a press has to travel to that device
|
||||
* instead of starting a second stream here. Controls that reach into the
|
||||
* playback store directly work on the desktop and silently do nothing useful
|
||||
* from a phone, so there is one place that makes the choice.
|
||||
*/
|
||||
export interface Transport {
|
||||
play: () => void;
|
||||
pause: () => void;
|
||||
toggle: () => void;
|
||||
next: () => void;
|
||||
prev: () => void;
|
||||
seek: (seconds: number) => void;
|
||||
/** True when the presses are being forwarded rather than played here. */
|
||||
remote: boolean;
|
||||
}
|
||||
|
||||
export function useTransport(): Transport {
|
||||
const sync = useOptionalPlaybackSync();
|
||||
const hasRemoteOwner = sync?.hasRemoteOwner ?? false;
|
||||
const sendCommand = sync?.sendCommand;
|
||||
|
||||
return useMemo(() => {
|
||||
const local = () => usePlaybackStore.getState();
|
||||
const away = hasRemoteOwner && sendCommand !== undefined;
|
||||
return {
|
||||
play: () => (away ? void sendCommand!({ type: 'play' }) : local().play()),
|
||||
pause: () => (away ? void sendCommand!({ type: 'pause' }) : local().pause()),
|
||||
toggle: () => {
|
||||
const playing = local().isPlaying;
|
||||
if (away) void sendCommand!({ type: playing ? 'pause' : 'play' });
|
||||
else if (playing) local().pause();
|
||||
else local().play();
|
||||
},
|
||||
next: () => (away ? void sendCommand!({ type: 'next' }) : local().next()),
|
||||
prev: () => (away ? void sendCommand!({ type: 'prev' }) : local().prev()),
|
||||
seek: (seconds: number) =>
|
||||
away ? void sendCommand!({ type: 'seek', position: seconds }) : local().setPosition(seconds),
|
||||
remote: away,
|
||||
};
|
||||
}, [hasRemoteOwner, sendCommand]);
|
||||
}
|
||||
Reference in New Issue
Block a user