From 85ca9cf54385319b5fab899a5b86497b0517b60e Mon Sep 17 00:00:00 2001
From: kami <kami@homesrv>
Date: Sat, 8 Aug 2026 20:56:30 +0400
Subject: [PATCH] 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>
---
 frontend/src/components/NowPlayingPanel.tsx   | 21 ++++--
 frontend/src/components/PlaybackBar.tsx       | 25 ++-----
 .../src/components/PlaybackSyncProvider.tsx   | 11 ++-
 frontend/src/hooks/useTransport.test.tsx      | 68 +++++++++++++++++++
 frontend/src/hooks/useTransport.ts            | 48 +++++++++++++
 5 files changed, 147 insertions(+), 26 deletions(-)
 create mode 100644 frontend/src/hooks/useTransport.test.tsx
 create mode 100644 frontend/src/hooks/useTransport.ts

diff --git a/frontend/src/components/NowPlayingPanel.tsx b/frontend/src/components/NowPlayingPanel.tsx
index 2b29d60..9702f0a 100644
--- a/frontend/src/components/NowPlayingPanel.tsx
+++ b/frontend/src/components/NowPlayingPanel.tsx
@@ -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>
 
diff --git a/frontend/src/components/PlaybackBar.tsx b/frontend/src/components/PlaybackBar.tsx
index 5b63289..4afadfc 100644
--- a/frontend/src/components/PlaybackBar.tsx
+++ b/frontend/src/components/PlaybackBar.tsx
@@ -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"
diff --git a/frontend/src/components/PlaybackSyncProvider.tsx b/frontend/src/components/PlaybackSyncProvider.tsx
index bf11ae5..ef41010 100644
--- a/frontend/src/components/PlaybackSyncProvider.tsx
+++ b/frontend/src/components/PlaybackSyncProvider.tsx
@@ -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);
+}
diff --git a/frontend/src/hooks/useTransport.test.tsx b/frontend/src/hooks/useTransport.test.tsx
new file mode 100644
index 0000000..4e59f44
--- /dev/null
+++ b/frontend/src/hooks/useTransport.test.tsx
@@ -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);
+  });
+});
diff --git a/frontend/src/hooks/useTransport.ts b/frontend/src/hooks/useTransport.ts
new file mode 100644
index 0000000..c17355f
--- /dev/null
+++ b/frontend/src/hooks/useTransport.ts
@@ -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]);
+}