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); + }); +});