From 6a5a3c33dc8c4643a2e0edde67154d09635fd728 Mon Sep 17 00:00:00 2001 From: Logan Date: Fri, 21 Aug 2026 19:45:20 -0400 Subject: [PATCH 1/2] fix(shell): raise the page header's controls to the touch floor #56 sized the playback transport for a thumb and named 44px; the queue header keeps it. Nothing else was resized, so the controls a user meets on *every* screen sat between a third and two thirds of the app's own floor. Measured on the reference device at 424x439: page-sort 99x23, page-sort-direction **28x21**, page-actions-more 38x27, and search-trigger 40x40. **Both questions the issue left open are answered by one measurement.** The header is 63px tall and its controls are 20-23px, so the vertical room was already there; the select and its direction arrow are 6px apart, so the horizontal room was not. That makes this min-size rather than padding with a negative margin, which is what the seek bar needed (#187), and the difference decides everything else. There the painted track had to stay thin, so the target was grown past its own box and had to be checked against its neighbours. Here the control *is* the target: the boxes are flex items, so the gap keeps them apart and **no two targets can overlap by construction**. From which: **There is no phone branch.** A 44px control on a desktop is merely large, and a second declaration of what a phone shows is a second thing to keep in step -- which is why this component has never had one. It also avoids a media query no tier here renders, which is exactly how the seek bar's phone rule came to be dead for months. **#69's overflow fit does not move.** That pass measures inline size, so the height costs it nothing, and only the two square controls grow the header's content -- by 22px in total. header-action-overflow.spec.ts passes unchanged at all four of its widths, which was the check rather than the assumption. Verified on the device that the count is still shown at 424px, so nothing has started yielding. search-trigger is the sharpest case and is fixed in the same pass: #57 created it as the phone's replacement for the header search box, so it exists *only* where there is a thumb, and it shipped at 40x40 under a comment calling that "the smallest a touch target should be". That was the floor restated four pixels short rather than a second opinion about it, and the comment now says so. Unlike #187 this can be measured rather than inferred: the controls are plain elements and the rule is a min-size, so it holds at every width and a real Chromium rendering a real page-header gives the actual answer. The tests fail with the device's own numbers -- 29x21, 38, 40. Verified on the device: every control in the header is now at least 44x44, and so is the phone's search button. **This is the Direction's first step, not all of it.** config-field's 93 Settings controls and explore-view's search row are the second pass; Settings is a form with one shape for every row and wants its own argument. #186 stays open for them. --- .../src/components/page-header/page-header.ts | 38 ++++++ .../search-dialog/search-trigger.ts | 20 ++- .../test/components/search-dialog.test.ts | 19 +++ .../test/components/touch-targets.test.ts | 122 ++++++++++++++++++ 4 files changed, 194 insertions(+), 5 deletions(-) create mode 100644 frontend/test/components/touch-targets.test.ts diff --git a/frontend/src/components/page-header/page-header.ts b/frontend/src/components/page-header/page-header.ts index 48bba04..723ad46 100644 --- a/frontend/src/components/page-header/page-header.ts +++ b/frontend/src/components/page-header/page-header.ts @@ -298,6 +298,32 @@ export class PageHeader extends LitElement { flex-shrink: 0; } + /* Every control in this header meets the app's 44px touch + floor -- the number #56 set for the transport and the + queue header already keeps (#186). + + It is min-size rather than padding with a negative + margin, which is what the seek bar needed (#187), and + the difference is worth stating because it decides + whether targets can collide. There the painted track had + to stay thin, so the target was grown past its own box + and had to be checked against its neighbours. Here the + control *is* the target: the boxes are flex items, so + the gap keeps them apart and no two can overlap by + construction. + + There is no phone branch. With the target being the box, + a 44px control on a desktop is merely large, and a + second declaration of what a phone shows is a second + thing to keep in step -- which is the reason this + component has never had one. It also avoids a media + query that no tier here renders, which is exactly how + the seek bar's phone rule came to be dead for months. + + Only the *width* of this reaches the overflow fit below: + that pass measures inline size, so the height costs it + nothing, and the two square controls grow the header's + content by 22px in total. */ .sort select { font: inherit; color: inherit; @@ -306,6 +332,7 @@ export class PageHeader extends LitElement { border-radius: 4px; padding: 3px 6px; cursor: pointer; + min-block-size: 44px; } .sort-dir { @@ -318,6 +345,11 @@ export class PageHeader extends LitElement { color: inherit; cursor: pointer; padding: 3px 5px; + /* 28x21 before this, the smallest control in the + header and the only one that failed the floor in + both directions. */ + min-inline-size: 44px; + min-block-size: 44px; } .sort-dir:hover { @@ -377,10 +409,16 @@ export class PageHeader extends LitElement { gap: 6px; white-space: nowrap; flex-shrink: 0; + justify-content: center; + min-block-size: 44px; } .more-button { padding: 6px 10px; + /* 38x27, and it is the route to every collapsed + action, so it is the last control that should be + hard to hit. */ + min-inline-size: 44px; } /* The display: flex above outranks the UA stylesheet's diff --git a/frontend/src/components/search-dialog/search-trigger.ts b/frontend/src/components/search-dialog/search-trigger.ts index 16d0b71..d3e8efe 100644 --- a/frontend/src/components/search-dialog/search-trigger.ts +++ b/frontend/src/components/search-dialog/search-trigger.ts @@ -61,11 +61,21 @@ export class SearchTrigger extends LitElement { display: inline-flex; align-items: center; justify-content: center; - /* The smallest a touch target should be. The header's - own action buttons are smaller because they carry a - label; this one is a glyph. */ - min-width: 40px; - min-height: 40px; + /* The app's touch floor, from #56 -- and this is the + control that should least have to argue for it: #57 + created it as the phone's replacement for the header + search box, so it exists *only* where there is a + thumb. + + It shipped at 40px under a comment calling that "the + smallest a touch target should be", which was the + floor being restated four pixels short rather than a + second opinion about it (#186). The rest of that + comment said the header's own action buttons are + smaller because they carry a label; they are 44px + now too, so that no longer distinguishes anything. */ + min-width: 44px; + min-height: 44px; padding: 0; background: none; border: 1px solid var(--yj-border-subtle, #555); diff --git a/frontend/test/components/search-dialog.test.ts b/frontend/test/components/search-dialog.test.ts index 03f83de..96193b2 100644 --- a/frontend/test/components/search-dialog.test.ts +++ b/frontend/test/components/search-dialog.test.ts @@ -99,6 +99,25 @@ describe('', () => { } }); + it('meets the touch floor it was shipped four pixels under', async () => { + stubPhone(true); + + // #57 created this as the phone's replacement for the header search + // box, so it exists *only* where there is a thumb -- and it shipped + // at 40x40 under a comment calling that "the smallest a touch + // target should be", which was the app's own 44px floor (#56) + // restated short rather than a second opinion about it. #186. + const el = await fixture('search-trigger'); + const button = shadow(el, '[data-testid="search-trigger"]'); + + expect(button).not.toBeNull(); + + const box = button!.getBoundingClientRect(); + + expect(Math.round(box.width)).toBeGreaterThanOrEqual(44); + expect(Math.round(box.height)).toBeGreaterThanOrEqual(44); + }); + it('names what the button will search', async () => { stubPhone(true); diff --git a/frontend/test/components/touch-targets.test.ts b/frontend/test/components/touch-targets.test.ts new file mode 100644 index 0000000..f5c459f --- /dev/null +++ b/frontend/test/components/touch-targets.test.ts @@ -0,0 +1,122 @@ +/** + * Every control a finger meets is at least 44px (#186). + * + * #56 sized the playback transport for a thumb and named 44px; the + * queue header keeps it; nothing else was resized. So the controls a + * user meets on *every* screen — the sort control, its direction + * button, the page actions, the overflow trigger and the phone's search + * button — sat between a third and two thirds of the app's own floor. + * Measured on the reference device (TLP301, 424x439): `page-sort` 99x23, + * `page-sort-direction` **28x21**, `page-actions-more` 38x27, + * `search-trigger` 40x40. + * + * Unlike the seek bar's target (#187), this one can be measured here + * rather than inferred from the stylesheet. There the painted track had + * to stay thin, so the hit area was grown past its own box and only a + * phone-width layout of a third-party slider could show it. Here the + * control *is* the target, so a real Chromium rendering a real + * `page-header` gives the actual answer — and because it is a `min-size` + * rather than a media query, the answer is the same at every width, + * which is what makes it checkable in this tier at all. + * + * That is also why there is no phone branch to test: a 44px control on + * a desktop is merely large, and a second declaration of what a phone + * shows is a second thing to keep in step. + */ +import { describe, expect, it } from 'vitest'; +import type { PageAction, PageHeader } from '@components/page-header/page-header'; + +import '@components/page-header/page-header'; +import { fixture, shadowAll } from '@test/support/render'; + +/** The app's touch floor, from #56. */ +const FLOOR = 44; + +const SORTS = [ + { id: 'name', label: 'Name' }, + { id: 'tracks', label: 'Tracks' }, +]; + +function actions(): PageAction[] { + return [ + { id: 'import', label: 'Import', icon: 'file-import', priority: 0, onSelect: () => {} }, + { id: 'new', label: 'New Playlist', icon: 'plus', priority: 2, onSelect: () => {} }, + ]; +} + +/** Every visible control in the header's own shadow root. */ +function controlsOf(el: PageHeader): { name: string; el: HTMLElement }[] { + return shadowAll(el, 'button, select') + .filter((c) => !(c as HTMLButtonElement).hidden) + .map((c) => ({ + name: c.dataset.testid ?? (c.className || c.tagName.toLowerCase()), + el: c, + })); +} + +function tooSmall(controls: { name: string; el: HTMLElement }[]): string[] { + return controls + .map(({ name, el }) => { + const b = el.getBoundingClientRect(); + + return { name, w: Math.round(b.width), h: Math.round(b.height) }; + }) + .filter((c) => c.w < FLOOR || c.h < FLOOR) + .map((c) => `${c.name} ${c.w}x${c.h}`); +} + +describe("the page header's controls", () => { + it('all meet the touch floor', async () => { + const el = await fixture('page-header', { + heading: 'Playlists', + count: 50, + countNoun: 'playlist', + sortOptions: SORTS, + sortField: 'name', + sortDirection: 'asc', + actions: actions(), + }); + + const controls = controlsOf(el); + + // A sweep that found no controls passes vacuously — the same first + // assertion icon-language.test.ts makes, for the same reason. + expect(controls.length).toBeGreaterThan(0); + + // The two that were smallest, named so a regression says which. + expect(controls.map((c) => c.name)).toContain('page-sort-direction'); + expect(controls.map((c) => c.name)).toContain('page-sort'); + + expect(tooSmall(controls)).toEqual([]); + }); + + it('includes the overflow trigger, which is the route to the rest', async () => { + // At 320px the fit pass collapses actions into the menu, so the + // trigger is rendered — and it is then the only way to reach them, + // which makes it the last control that should be hard to hit. + const el = await fixture('page-header', { + heading: 'Playlists', + sortOptions: SORTS, + sortField: 'name', + actions: actions(), + }); + + el.style.width = '320px'; + + for (let frame = 0; frame < 3; frame += 1) { + await new Promise((r) => requestAnimationFrame(r)); + await el.updateComplete; + } + + const more = shadowAll(el, '.more-button').filter( + (b) => !b.hidden, + ); + + expect(more.length).toBe(1); + + const box = more[0]!.getBoundingClientRect(); + + expect(Math.round(box.width)).toBeGreaterThanOrEqual(FLOOR); + expect(Math.round(box.height)).toBeGreaterThanOrEqual(FLOOR); + }); +}); From 0d331666d68bf0119d747f22e2fef4e08930ba78 Mon Sep 17 00:00:00 2001 From: Logan Date: Fri, 21 Aug 2026 20:12:51 -0400 Subject: [PATCH 2/2] fix(shell): make the header's touch targets cost no width The first pass grew the two square controls to 44px as boxes, which added 22px to the header. That fit at every width Chromium was checked at and **clipped the overflow trigger at 320x600 in WebKit** -- the engine closest to what ships, and the one no machine here can run: every action is reachable at 320x600 (400% zoom) - Array [] + Array [ "more" ] Two things were wrong, and only one of them was the code. **The claim was checked on one engine and stated as a property.** The previous commit said #69's fit "does not move ... the check rather than the assumption", on the strength of running that spec against chromium alone. CI runs both browsers precisely because they are not the same answer. **And the box was the wrong thing to grow**, which the issue already said: "reached by growing the *hit* area rather than the visual weight where the two can differ -- padding on the control, not size on the icon". #69's pass measures inline size, so a taller control is free and a wider one is not. So height stays a box -- the header has the room and nothing measures it -- and width is padding with a negative margin handing the space back, which is the seek bar's shape from #187. Measured in the component tier at 320px: the arrow's rect is 45x44 and it occupies 29, the overflow trigger 44x44 occupying 38, the search button 44x44 occupying 40. Those three occupancies are what they were before any of this, so the fit pass sees a header identical to main's and the 320px case cannot regress. The arrow's target is lopsided for #187's reason: the select is 6px to its left and there is open space to its right, so it takes the side with nothing to steal from. The overflow trigger's can be symmetric, the actions row having an 8px gap. `search-trigger` is border-box, so its 44px min-width is the whole target and the margin alone gives the four pixels back. The new assertion is the one that would have caught this: every grown control must carry negative inline margins, because that is what keeps the box out of the fit. The rect assertions stay -- getBoundingClientRect includes padding, so the target is still measured directly rather than inferred. --- .../src/components/page-header/page-header.ts | 42 +++++++++++++++---- .../search-dialog/search-trigger.ts | 13 +++++- .../test/components/touch-targets.test.ts | 40 ++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/frontend/src/components/page-header/page-header.ts b/frontend/src/components/page-header/page-header.ts index 723ad46..29d064a 100644 --- a/frontend/src/components/page-header/page-header.ts +++ b/frontend/src/components/page-header/page-header.ts @@ -320,10 +320,25 @@ export class PageHeader extends LitElement { query that no tier here renders, which is exactly how the seek bar's phone rule came to be dead for months. - Only the *width* of this reaches the overflow fit below: - that pass measures inline size, so the height costs it - nothing, and the two square controls grow the header's - content by 22px in total. */ + **The height is the box and the width is not**, and that + asymmetry is the whole of what the overflow fit below + cares about. That pass measures inline size, so a taller + control costs it nothing and a wider one costs it + directly. Growing the two square controls to 44px wide + added 22px, which fits at every width Chromium was + checked at and clipped the overflow trigger at 320px in + **WebKit** -- the engine closest to what actually ships, + and the one no machine here can run. So the horizontal + half is padding with the margin cancelling it, which is + what the issue asked for in the first place: the target + grows and the layout does not. + + The cost is that a horizontal target can now overlap a + neighbour, which the box version could not. The arrow's + is deliberately lopsided for the seek bar's reason + (#187): the select is 6px to its left and there is open + space to its right, so it takes the side with nothing to + steal from. */ .sort select { font: inherit; color: inherit; @@ -347,9 +362,16 @@ export class PageHeader extends LitElement { padding: 3px 5px; /* 28x21 before this, the smallest control in the header and the only one that failed the floor in - both directions. */ - min-inline-size: 44px; + both directions. + + Vertically the box grows, because the header has the + room and nothing measures it. Horizontally the box + must not: 28 + 2 + 14 is a 44px target over a 28px + layout box, weighted right because the select is 6px + to the left. */ min-block-size: 44px; + padding-inline: 5px 21px; + margin-inline: 0 -16px; } .sort-dir:hover { @@ -417,8 +439,12 @@ export class PageHeader extends LitElement { padding: 6px 10px; /* 38x27, and it is the route to every collapsed action, so it is the last control that should be - hard to hit. */ - min-inline-size: 44px; + hard to hit -- and the one WebKit clipped at 320px + when this was 6px wider as a box. 38 + 3 + 3 is a + 44px target over a 38px layout box; the actions row + has an 8px gap, so this one can be symmetric. */ + padding-inline: 13px; + margin-inline: -3px; } /* The display: flex above outranks the UA stylesheet's diff --git a/frontend/src/components/search-dialog/search-trigger.ts b/frontend/src/components/search-dialog/search-trigger.ts index d3e8efe..45e9765 100644 --- a/frontend/src/components/search-dialog/search-trigger.ts +++ b/frontend/src/components/search-dialog/search-trigger.ts @@ -73,9 +73,20 @@ export class SearchTrigger extends LitElement { second opinion about it (#186). The rest of that comment said the header's own action buttons are smaller because they carry a label; they are 44px - now too, so that no longer distinguishes anything. */ + now too, so that no longer distinguishes anything. + + The extra width is a target rather than a box, for + page-header's reason: this button sits in that + header, whose overflow fit (#69) measures inline + size, and four pixels there is four pixels the + trigger for every collapsed action does not get at + 320px. Height is free -- nothing measures it. */ min-width: 44px; min-height: 44px; + /* Border-box, so the 44 above is the whole target and + the margin is what hands the four extra pixels back + to the row. */ + margin-inline: -2px; padding: 0; background: none; border: 1px solid var(--yj-border-subtle, #555); diff --git a/frontend/test/components/touch-targets.test.ts b/frontend/test/components/touch-targets.test.ts index f5c459f..e6c0912 100644 --- a/frontend/test/components/touch-targets.test.ts +++ b/frontend/test/components/touch-targets.test.ts @@ -90,6 +90,46 @@ describe("the page header's controls", () => { expect(tooSmall(controls)).toEqual([]); }); + it('grows the target without growing the box, so the overflow fit is untouched', async () => { + // The regression this exists for, and it was a real one: growing + // the two square controls to 44px *wide* added 22px to the header, + // which fit at every width Chromium was checked at and clipped the + // overflow trigger at 320x600 in WebKit -- the engine closest to + // what ships, and the one no machine here can run. #69's fit pass + // measures inline size, so a taller control is free and a wider one + // is not. + // + // Negative inline margins are what keep the box out of it: the + // padding makes the target, and the margin gives the space back. + const el = await fixture('page-header', { + heading: 'Playlists', + sortOptions: SORTS, + sortField: 'name', + actions: actions(), + }); + + el.style.width = '320px'; + + for (let frame = 0; frame < 3; frame += 1) { + await new Promise((r) => requestAnimationFrame(r)); + await el.updateComplete; + } + + for (const selector of ['.sort-dir', '.more-button']) { + const control = shadowAll(el, selector).filter( + (c) => !(c as HTMLButtonElement).hidden, + )[0]; + + expect(control, selector).toBeTruthy(); + + const style = getComputedStyle(control!); + const added = + parseFloat(style.marginInlineStart) + parseFloat(style.marginInlineEnd); + + expect(added, `${selector} gives its extra width back`).toBeLessThan(0); + } + }); + it('includes the overflow trigger, which is the route to the rest', async () => { // At 320px the fit pass collapses actions into the menu, so the // trigger is rendered — and it is then the only way to reach them,