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.
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -99,6 +99,25 @@ describe('<search-trigger>', () => {
|
||||
}
|
||||
});
|
||||
|
||||
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<HTMLButtonElement>(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);
|
||||
|
||||
|
||||
@@ -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<HTMLElement>(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<PageHeader>('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<PageHeader>('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<HTMLButtonElement>(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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user