Compare commits

..
Author SHA1 Message Date
logan d6b48fb3ac fix(player): stop the seek bar resizing as its clocks count
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m28s
CI / e2e (pull_request) Successful in 6m16s
Two different things moved it and they need different answers. Digits
in a proportional font are different widths, so 1:11 is narrower than
4:08 and the bar breathed once a second -- tabular figures fix that.
The character *count* changes too, at the hundredth minute and whenever
the right-hand clock is toggled to remaining and grows a minus sign,
which a figure width cannot fix -- so each clock reserves the widest
string this track can put in it.

The budget is per track rather than a constant: reserving six
characters on every track would push the slider in by a character at
each end to buy nothing.

Measured in the component tier: 4.5px of drift across three positions
before, none after.

Refs #13
2026-08-18 11:04:21 -04:00
4 changed files with 98 additions and 110 deletions
@@ -68,6 +68,29 @@ export class SeekBar extends LitElement {
align-items: center;
}
/* The clocks must not resize as they count.
Two things move them, and they need different answers. Digits in
a proportional font are different widths, so 1:11 is narrower
than 4:08 and the bar breathed once a second -- that is what
tabular figures fix. The character *count* changes too, at the
hundredth minute and whenever the right-hand clock is toggled to
remaining and grows a minus sign, and a figure width cannot fix
that -- so each clock also reserves the widest string this track
can put in it. The budget is per track rather than a constant
because reserving six characters on every track would push the
slider in by a character at each end for nothing. */
#seek-bar-container small,
.time-toggle {
font-variant-numeric: tabular-nums;
flex: 0 0 auto;
min-width: calc(var(--yj-clock-chars, 5) * 1ch);
}
#seek-bar-container small {
text-align: left;
}
.time-toggle {
background: none;
border: none;
@@ -76,6 +99,9 @@ export class SeekBar extends LitElement {
font: inherit;
font-size: var(--wa-font-size-s, 0.875rem);
cursor: pointer;
/* One more for the minus sign the remaining form carries. */
min-width: calc((var(--yj-clock-chars, 5) + 1) * 1ch);
text-align: right;
}
.time-toggle:hover,
@@ -219,8 +245,19 @@ export class SeekBar extends LitElement {
: formatSeconds(this.trackLength);
const rightTime = this.hasTrack ? rightLabel : '--:--';
// The widest string either clock can hold for *this* track. The
// duration is the longest elapsed value there can be, so its length
// is the budget; `--:--` is five, which is also the floor.
const clockChars = Math.max(
5,
this.hasTrack ? formatSeconds(this.trackLength).length : 0,
);
return html`
<div id="seek-bar-container">
<div
id="seek-bar-container"
style="--yj-clock-chars: ${clockChars}"
>
<small data-testid="elapsed-time">${elapsedTime}</small>
<wa-slider
label="Seek"
@@ -662,21 +662,34 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost {
font-weight: 400;
}
/* The request control is offered on every row that has
* something to request, and is not revealed on hover.
/* The request control is only offered where there is
* something to request, and only when the row is being
* attended to — a column of plus signs down a mostly-owned
* album is the clutter the green ticks were.
*
* It used to be transparent until the row was hovered or
* focused, on the reasoning that a column of plus signs
* down a mostly-owned album is clutter. That reasoning was
* inherited from the green ticks it replaced and does not
* survive the rule those were removed for: a tick marked
* the *common* case, while this marks the rows that are
* **not** here. A mark on the exception is the information
* on this page — and one that appears only under the
* pointer cannot be seen, counted, or reached by anyone
* driving this with a finger. */
* Hidden with opacity, never display:none or visibility,
* so it keeps its place in the layout (rows do not reflow
* as the pointer moves) and stays in the tab order and the
* accessibility tree. focus-within is what makes it
* reachable without a mouse: tabbing to the button reveals
* it, and the row's own focus reveals it before you get
* there. */
.track-row .track-request {
flex-shrink: 0;
opacity: 0;
transition: opacity 0.12s ease;
}
.track-row:hover .track-request,
.track-row:focus-within .track-request,
.track-row .track-request:focus-visible {
opacity: 1;
}
@media (prefers-reduced-motion: reduce) {
.track-row .track-request {
transition: none;
}
}
`,
];
@@ -1,97 +0,0 @@
/**
* The request badge on an unowned row is there without being hovered.
*
* It used to be transparent until the row was hovered or focused, on
* the reasoning that a column of plus signs down a mostly-owned album
* is clutter. That reasoning came from the green ticks it replaced and
* does not survive the rule those were removed for: a tick marked the
* **common** case, while this marks the rows that are *not* here. A
* mark on the exception is the information on this page, and one that
* exists only under the pointer cannot be seen, counted, or reached by
* anyone driving the app with a finger.
*
* That the badge *repaints* when clicked is the other half of #33 and
* is covered by `album-track-request.test.ts`.
*/
import { describe, expect, it, beforeEach } from 'vitest';
import type { LitElement } from 'lit';
import '@components/explore-album-details/explore-album-details';
import { stub, flush, resetHarness } from '@test/support/harness';
import { fixture, shadowAll } from '@test/support/render';
function track(n: number, owned: boolean) {
return {
position: n,
discNumber: 1,
title: `Track ${n}`,
length: 200000,
mbid: `mbid-${n}`,
inLibrary: owned,
};
}
/** An album with one owned track and one that is not here. */
async function albumWithAnUnownedTrack(): Promise<LitElement> {
const el = await fixture<LitElement>('explore-album-details', {
albumName: 'Glass Harbour',
releaseGroupMBID: 'rg-1',
});
stub('library.Library.GetFilePathsByRecordingMBIDs', {
'mbid-1': ['/music/mbid-1.mp3'],
});
Object.assign(el, {
versionEntries: [
{
key: 'v1',
label: '2019',
sublabel: '2 tracks',
tracks: [track(1, true), track(2, false)],
},
],
selectedVersionKey: 'v1',
loadingReleases: false,
loadingInfo: false,
});
el.requestUpdate();
await flush();
await el.updateComplete;
return el;
}
const badges = (el: LitElement) =>
shadowAll(el, 'library-status-indicator.track-request');
describe('the tracklists request badge', () => {
beforeEach(() => {
resetHarness();
stub('library.Library.GetFilePathsByRecordingMBIDs', {});
stub('library.Library.GetFilePathsByAlbums', {});
stub('library.Library.GetAlbumTracks', []);
stub('library.Library.GetAllLibrariesWithTrackCounts', []);
stub('download.Service.ProviderKinds', []);
stub('download.Service.ListProviders', []);
stub('download.Service.ListDownloads', []);
stub('download.Service.ListRequests', []);
});
it('is visible without a pointer anywhere near it', async () => {
const el = await albumWithAnUnownedTrack();
const [badge] = badges(el);
expect(badge).toBeTruthy();
// Computed opacity rather than the absence of a rule, because the
// rule could come back under a different selector.
expect(getComputedStyle(badge!).opacity).toBe('1');
});
it('is still only on the rows with something to request', async () => {
// Always-visible is not the same as everywhere: an owned track has
// nothing left to ask for, and a badge on it would be the column of
// green ticks this page deliberately stopped drawing.
expect(badges(await albumWithAnUnownedTrack())).toHaveLength(1);
});
});
@@ -227,6 +227,41 @@ describe('<seek-bar>', () => {
expect(text(el, '[data-testid="remaining-time"]')).toBe('01:30');
});
it('keeps the slider still as the clocks count', async () => {
const el = await fixture('seek-bar');
emit(Events.TrackChanged, { ...TRACK, trackChangeId: 31 });
await flush();
await el.updateComplete;
const slider = () =>
shadow(el, 'wa-slider')!.getBoundingClientRect();
const before = slider();
// 1:11 against 4:08 is the reported jitter: different digits, and
// in a proportional font different widths. Toggling the right-hand
// clock is the other half -- the minus sign is a whole character.
for (const positionSeconds of [8, 71, 88]) {
emit(Events.PlaybackPositionChanged, {
positionSeconds,
trackLength: 90,
trackChangeId: 31,
seq: positionSeconds,
playing: true,
});
await flush();
await el.updateComplete;
expect(slider().width).toBeCloseTo(before.width, 1);
expect(slider().left).toBeCloseTo(before.left, 1);
}
await click(el, '[data-testid="remaining-time"]');
await el.updateComplete;
expect(slider().width).toBeCloseTo(before.width, 1);
});
it('renders the position the backend reports rather than its own count', async () => {
const el = await fixture('seek-bar');