diff --git a/e2e/specs/layout-overflow.spec.ts b/e2e/specs/layout-overflow.spec.ts index c5b6401..eaa1953 100644 --- a/e2e/specs/layout-overflow.spec.ts +++ b/e2e/specs/layout-overflow.spec.ts @@ -33,6 +33,14 @@ const VIEWPORTS = [ // was missing its own worst case. { name: '900×600 (the widest sidebar, so the narrowest content)', width: 900, height: 600 }, { name: `the minimum (${MIN_VIEWPORT.width}×${MIN_VIEWPORT.height})`, ...MIN_VIEWPORT }, + // Below the enforced minimum on purpose, and for the reason 700×480 + // is below it further down: a scaled display or a large system font + // lands the layout here without the window ever being dragged there, + // and 600 is the last width before the phone layout takes over. The + // *narrowest header* is a different question from the narrowest + // content area and has a different answer — this one (#143), where + // the bar was 611px inside 600 sitting still. + { name: '600×600 (the bottom of the Compact band)', width: 600, height: 600 }, ]; /** @@ -145,6 +153,12 @@ test.describe('the app fits in its own window', () => { test.describe('the title block fits its bar', () => { test('the hgroup stays inside the 4em top bar', async ({ app }) => { + // Stated rather than inherited from whatever ran last. Since #143 + // the wordmark is visually hidden at widths where the bar cannot + // afford it, so a test about its *vertical* fit has to say which + // width it is asking about. + await app.setViewportSize({ width: 1440, height: 900 }); + // The state a11y.29 landed in. The pair is flex-centred and a UA // gives an `h1` a 0.67em top margin, so the block measured 67px // inside 64 — pre-existing, and invisible until dropping the h3's @@ -246,16 +260,26 @@ test.describe('the shell reflows rather than hiding what does not fit', () => { test(`no scrollbar appears at ${vp.name}`, async ({ app }) => { await app.setViewportSize({ width: vp.width, height: vp.height }); - const excess = await app.evaluate(() => { - const de = document.documentElement; + // Polled, for the reason the track-row test above is: since #143 + // the top bar's fit is *measured* — a ResizeObserver decides what + // it can afford at this width — so a single read taken straight + // after the resize races the observer and reports the frame + // before it. Read once, this passed alone and failed in the full + // suite, which is the shape of a timing assumption rather than of + // a defect. + // + // The other half of the assertion: at every size this app + // promises, the fix costs nothing. A scrollbar that is always + // there is a worse answer than the clipping it replaced. + await expect + .poll(() => + app.evaluate(() => { + const de = document.documentElement; - return de.scrollWidth - de.clientWidth; - }); - - // The other half: at every size this app promises, the fix costs - // nothing. A scrollbar that is always there is a worse answer - // than the clipping it replaced. - expect(excess).toBe(0); + return de.scrollWidth - de.clientWidth; + }), + ) + .toBe(0); }); } }); diff --git a/e2e/specs/top-bar-fit.spec.ts b/e2e/specs/top-bar-fit.spec.ts new file mode 100644 index 0000000..747fdd4 --- /dev/null +++ b/e2e/specs/top-bar-fit.spec.ts @@ -0,0 +1,207 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * The top bar fits the window it is in (#143). + * + * **This is measured per child, not on the shell**, which is #69's + * lesson repeated one component over: `layout-overflow.spec.ts` asserts + * the *document* needs no sideways scrolling, and clipping inside a + * component is invisible to it — which is exactly why that spec was + * green throughout this defect. What a user sees is a control rendered + * past the edge of the bar it belongs to, so that is what is asserted. + * + * **And it is measured with a job running**, which is the half the + * original report missed. `job-indicator` is `hidden` while idle and up + * to 235px wide when it is not, so the bar was 611px inside 600 sitting + * still and 862px during a scan — 171 to 262px of overflow, arriving + * exactly when a user has reason to look at that bar. Nothing else in + * this suite has ever measured a layout with work in flight; + * `/__test/emit` stages it without staging the scan. + */ +type Page = import('@playwright/test').Page; + +/** + * The widths this asks about. + * + * 600 is the bottom of the Compact band (#24) and where the defect + * lands; 899 and 900 straddle `nav-history` appearing (68px more to + * find, at the width that just gained the sidebar's labels); 800 is the + * enforced minimum; 390 is a phone, where the answer must be that + * nothing collapses because the media queries already did the work. + */ +const WIDTHS = [390, 600, 800, 899, 900, 1440]; + +/** + * A scan whose title is as long as a real one gets. The label is capped + * at 12rem by the component, so this is the widest the indicator can + * be — measuring with "Scanning" instead reports a bar that fits and a + * defect that is 100px smaller than it is. + */ +const LONG_JOB = { + id: 'top-bar-fit', + kind: 'library-scan', + state: 'running', + title: 'Scanning Music from the external drive', + current: 40, + total: 100, +}; + +/** + * Every child's right edge against the bar's own content box. + * + * The content box, not `clientWidth`: the bar has a 2em right gutter, + * and a control sitting in the padding is already the failure — it is + * simply one that `scrollWidth` under-reports, because `scrollWidth` + * counts the left padding and not the right. + */ +const overflowingChildren = (page: Page) => + page.evaluate(() => { + const bar = document.querySelector('header.top-bar')!; + const style = getComputedStyle(bar); + const box = bar.getBoundingClientRect(); + const left = box.left + parseFloat(style.paddingLeft); + const right = box.right - parseFloat(style.paddingRight); + + return [...bar.children] + .filter((child) => { + const cs = getComputedStyle(child); + + // Out of flow is out of the question: a collapsed wordmark is + // `position: absolute` and 1px wide precisely so it costs the + // row nothing. + if (cs.display === 'none' || cs.position === 'absolute') return false; + + const r = child.getBoundingClientRect(); + + return r.width > 0 && (r.right > right + 0.5 || r.left < left - 0.5); + }) + .map((child) => { + const r = child.getBoundingClientRect(); + + return `${child.tagName.toLowerCase()}: ${Math.round(r.left)}..${Math.round(r.right)} outside ${Math.round(left)}..${Math.round(right)}`; + }); + }); + +/** What the fit pass gave up, read back off the DOM it changed. */ +const collapsed = (page: Page) => + page.evaluate(() => ({ + wordmark: !!document.querySelector('header.top-bar hgroup.yj-collapsed'), + jobLabel: !!document.querySelector('job-indicator[compact]'), + })); + +test.describe('the top bar fits the window', () => { + for (const width of WIDTHS) { + test(`no control sits outside the bar at ${width}px, idle`, async ({ + app, + }) => { + await app.setViewportSize({ width, height: 600 }); + + await expect.poll(() => overflowingChildren(app)).toEqual([]); + }); + + test(`no control sits outside the bar at ${width}px, with a job running`, async ({ + app, + testctl, + }) => { + await app.setViewportSize({ width, height: 600 }); + await testctl.emit('JobsChanged', [LONG_JOB]); + + // The indicator has to actually be up, or this test passes by + // measuring the idle case under another name. + await expect(app.locator('job-indicator')).toBeVisible(); + + await expect.poll(() => overflowingChildren(app)).toEqual([]); + }); + } + + /** + * The other half of "measured, never breakpointed": a rule that + * collapses defensively at every narrow width fits just as well and + * is a worse app. 1440 is roomy at any job title; 899 was measured to + * fit with the longest one, because `nav-history` is not there yet. + */ + test('nothing is given up where there is room for it', async ({ + app, + testctl, + }) => { + await app.setViewportSize({ width: 1440, height: 900 }); + await testctl.emit('JobsChanged', [LONG_JOB]); + await expect(app.locator('job-indicator')).toBeVisible(); + + await expect.poll(() => collapsed(app)).toEqual({ + wordmark: false, + jobLabel: false, + }); + }); + + /** + * And it gives them back. The pass starts from all-visible every + * time, so this is the property that a rule which only ever *added* + * to the collapsed set would fail — the wordmark would be gone for + * the rest of the session after one narrow moment. + */ + test('the wordmark comes back when the window does', async ({ + app, + testctl, + }) => { + await testctl.emit('JobsChanged', [LONG_JOB]); + await app.setViewportSize({ width: 600, height: 600 }); + + await expect.poll(() => collapsed(app)).toEqual({ + wordmark: true, + jobLabel: true, + }); + + await app.setViewportSize({ width: 1440, height: 900 }); + + await expect.poll(() => collapsed(app)).toEqual({ + wordmark: false, + jobLabel: false, + }); + }); + + /** + * The wordmark yields its width and not its existence: `display: + * none` would take the document from one top-level heading to none. + */ + test('the collapsed wordmark is still the document heading', async ({ + app, + testctl, + }) => { + await testctl.emit('JobsChanged', [LONG_JOB]); + await app.setViewportSize({ width: 600, height: 600 }); + + await expect.poll(() => collapsed(app)).toMatchObject({ wordmark: true }); + + await expect( + app.getByRole('heading', { name: 'YellowJacket', level: 1 }), + ).toHaveCount(1); + }); + + /** + * And the indicator keeps saying what it is doing after its visible + * label goes — the `sr-only` live region is what announces the state, + * which is the same argument the phone's own rule was written on. + */ + test('the job indicator still announces its state without its label', async ({ + app, + testctl, + }) => { + await testctl.emit('JobsChanged', [LONG_JOB]); + await app.setViewportSize({ width: 600, height: 600 }); + + await expect.poll(() => collapsed(app)).toMatchObject({ jobLabel: true }); + + const spoken = await app + .locator('job-indicator') + .evaluate( + (el) => + el.shadowRoot?.querySelector('[aria-live]')?.textContent?.trim() ?? '', + ); + + expect(spoken).toContain('Scanning Music from the external drive'); + + // Leave the app as the next spec expects to find it. + await app.setViewportSize({ width: 1440, height: 900 }); + }); +}); diff --git a/frontend/index.css b/frontend/index.css index 44e6a47..4431617 100644 --- a/frontend/index.css +++ b/frontend/index.css @@ -104,6 +104,42 @@ p { flex: 0 1 320px; } +/* What the bar gives up when it does not fit is decided by measuring + it (`services/top-bar-fit.ts`, #143). Two rules here are what make + that measurement mean anything. + + **Nothing but the search box may shrink.** `scrollWidth` reports a + perfect fit while a child quietly truncates -- #69's trap, one + component over -- and the indicator's label is `text-overflow: + ellipsis`, so it would have absorbed the deficit and hidden it. The + search box is exempt because it shrinks between its 320px basis and + the 200px floor its own stylesheet sets, and a narrower input hides + nothing it was showing. */ +.top-bar hgroup, +.top-bar library-filter, +.top-bar job-indicator { + flex-shrink: 0; +} + +/* **The wordmark yields its width, not its existence.** It is the + app's top-level heading as well as its brand, and `display: none` + would take a document from one `h1` to none at exactly the widths + where the view's own header is the only thing left saying where you + are. This is `styles/sr-only.css.ts`'s recipe, written out because + that one is a `CSSResult` for shadow roots and this is the light + DOM. */ +.top-bar hgroup.yj-collapsed { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip-path: inset(50%); + white-space: nowrap; + border: 0; +} + /* The bar is `justify-content: space-between`, which with four children spreads them evenly and left back/forward floating in the middle of nothing. Collecting the free space *after* this one puts the pair diff --git a/frontend/index.ts b/frontend/index.ts index 3ebfa2b..61602b2 100644 --- a/frontend/index.ts +++ b/frontend/index.ts @@ -54,6 +54,7 @@ import '@store/theme-store'; import './src/services/keyboard-shortcut-service'; import { activateView, deactivateView } from '@utils/view-lifecycle'; import { installLongPressContextMenu } from '@utils/long-press'; +import { installTopBarFit } from './src/services/top-bar-fit'; import { hasTrackPayload, getDragPayload, @@ -73,6 +74,14 @@ registerBundledIcons(); // on `pointerType === 'touch'` only. installLongPressContextMenu(); +// The top bar decides what it can afford to show (#143). Here rather +// than in a component because the bar is light DOM in index.html and +// its children are five separate elements; the shell is the only thing +// that can see all five at once. +const topBar = document.querySelector('header.top-bar'); + +if (topBar) installTopBarFit(topBar); + // --------------------------------------------------------------------------- // View caching navigation system // --------------------------------------------------------------------------- diff --git a/frontend/src/components/jobs/job-indicator.ts b/frontend/src/components/jobs/job-indicator.ts index 0791b04..5d95403 100644 --- a/frontend/src/components/jobs/job-indicator.ts +++ b/frontend/src/components/jobs/job-indicator.ts @@ -155,13 +155,27 @@ export class JobIndicator extends LitElement { goes -- the live region in render() is what announces this, and it is unaffected, so the ring keeps its accessible name and screen readers keep hearing the - state change. */ + state change. + + [compact] is the same removal asked for by measurement + rather than by width, and it is set from outside: the + shell's fit pass (services/top-bar-fit.ts, #143) owns + it, because between 600 and 900 whether this label fits + depends on what else is in the bar and on how long the + running job's title is -- 235px for "Scanning Music from + the external drive" -- rather than on the viewport. Two + triggers, one effect, and the phone's is unconditional + because it was argued and pinned before this existed. */ @media (max-width: 599px) { .label { display: none; } } + :host([compact]) .label { + display: none; + } + .alert-dot { width: 6px; height: 6px; diff --git a/frontend/src/services/top-bar-fit.ts b/frontend/src/services/top-bar-fit.ts new file mode 100644 index 0000000..d253b46 --- /dev/null +++ b/frontend/src/services/top-bar-fit.ts @@ -0,0 +1,199 @@ +/** + * What the top bar drops when it runs out of room (#143). + * + * The bar holds five children — the wordmark, back/forward, the library + * filter, the search box and the job indicator — and at the bottom of + * the Compact band they do not all fit. Measured on `main` at 600×600: + * the bar is 611px inside a 600px viewport sitting still, and **862px + * while a scan with a long title is running**, because `job-indicator` + * is `hidden` when idle and up to 235px wide when it is not. `body` is + * `overflow-x: auto`, so what a user sees is a horizontal scrollbar on + * a shell that #24 promised would not need one. + * + * **The fit is measured, never breakpointed**, which is `page-header`'s + * rule (#69) and applies here for a reason specific to this bar: three + * of its five children are as wide as their *content*. The library + * filter is a `