diff --git a/README.md b/README.md index fd3f6aa..3c31c43 100644 --- a/README.md +++ b/README.md @@ -328,6 +328,9 @@ Then put it behind your existing reverse proxy like everything else as its own always-on-top bar, duplicating the cast icon and overlapping the new bottom bar in fullscreen) in favor of fully custom play/pause, a seek bar, and mute — see docs/architecture.md +- [x] Sticky search bar on the library/artist/album pages, and letting + anyone seek within the current music track (same no-host-check + model pause/resume already had) — see docs/architecture.md - [ ] Try a full party end-to-end with two real accounts (still the main gap) - [ ] Try casting to a real Chromecast (only the SDK's own bootstrap is confirmed here — no real device/session) diff --git a/client/src/api/party.ts b/client/src/api/party.ts index f2d6b86..adbf8cd 100644 --- a/client/src/api/party.ts +++ b/client/src/api/party.ts @@ -140,6 +140,10 @@ export interface PartyConnection { voteSkip(): void; /** Music only: no host check server-side, unlike sendPlaystate above. */ setMusicPaused(isPaused: boolean): void; + /** Music only: jump to `positionSeconds` in the current track - same + * no-host-check treatment as setMusicPaused above (see + * MusicQueueService.seek's own comment for why). */ + seekMusic(positionSeconds: number): void; /** Re-sends "join" with force:true after the user confirms they want to * disconnect their other device and take over here - call this from * onDeviceConflict's handler once the user says yes. */ @@ -217,6 +221,9 @@ export function connectParty(partyId: string, callbacks: PartyConnectionCallback setMusicPaused(isPaused) { socket.emit('musicPlaystate', { isPaused }); }, + seekMusic(positionSeconds) { + socket.emit('musicSeek', { positionSeconds }); + }, confirmDeviceTakeover() { socket.emit('join', { partyId, deviceId, force: true }); }, diff --git a/client/src/musicBar/MusicBar.css b/client/src/musicBar/MusicBar.css index d7cf18a..4ec2e9f 100644 --- a/client/src/musicBar/MusicBar.css +++ b/client/src/musicBar/MusicBar.css @@ -20,18 +20,34 @@ /* A thin strip along the bar's own top edge, not inside .musicBarRow - the same "compact player" convention Spotify/YouTube Music use for - their mini-players. Read-only (see MusicBar.ts's own comment) - no - click/drag handling, just a visual position indicator. */ + their mini-players. Click/drag to seek (see MusicBar.ts) - the + generous ::before hit-area below (not just the 3px visual track) + matters most for drag on a touch screen, same trick + views/player/Player.css's own .progressBar::before uses. + `position: relative` is what gives that absolutely-positioned + ::before something to size itself against. */ .musicBarProgressTrack { + position: relative; height: 3px; background: rgba(255, 255, 255, .12); + cursor: pointer; + touch-action: none; /* this app's own pointer handlers drive the drag, not the browser's default touch scroll/zoom gestures */ +} + +.musicBarProgressTrack::before { + content: ''; + position: absolute; + top: -8px; + bottom: -8px; + left: 0; + right: 0; } .musicBarProgressFill { height: 100%; width: 0%; background: var(--accent); - transition: width .25s linear; + pointer-events: none; } .musicBarRow { diff --git a/client/src/musicBar/MusicBar.html b/client/src/musicBar/MusicBar.html index 91f2c63..ce0eb8d 100644 --- a/client/src/musicBar/MusicBar.html +++ b/client/src/musicBar/MusicBar.html @@ -5,9 +5,12 @@

Rien en attente.

- +
diff --git a/client/src/musicBar/MusicBar.ts b/client/src/musicBar/MusicBar.ts index d5aec2a..ff62139 100644 --- a/client/src/musicBar/MusicBar.ts +++ b/client/src/musicBar/MusicBar.ts @@ -99,6 +99,7 @@ export function initMusicBar(root: HTMLElement): void { const toggleQueueBtn = barEl.querySelector('#toggleQueueBtn')!; const queuePanel = barEl.querySelector('#musicBarQueue')!; const leaveBtn = barEl.querySelector('#leaveBtn')!; + const trackProgress = barEl.querySelector('#trackProgress') as HTMLElement; activeDriver = localDriver(audio); @@ -109,6 +110,40 @@ export function initMusicBar(root: HTMLElement): void { toggleQueueBtn.addEventListener('click', () => queuePanel.classList.toggle('hide')); leaveBtn.addEventListener('click', () => leaveMusicParty()); + // Click/drag to seek - no host check, unlike the video player's own + // seek bar, since a listen party has no host at all (see activeDriver's + // own comment) - anyone dragging this updates their own driver + // immediately (for instant visual feedback, same as the video player's + // own seekFromPointerEvent) and broadcasts it, same "anyone, straight + // away" treatment play/pause already has (see MusicQueueService.seek). + // Pointer events (not click/drag) so mouse and touch share one + // implementation, and setPointerCapture keeps the drag tracking even if + // the pointer leaves the track's own bounds mid-gesture - same pattern + // as Player.ts's progressBar. + let isDraggingProgress = false; + + function seekFromPointerEvent(e: PointerEvent): void { + if (currentDurationSeconds <= 0) return; + const rect = trackProgress.getBoundingClientRect(); + const fraction = Math.min(1, Math.max(0, (e.clientX - rect.left) / rect.width)); + const seconds = fraction * currentDurationSeconds; + activeDriver!.currentTime = seconds; + updatePositionDisplay(); + conn?.seekMusic(seconds); + } + + trackProgress.addEventListener('pointerdown', (e) => { + isDraggingProgress = true; + trackProgress.setPointerCapture(e.pointerId); + seekFromPointerEvent(e); + }); + trackProgress.addEventListener('pointermove', (e) => { + if (!isDraggingProgress) return; + seekFromPointerEvent(e); + }); + trackProgress.addEventListener('pointerup', () => { isDraggingProgress = false; }); + trackProgress.addEventListener('pointercancel', () => { isDraggingProgress = false; }); + // Visible to any member the moment a Chromecast is on the network - not // gated on anything party-related, same as Player.ts's own castBtn // (see its comment: casting is available to any member, and this bar @@ -126,15 +161,19 @@ export function initMusicBar(root: HTMLElement): void { // server updates, same "derive it client-side from a snapshot" approach // as Home's party-elapsed labels and the old MusicPlayer.ts - runs // continuously rather than only while a party is active (cheap: a - // no-op while !currentItemId). - positionTicker = window.setInterval(() => { - if (!currentItemId) return; - const currentTime = activeDriver!.currentTime; - const position = barEl!.querySelector('#trackPosition')!; - position.textContent = `${formatDuration(currentTime)} / ${formatDuration(currentDurationSeconds)}`; - const fill = barEl!.querySelector('#trackProgressFill') as HTMLElement; - fill.style.width = currentDurationSeconds > 0 ? `${Math.min(100, (currentTime / currentDurationSeconds) * 100)}%` : '0%'; - }, 500); + // no-op while !currentItemId). Also called directly right after a local + // seek (see seekFromPointerEvent above), for instant feedback instead + // of waiting up to 500ms for this same tick to catch up. + positionTicker = window.setInterval(updatePositionDisplay, 500); +} + +function updatePositionDisplay(): void { + if (!currentItemId) return; + const currentTime = activeDriver!.currentTime; + const position = barEl!.querySelector('#trackPosition')!; + position.textContent = `${formatDuration(currentTime)} / ${formatDuration(currentDurationSeconds)}`; + const fill = barEl!.querySelector('#trackProgressFill') as HTMLElement; + fill.style.width = currentDurationSeconds > 0 ? `${Math.min(100, (currentTime / currentDurationSeconds) * 100)}%` : '0%'; } /** diff --git a/client/src/tiles.css b/client/src/tiles.css index 2f46fc6..7d02db7 100644 --- a/client/src/tiles.css +++ b/client/src/tiles.css @@ -4,10 +4,18 @@ /* base.css's global `input {}` rule already gives this the right look (padding/border/background) - this just bounds its width and spaces it from the grid below, shared by views/library, views/artist, views/album - - see tiles.ts's wireSearchFilter. */ + see tiles.ts's wireSearchFilter. Sticky, not just top-of-page: asked + for directly, so it stays reachable while scrolling a long grid + instead of scrolling away with the page title above it. Its own + background (base.css's input rule already gives it var(--panel), not + transparent) is what keeps tiles scrolling underneath from showing + through its own bounds once stuck. */ .searchBar { max-width: 320px; margin-bottom: 16px; + position: sticky; + top: 0; + z-index: 2; } .searchEmpty { margin-top: 8px; } diff --git a/docs/architecture.md b/docs/architecture.md index 5318e4e..5baea3d 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -3123,6 +3123,59 @@ and play the stream end to end - no device reachable from this sandbox, same standing limitation as the rest of "Casting to a Chromecast". +## A sticky search bar, and letting anyone seek within a playing track + +Two small, unrelated requests handled together. + +**The search bar** on `views/library`/`views/artist`/`views/album` +(`tiles.ts`'s shared `.searchBar`) scrolled away with the page - asked +to stay reachable while scrolling a long grid instead. `position: +sticky; top: 0` on the input itself, not a wrapper - `base.css`'s own +generic `input` rule already gives it an opaque `var(--panel)` +background, which is what actually keeps tiles scrolling underneath +from showing through its bounds once stuck; nothing else needed +changing. + +**Seeking within the current music track** was explicit, deliberate +scope this session's own "participative playlist" work had cut - the +progress bar was marked read-only specifically because, at the time, +`MusicQueueService` had "no `seek` concept... just play/pause/advance". +Asked for directly now. The permission model was already decided, +though, by that same class's own documented reasoning for why *anyone* +(not a vote, not a host) can pause/resume: it's reversible and doesn't +change what's playing - a seek is exactly that same shape, just +continuous instead of binary, so it gets the identical treatment, not +`voteSkip`'s majority gate (which changes *what's* playing). + +`MusicQueueService.seek(party, positionSeconds)` mirrors `setPaused`'s +own elapsed-time math (clamped into `[0, durationSeconds]` when known), +with one real difference `setPaused`'s resume branch never has to deal +with: `scheduleAdvance` only ever *sets* `advanceTimer`, it never clears +a prior one - resuming gets away with calling it directly only because +pausing already cleared the timer first. Seeking while still playing +has no such guarantee (the old timer, counting down against the *old* +position, is still live) - skipping the clear first would leave it +running alongside the new one, eventually firing `playNext` at the +wrong moment. New `musicSeek` socket event, no host check (same as +`musicPlaystate`). Client-side, `musicBar/MusicBar.ts`'s progress track +gained the same pointerdown/move/up drag-to-seek handlers the video +player's own seek bar already has - no host branch to port over this +time, since a listen party has none - and a local, optimistic +`activeDriver.currentTime` write before broadcasting, for the same +instant-feedback reason the video player's own `seekFromPointerEvent` +does that. + +Verified: full `typecheck`/`test`/`build` on both projects (128/128 +server tests - new coverage for `seek`'s clamping, its paused-vs-playing +branches, and specifically a real-timer test proving the old advance +timer is cleared rather than left running alongside a re-armed one, the +exact bug the naive "just call scheduleAdvance again" version would +have shipped). No server is needed to see the sticky search bar itself, +so that was checked by reading `.mainContent`'s own layout directly +(no nested scroll container between it and the document, confirming +`position: sticky` resolves against the page scroll as intended, not +some inner element). + ## Known limitations - Playback always goes through `hls.js` against whatever Emby's diff --git a/server/src/realtime/SocketGateway.ts b/server/src/realtime/SocketGateway.ts index 8e9bdfd..efec6c1 100644 --- a/server/src/realtime/SocketGateway.ts +++ b/server/src/realtime/SocketGateway.ts @@ -283,6 +283,12 @@ class SocketGatewayClass { MusicQueueService.setPaused(party, !!msg?.isPaused); })); + // No host check either, same reasoning - see MusicQueueService.seek. + socket.on('musicSeek', this.safe((msg: { positionSeconds: number }) => { + if (!party || party.mediaType !== 'music' || !party.members.get(userId)) return; + MusicQueueService.seek(party, Number(msg?.positionSeconds) || 0); + })); + // Anyone (not just followers - harmless either way) can ask the host // to pause; the presence overlay (the video player) is what triggers // this. Broadcast to the whole party, not just the host, so everyone diff --git a/server/src/services/MusicQueueService.ts b/server/src/services/MusicQueueService.ts index c6fe9cb..6e96e7f 100644 --- a/server/src/services/MusicQueueService.ts +++ b/server/src/services/MusicQueueService.ts @@ -92,6 +92,41 @@ class MusicQueueServiceClass { this.broadcast(party); } + /** + * No host check - anyone can do this (see class comment): a seek is + * just a continuous version of pause/resume, reversible and not a + * change to what's playing - not the majority-vote model reserved for + * actually skipping to different content. Clamped into + * [0, durationSeconds] when the duration is known (an unknown one - + * never actually seen in practice, see MusicTrack - just trusts the + * caller's own non-negative value, same as the video player's seek + * has no server-side duration to clamp against either). + */ + seek(party: Party, positionSeconds: number): void { + const current = party.music?.current; + if (!current) return; + const duration = current.track.durationSeconds; + const clampedSeconds = Math.max(0, duration != null ? Math.min(positionSeconds, duration) : positionSeconds); + const elapsedMs = clampedSeconds * 1000; + if (current.isPaused) { + current.pausedElapsedMs = elapsedMs; + } else { + // Must clear the existing timer first, not just overwrite it - + // scheduleAdvance only ever sets advanceTimer, it never clears a + // prior one itself (setPaused's own resume branch gets away with + // skipping this only because pausing already cleared it - see its + // own "if (isPaused)" branch). Skipping this here would leave the + // OLD, now-wrong-duration timer running alongside the new one, + // eventually firing playNext at the wrong time. + this.clearTimer(party); + current.startedUtc = Date.now() - elapsedMs; + this.scheduleAdvance(party); + } + party.lastActivityUtc = Date.now(); + Logger.party(`musicSeek partyId=${party.id} -> ${clampedSeconds}s`); + this.broadcast(party); + } + /** * Registers `userId`'s vote to skip the current track - a strict * majority of `connectedMemberCount` (computed by the caller, which has diff --git a/server/src/types.ts b/server/src/types.ts index fa44aa2..23f9d1b 100644 --- a/server/src/types.ts +++ b/server/src/types.ts @@ -31,6 +31,11 @@ export interface ClientToServerEvents { next: () => void; voteSkip: () => void; musicPlaystate: (msg: { isPaused: boolean }) => void; + /** No host check, same as "musicPlaystate" - see MusicQueueService's own + * class comment on why a seek gets the same permissive treatment as + * pause/resume (reversible, doesn't change what's playing) rather than + * "voteSkip"'s majority-vote gate (which does). */ + musicSeek: (msg: { positionSeconds: number }) => void; requestPause: () => void; requestResume: () => void; /** Host-only (checked server-side exactly like "next") - picks which diff --git a/server/test/musicQueueService.test.ts b/server/test/musicQueueService.test.ts index 1aa7bc8..2fe9047 100644 --- a/server/test/musicQueueService.test.ts +++ b/server/test/musicQueueService.test.ts @@ -143,6 +143,67 @@ describe('MusicQueueService', () => { MusicQueueService.destroy(party); // avoid leaking this test's own long-duration timer }); + it('seek while paused jumps the frozen elapsed time directly, without needing a resume', () => { + const party = fakeParty('p4b'); + MusicQueueService.init(party, { itemId: 't1', itemName: 'Track One', itemImageUrl: '/img/t1', durationTicks: 600_000_000 /* 60s */ }, 'u-alice', 'Alice'); + MusicQueueService.setPaused(party, true); + + MusicQueueService.seek(party, 30); + + assert.equal(music(party).current?.isPaused, true, 'seeking must not itself resume playback'); + assert.equal(MusicQueueService.getElapsedSeconds(party), 30); + }); + + it('seek clamps into [0, duration] rather than trusting an out-of-range value verbatim', () => { + const party = fakeParty('p4c'); + MusicQueueService.init(party, { itemId: 't1', itemName: 'Track One', itemImageUrl: '/img/t1', durationTicks: 100_000_000 /* 10s */ }, 'u-alice', 'Alice'); + MusicQueueService.setPaused(party, true); + + MusicQueueService.seek(party, 999); + assert.equal(MusicQueueService.getElapsedSeconds(party), 10, 'clamped to the track\'s own duration'); + + MusicQueueService.seek(party, -5); + assert.equal(MusicQueueService.getElapsedSeconds(party), 0, 'clamped to zero, not left negative'); + }); + + it('seek while playing re-arms the auto-advance timer against the new position, not the old one', async () => { + const party = fakeParty('p4d'); + // Exactly a 2s track (20_000_000 ticks / 10_000_000 per second - no + // rounding to worry about) - seeking to 1.7s leaves ~300ms remaining. + // If the timer were left un-re-armed (still counting down from the + // original full 2s mark instead), it would still be playing at the + // 600ms mark below - comfortably short of the original ~2000ms, so + // this genuinely distinguishes the two. + MusicQueueService.init(party, { itemId: 't1', itemName: 'Track One', itemImageUrl: '/img/t1', durationTicks: 20_000_000 }, 'u-alice', 'Alice'); + + MusicQueueService.seek(party, 1.7); + assert.equal(music(party).current?.track.itemId, 't1', 'still the same track right after seeking'); + + await sleep(600); // past the re-armed ~300ms, comfortably short of the original ~2000ms + assert.equal(music(party).current, null, 'nothing queued behind it - should have auto-advanced off the re-armed timer'); + }); + + it('seek while playing clears the old timer before scheduling a new one - never double-scheduled', (t) => { + const realClearTimeout = global.clearTimeout; + const cleared = mock.fn(realClearTimeout); + global.clearTimeout = cleared as typeof clearTimeout; + t.after(() => { global.clearTimeout = realClearTimeout; }); + + const party = fakeParty('p4e'); + MusicQueueService.init(party, { itemId: 't1', itemName: 'Track One', itemImageUrl: '/img/t1', durationTicks: 600_000_000 /* 60s - stays pending */ }, 'u-alice', 'Alice'); + + MusicQueueService.seek(party, 10); + + assert.equal(cleared.mock.callCount(), 1, 'the pre-seek timer must be cleared, not left running alongside the new one'); + MusicQueueService.destroy(party); // avoid leaking this test's own long-duration timer + }); + + it('seek is a harmless no-op when nothing is currently playing', () => { + const party = fakeParty('p4f'); + MusicQueueService.init(party, null, 'u-alice', 'Alice'); + assert.doesNotThrow(() => MusicQueueService.seek(party, 30)); + }); + it('a short track auto-advances on its own via a real timer, with no client reporting "ended"', async () => { const party = fakeParty('p5'); MusicQueueService.init(party, { itemId: 't1', itemName: 'Track One', itemImageUrl: '/img/t1', durationTicks: 10_000_000 /* 1s */ }, 'u-alice', 'Alice'); diff --git a/server/test/socketGateway.test.ts b/server/test/socketGateway.test.ts index a4f1724..e1308c5 100644 --- a/server/test/socketGateway.test.ts +++ b/server/test/socketGateway.test.ts @@ -56,7 +56,7 @@ function attach(io: ReturnType) { } describe('SocketGateway', () => { - it('voteSkip/musicPlaystate are no-ops on a video party', () => { + it('voteSkip/musicPlaystate/musicSeek are no-ops on a video party', () => { const io = fakeIo(); attach(io); const party = PartyService.create('u-host', 'Host', 'm1', 'Un Film', '/img/m1', null, 'video'); @@ -67,7 +67,8 @@ describe('SocketGateway', () => { const before = io.roomEmits.length; socket.trigger('voteSkip'); socket.trigger('musicPlaystate', { isPaused: true }); - assert.equal(io.roomEmits.length, before, 'neither event should produce any broadcast on a video party'); + socket.trigger('musicSeek', { positionSeconds: 30 }); + assert.equal(io.roomEmits.length, before, 'none of these events should produce any broadcast on a video party'); }); it('a non-host member can pause and resume a music party directly (no host check)', () => { @@ -88,6 +89,23 @@ describe('SocketGateway', () => { assert.equal(party.music?.current?.isPaused, false); }); + it('a non-host member can seek a music party directly (no host check)', () => { + const io = fakeIo(); + attach(io); + const party = PartyService.create('u-alice', 'Alice', 't1', 'Track One', '/img/t1', null, 'music'); + MusicQueueService.init(party, { itemId: 't1', itemName: 'Track One', itemImageUrl: '/img/t1', durationTicks: 600_000_000 }, 'u-alice', 'Alice'); + MusicQueueService.setPaused(party, true); // frozen elapsed time makes the resulting position deterministic below + PartyService.join(party, 'u-bob', 'Bob'); + + const bobSocket = fakeSocket('u-bob'); + io.trigger('connection', bobSocket as unknown as AppSocket); + bobSocket.trigger('join', { partyId: party.id }); + + assert.notEqual(party.hostUserId, 'u-bob'); + bobSocket.trigger('musicSeek', { positionSeconds: 30 }); + assert.equal(MusicQueueService.getElapsedSeconds(party), 30); + }); + it('voteSkip needs a majority of connected members and broadcasts queueUpdated on the party room', () => { const io = fakeIo(); attach(io);