yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

fix(player): give the seek bar's interval one owner

Browse Source 
`handleInput()` called `stopProgress()` and mutated 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`
event or the next backend report could bring it back — so an `input`
that never commits 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, all ordinary gestures on a phone. While playing the
1 Hz report papered over it within a second; with reports not arriving
it was permanent.

The drag is `@state` now and `updated()` decides whether the interval
runs, so there is one place that knows. `handleChange` no longer starts
it directly for the same reason.

A flag set on `input` can strand, which would turn a stall of up to a
second into a permanent one — the failure this removes. `change` is the
ordinary end; `pointerup`/`pointercancel`/`touchend`/`touchcancel` on
the document are the ends that are not, attached with the drag and
dropped with it, because the pointer is routinely released outside the
element it started in.

The other half is that a report arriving mid-drag used to overwrite
`seekValue` and pull the thumb out from under the finger once a second.
It is skipped while dragging, and its seq is deliberately left
unrecorded so the first report after the drag still counts as fresh.

Three tests, all exercised against the fault: two fail on the old
component, and the third fails if the drag flag is left set — which is
the failure mode the fix introduces and the listeners exist to prevent.
Verified on the device too (Chrome 113): mid-drag the bar holds its
value and ignores reports, and on release it adopts the backend's real
position and resumes ticking.

Closes #164
This commit is contained in:
logan
2026-08-20 17:17:23 -04:00
parent de04339494
commit fe1fbefee7
2 changed files with 178 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files
frontend/src/components/audio-player/seekbar/seek-bar.ts
+73 -9
View File
@@ -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;
frontend/test/components/transport.test.ts
+105
View File
@@ -397,6 +397,111 @@ describe('<seek-bar>', () => {
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<HTMLElement & { value: number }>(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<HTMLElement & { value: number }>(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<HTMLElement & { value: number }>(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');