From ec64dbded0935415cae5d642dc201e014e36a8cc Mon Sep 17 00:00:00 2001 From: Logan Date: Fri, 21 Aug 2026 18:12:29 -0400 Subject: [PATCH] fix(player): give the seek bar a thumb-sized hit area On now-playing-view -- the screen that exists so a phone has somewhere to seek from -- the slider measured 261x6 on the reference device. Six pixels is the whole of the drag target on the app's primary seeking affordance, against the 44px floor the app set for itself in #56 and holds to in the queue panel. **The phone rule had never applied**, which is why the issue read as "the thickening stops short" rather than "there is no thickening". seek-bar's stylesheet asked for a 12px track below 599px and then set 6px in a plain `wa-slider` rule *written after it*. A media query adds no specificity, so the plain rule won at every width: the source said 12 and the device said 6. That is index.css's documented rule -- "the phone section is last on purpose" -- met inside a component's own stylesheet, where nothing in any tier renders differently to say so. The block is last now, and the 12px track it always asked for is real. **And 12px is still under the floor**, so the target is built around the painted track rather than by thickening it. The two are allowed to differ and a slider is the clearest case where they should: a 44px progress bar would be wrong-looking and would cost the album art the vertical space #51 spent an issue recovering. Two things about how it is built, both settled by measurement on the device rather than by choosing a number. **The padding goes on ::part(slider), not on the host.** That is the issue's untested claim, and the answer is the pessimistic one: the inner div is what carries the gesture -- it holds the listener and the touch-action: none -- and it is exactly the host's size, so padding the host would grow a box that does not take the press. **The padding is asymmetric and the margins cancel it**, so the row does not grow by the difference. The seek row is 19px -- its clocks, not the track, decide that -- and the play button's top edge is 8px below it, while `.art` above is a non-interactive div. A symmetric 44px target reaches into the play button, and growing the row instead cost the art 25px of 143 when it was tried. So the target takes the space above. Verified on the device at 424x439: hit area 261x44 where it was 261x6, painted track 12px, seek row still 19px, album art still 143px, 7px of clearance left under the play button, a press 26px above the track seeks, and a hit test on the play button's top edge still reaches the play button. The desktop bottom bar is untouched: the rule is inside the phone query and that instance is display:none below 600px anyway. The test asserts the parsed stylesheet, on hover-affordance.test.ts's precedent and with the same limitation stated -- no tier here lays out a real wa-slider at a phone width, and a number measured on a phone is not a number CI can assert. What it holds is the shape: that the phone block is last, that padding plus track clears 44, that the margins cancel the padding, and that the growth is upward. All four are invisible on a desktop, and the first is exactly what a tidy-up undoes. Closes #187 --- .../audio-player/seekbar/seek-bar.ts | 63 +++++- .../test/components/seek-bar-hit-area.test.ts | 211 ++++++++++++++++++ 2 files changed, 262 insertions(+), 12 deletions(-) create mode 100644 frontend/test/components/seek-bar-hit-area.test.ts diff --git a/frontend/src/components/audio-player/seekbar/seek-bar.ts b/frontend/src/components/audio-player/seekbar/seek-bar.ts index 85cacf4..8ec51c2 100644 --- a/frontend/src/components/audio-player/seekbar/seek-bar.ts +++ b/frontend/src/components/audio-player/seekbar/seek-bar.ts @@ -45,18 +45,6 @@ export class SeekBar extends LitElement { private showRemaining: boolean = true; static override styles = [designTokens, waSliderLabel, css` - /* 12px below the phone breakpoint. The bottom bar's seek bar is - display:none there (016 B2 phase 1), so the only instance a - viewport media query can reach at that width is the full-screen - now-playing view's -- which is exactly the one a thumb uses. - The track size lives on wa-slider inside this shadow root, so a - custom property set by the host would not reach it. */ - @media (max-width: 599px) { - wa-slider { - --track-size: 12px; - } - } - wa-slider { --track-size: 6px; flex: 1; @@ -80,6 +68,57 @@ export class SeekBar extends LitElement { background: var(--yj-bg-base, black); } + /* The phone's seek bar, and this block is last on purpose. + + A media query adds no specificity, so this lived above the plain + "wa-slider" rule and lost to it at every width: the 12px track it + asks for had never once applied, and the bar measured 261x6 on + the device while the source said 12. That is index.css's rule + ("the phone section is last on purpose") met inside a component's + own stylesheet, and nothing renders differently in any tier here + to say so. + + The bottom bar's seek bar is display:none below this width (016 + B2 phase 1), so the only instance a viewport media query can + reach is the full-screen now-playing view's -- which is exactly + the one a thumb uses. The desktop bar keeps its 6px, where a + mouse is precise and the thickness is right. + + The painted track and the thing you can hit are allowed to + differ, and a slider is the clearest case where they should: 12px + is a progress bar you can see, and 44px is the app's touch floor + (#56). A 44px-*thick* bar would be wrong-looking and would cost + the album art the vertical space #51 spent an issue recovering. + + Two things about how the target is built. + + The padding goes on ::part(slider) rather than on the host, + because that inner div is what carries the gesture -- it has the + listener and the touch-action: none, and it is exactly the host's + size, so padding the host would grow a box that does not take the + press. + + The padding is asymmetric and the margins cancel it, so the row + does not grow by the difference. Both halves are measured: the + seek row is 19px (its clocks, not the track, decide that) and the + play button's top edge is 8px below it, so the target takes the + space *above*, where .art is a non-interactive div. Growing the + row instead cost the art 25px of 143. Verified on the device at + 424x439: hit area 44px, painted track 12px, row still 19px, art + still 143px, 8px of clearance left under the play button, a press + 26px above the track seeks, and a hit test on the play button's + top edge still reaches the play button. */ + @media (max-width: 599px) { + wa-slider { + --track-size: 12px; + } + + wa-slider::part(slider) { + padding-block: 28px 4px; + margin-block: -28px -4px; + } + } + #seek-bar-container { display: flex; justify-content: space-between; diff --git a/frontend/test/components/seek-bar-hit-area.test.ts b/frontend/test/components/seek-bar-hit-area.test.ts new file mode 100644 index 0000000..9517751 --- /dev/null +++ b/frontend/test/components/seek-bar-hit-area.test.ts @@ -0,0 +1,211 @@ +/** + * The seek bar's painted track and the thing you can hit are allowed to + * differ, and a slider is the clearest case where they should. + * + * On `now-playing-view` — the screen that exists so a phone has + * somewhere to seek from — the slider measured 261x6 on the reference + * device (#187). Six pixels is the whole of the drag target on the + * app's primary seeking affordance, against a 44px floor the app set + * for itself in #56 and holds to in the queue panel. + * + * Two separate faults, and the first is why the second was not obvious. + * + * **The phone rule had never applied.** `seek-bar`'s stylesheet asked + * for a 12px track below 599px and then set 6px in a plain `wa-slider` + * rule *written after it*. A media query adds no specificity, so the + * plain rule won at every width — which is `index.css`'s documented + * rule ("the phone section is last on purpose") reproduced inside a + * component's own stylesheet. The source said 12 and the device said 6. + * + * **And 12px would still be under the floor**, so the target is built + * around the track rather than by thickening it: padding on the part + * that carries the gesture, with margins cancelling it so the row does + * not grow. + * + * This is asserted against the *parsed stylesheet*, on + * `hover-affordance.test.ts`'s precedent and with the same limitation + * stated rather than hidden: no tier here renders at a phone width with + * a real `wa-slider` laid out, so what can be checked is the shape the + * browser built from the css`` literal. The pixel measurements that + * chose these numbers were taken on the device and are recorded on + * #187 and in the stylesheet's own comment — a number measured on a + * phone is not a number CI can assert. + * + * Which is the regression worth catching anyway. Both failures are + * invisible on a desktop: hoisting the block back above the plain rule + * renders identically at every width CI runs at, and it is exactly what + * a tidy-up does. + */ +import { describe, expect, it } from 'vitest'; + +import '@components/audio-player/seekbar/seek-bar'; +import { fixture } from '@test/support/render'; + +/** The app's touch floor, from #56. */ +const TOUCH_FLOOR = 44; + +/** The width below which the phone's rules apply. */ +const PHONE_QUERY = /max-width:\s*599px/; + +type Rule = { text: string; condition: string | null }; + +/** + * Every rule in the element's own adopted stylesheets, flattened **in + * order**, which is the whole point here: the fault being guarded is a + * rule sitting in the wrong place, not a rule being absent. + */ +function rulesOf(host: Element): Rule[] { + const sheets = host.shadowRoot?.adoptedStyleSheets ?? []; + const out: Rule[] = []; + + for (const sheet of sheets) { + for (const rule of Array.from(sheet.cssRules)) { + if (rule instanceof CSSMediaRule) { + for (const inner of Array.from(rule.cssRules)) { + out.push({ text: inner.cssText, condition: rule.conditionText }); + } + + continue; + } + + out.push({ text: rule.cssText, condition: null }); + } + } + + return out; +} + +/** + * The two px numbers of a `*-block` declaration, as [start, end]. + * + * A symmetric pair is **serialised back as one value** — `padding-block: + * 16px 16px` reads as `padding-block: 16px` — so a naive pair-reader + * fails on the shorthand rather than on the thing it is checking, and + * says the wrong thing about why. That is not hypothetical: it is what + * the symmetric-padding reversion did while this test was being + * proved. + */ +function blockPair(text: string, property: string): [number, number] | null { + const declaration = new RegExp(`${property}:\\s*([^;]+)`).exec(text)?.[1]; + + if (declaration === undefined) { + return null; + } + + const values = [...declaration.matchAll(/(-?[\d.]+)px/g)].map((m) => + Number(m[1]), + ); + + const [start, end] = values; + + if (start === undefined) { + return null; + } + + return [start, end ?? start]; +} + +describe("the seek bar's phone rules", () => { + it('are last, so they are not silently overridden', async () => { + const el = await fixture('seek-bar', {}); + const rules = rulesOf(el); + + // A sweep that read nothing passes vacuously — the same first + // assertion icon-language.test.ts makes, for the same reason. + expect(rules.length).toBeGreaterThan(0); + + const declaresTrackSize = (r: Rule) => /--track-size:/.test(r.text); + + const lastUnconditional = rules.findLastIndex( + (r) => r.condition === null && declaresTrackSize(r), + ); + const phoneOverride = rules.findLastIndex( + (r) => r.condition !== null && PHONE_QUERY.test(r.condition) + && declaresTrackSize(r), + ); + + expect(lastUnconditional).toBeGreaterThanOrEqual(0); + expect(phoneOverride).toBeGreaterThanOrEqual(0); + + // A media query adds no specificity. Written first, it loses. + expect(phoneOverride).toBeGreaterThan(lastUnconditional); + }); + + it('give the slider a pointer target of at least the touch floor', async () => { + const el = await fixture('seek-bar', {}); + const rules = rulesOf(el); + + const track = rules.find( + (r) => r.condition !== null && PHONE_QUERY.test(r.condition) + && /--track-size:/.test(r.text), + ); + const target = rules.find( + (r) => r.condition !== null && PHONE_QUERY.test(r.condition) + && r.text.includes('::part(slider)'), + ); + + expect(track).toBeDefined(); + expect(target).toBeDefined(); + + const trackSize = Number( + /--track-size:\s*(-?[\d.]+)px/.exec(track!.text)?.[1], + ); + const padding = blockPair(target!.text, 'padding-block'); + + expect(padding).not.toBeNull(); + + // The padding is on ::part(slider) rather than on the host because + // that inner div is what carries the gesture: it has the listener + // and the touch-action, and it is exactly the host's size, so + // padding the host grows a box that does not take the press. + const hitArea = trackSize + padding![0] + padding![1]; + + expect(hitArea).toBeGreaterThanOrEqual(TOUCH_FLOOR); + }); + + it('do not grow the row they sit in', async () => { + const el = await fixture('seek-bar', {}); + + const target = rulesOf(el).find( + (r) => r.condition !== null && PHONE_QUERY.test(r.condition) + && r.text.includes('::part(slider)'), + ); + + expect(target).toBeDefined(); + + const padding = blockPair(target!.text, 'padding-block'); + const margin = blockPair(target!.text, 'margin-block'); + + expect(padding).not.toBeNull(); + expect(margin).not.toBeNull(); + + // now-playing-view's vertical budget is fixed and #51 measured + // every pixel of it: letting the row grow by the difference cost + // the album art 25px of 143 when it was tried on the device. + expect(margin![0]).toBe(-padding![0]); + expect(margin![1]).toBe(-padding![1]); + }); + + it('take the space above, because what is below is the transport', async () => { + const el = await fixture('seek-bar', {}); + + const target = rulesOf(el).find( + (r) => r.condition !== null && PHONE_QUERY.test(r.condition) + && r.text.includes('::part(slider)'), + ); + + expect(target).toBeDefined(); + + const pair = blockPair(target!.text, 'padding-block'); + + expect(pair).not.toBeNull(); + + const [above, below] = pair!; + + // Measured at 424x439: the seek row is 19px and the play button's + // top edge is 8px below it, while `.art` above is a non-interactive + // div. A symmetric target would reach into the play button — the + // most important control on the screen — so the growth is upward. + expect(above).toBeGreaterThan(below); + }); +});