diff --git a/frontend/src/components/audio-player/seekbar/seek-bar.ts b/frontend/src/components/audio-player/seekbar/seek-bar.ts index 5ef3ea6..85cacf4 100644 --- a/frontend/src/components/audio-player/seekbar/seek-bar.ts +++ b/frontend/src/components/audio-player/seekbar/seek-bar.ts @@ -22,6 +22,24 @@ export class SeekBar extends LitElement { @state() private seekValue: number = 0; + /** + * Whether the user is dragging the thumb right now. + * + * It is `@state` rather than a plain field because `updated()` owns + * the interval and only reactive state brings `updated()` round. A + * bare `stopProgress()` in the input handler mutated nothing, so + * nothing re-rendered, so the tail of `updated()` that restarts the + * interval never ran — and the only things that could restart it + * were a `change` event or the next backend report. Any `input` + * without a committed `change` therefore froze the interpolation: + * a drag cancelled outside the element, a pointer taken by a scroll, + * or a touch on the track treated as a scrub, which on a phone are + * ordinary gestures. While playing, the 1 Hz report papered over it + * within a second; with reports not arriving it was permanent. + */ + @state() + private dragging: boolean = false; + /** Whether the right-hand clock shows time remaining or total. */ @state() private showRemaining: boolean = true; @@ -133,6 +151,7 @@ export class SeekBar extends LitElement { override disconnectedCallback() { super.disconnectedCallback(); this.stopProgress(); + this.endDrag(); } override updated() { @@ -154,18 +173,33 @@ export class SeekBar extends LitElement { // A report for a track that is no longer loaded is stale by // definition: the change id is the only thing that distinguishes // it, since the same file can play twice in a row. + // + // A report arriving mid-drag is deliberately *not* applied: the + // thumb belongs to the finger on it, and adopting a report once a + // second pulls it back out from under them. The seq is left + // unrecorded too, so the first report after the drag still counts + // as fresh. const position = this.player.position; const forThisTrack = position !== null && position.trackChangeId === currentChangeId; - if (position && forThisTrack && position.seq !== this.previousPositionSeq) { + if ( + position && + forThisTrack && + !this.dragging && + position.seq !== this.previousPositionSeq + ) { this.previousPositionSeq = position.seq; this.seekValue = position.positionSeconds; this.stopProgress(); } - // Start/stop progress interval based on playback state - if (this.isPlaying && this.hasTrack) { + // One owner for the interval, and this is it. Every other place + // that wants it started or stopped says so by changing state that + // brings us back here, so the timer cannot be left running by a + // path that forgot to stop it or stopped by a path that forgot to + // start it again. + if (this.isPlaying && this.hasTrack && !this.dragging) { this.startProgress(); } else { this.stopProgress(); @@ -210,18 +244,48 @@ export class SeekBar extends LitElement { private handleChange(e: Event) { const newSeekVal = (e.target as WaSlider).value; + this.endDrag(); this.setSeekValue(newSeekVal); this.player.seek(newSeekVal); + } - if (this.isPlaying) { - this.startProgress(); + /** + * The user is moving the thumb. + * + * This only records that fact; `updated()` decides what it means for + * the interval. `seekValue` follows the slider so the clocks track + * the thumb during the drag rather than jumping when it is released. + */ + private handleInput(e: Event) { + this.setSeekValue((e.target as WaSlider).value); + + if (this.dragging) { + return; } + + this.dragging = true; + + // A drag that never commits must not strand the flag, or this fix + // turns a stall of up to one second into a permanent one -- which + // is the failure it exists to remove. `change` is the ordinary + // end; these are the ones that are not, and they are on the + // document because the pointer is routinely released outside the + // element it started in. A drag's listeners belong to the drag, + // so they go on with it and come off with it. + document.addEventListener('pointerup', this.endDrag); + document.addEventListener('pointercancel', this.endDrag); + document.addEventListener('touchend', this.endDrag); + document.addEventListener('touchcancel', this.endDrag); } - // Stops progress while user is dragging the thumb - private handleInput() { - this.stopProgress(); - } + private endDrag = () => { + document.removeEventListener('pointerup', this.endDrag); + document.removeEventListener('pointercancel', this.endDrag); + document.removeEventListener('touchend', this.endDrag); + document.removeEventListener('touchcancel', this.endDrag); + + this.dragging = false; + }; private setSeekValue(val: number) { if (val < 0) val = 0; diff --git a/frontend/test/components/transport.test.ts b/frontend/test/components/transport.test.ts index f503c02..7863a05 100644 --- a/frontend/test/components/transport.test.ts +++ b/frontend/test/components/transport.test.ts @@ -397,6 +397,111 @@ describe('', () => { expect(lastArgs('player.Player.Seek')).toEqual([42]); }); + // #164. `handleInput` used to call `stopProgress()` and mutate no + // reactive state, so Lit scheduled no update, `updated()` never ran, + // and the tail of `updated()` that restarts the interval never + // executed. Only a `change` or the next backend report could bring + // it back -- so an `input` that never commits froze the clock, which + // on a touch device is an ordinary cancelled gesture. With no + // reports arriving, that is permanent. + it('keeps ticking after a drag that never commits', async () => { + vi.useFakeTimers(); + + const el = await fixture('seek-bar'); + + emit(Events.TrackChanged, TRACK); + emit(Events.PlaybackStateChanged, { state: 'playing' }); + await vi.advanceTimersByTimeAsync(2000); + await el.updateComplete; + + // A touch lands on the track and is then cancelled: `input`, and + // no `change` ever follows. + const slider = shadow(el, 'wa-slider'); + + if (slider) slider.value = 20; + + slider?.dispatchEvent(new Event('input')); + await el.updateComplete; + + document.dispatchEvent(new Event('pointerup')); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + await vi.advanceTimersByTimeAsync(3000); + await el.updateComplete; + + expect(text(el, '[data-testid="elapsed-time"]')).toBe('00:23'); + }); + + // The other half of the same fix: while the thumb is held, a report + // arriving once a second used to overwrite `seekValue` and pull it + // back out from under the finger. + it('leaves the thumb where the finger is while a drag is live', async () => { + vi.useFakeTimers(); + + const el = await fixture('seek-bar'); + + emit(Events.TrackChanged, { ...TRACK, trackChangeId: 20 }); + emit(Events.PlaybackStateChanged, { state: 'playing' }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + const slider = shadow(el, 'wa-slider'); + + if (slider) slider.value = 60; + + slider?.dispatchEvent(new Event('input')); + await el.updateComplete; + + emit(Events.PlaybackPositionChanged, { + positionSeconds: 4, + trackLength: 90, + trackChangeId: 20, + seq: 7, + playing: true, + }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + expect(text(el, '[data-testid="elapsed-time"]')).toBe('01:00'); + }); + + // And the drag must not hold the interval hostage once it ends: the + // report that was skipped mid-drag is not recorded as seen, so the + // next one is still fresh and is applied. + it('takes the backend back as the authority once the drag commits', async () => { + vi.useFakeTimers(); + + const el = await fixture('seek-bar'); + + emit(Events.TrackChanged, { ...TRACK, trackChangeId: 21 }); + emit(Events.PlaybackStateChanged, { state: 'playing' }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + const slider = shadow(el, 'wa-slider'); + + if (slider) slider.value = 60; + + slider?.dispatchEvent(new Event('input')); + await el.updateComplete; + + slider?.dispatchEvent(new Event('change')); + await el.updateComplete; + + emit(Events.PlaybackPositionChanged, { + positionSeconds: 61, + trackLength: 90, + trackChangeId: 21, + seq: 9, + playing: true, + }); + await vi.advanceTimersByTimeAsync(0); + await el.updateComplete; + + expect(text(el, '[data-testid="elapsed-time"]')).toBe('01:01'); + }); + it('bounds the slider by the track length', async () => { const el = await fixture('seek-bar');