yonlu/yellowjacket
1
1
Fork 0
Code Issues 55 Pull Requests 6 Actions Packages Projects Releases 12 Wiki Activity

feat(shell): take the top bar out of the phone's layout

Browse Source 
The row is deleted from the grid template below 600px, not the header
hidden. That is 3.25em of a 439 CSS px viewport -- the single biggest
vertical win the reference device has to give, and the reason the issue
asks for the row rather than for a smaller bar.

Each of the five things the bar held has somewhere else to be there:
nav-history is the platform's own back gesture and was already gone from
899 down, the job indicator is <job-band> (#62, which is what this was
blocked on), the search box is a modal opened from the view's own
header, the library filter is Settings -> Libraries, and the wordmark
stays where it is.

Three things are load-bearing.

**The header is visually hidden rather than display: none**, because
that h1 is the document's top-level heading and several pages have no
other one -- page-header renders no h1 when its heading is empty, and
Settings has no page-header at all. Its four controls are display: none
*inside* it, which is what keeps them out of the tab order: a
visually-hidden container is still focusable, and tabbing into a search
box nobody can see is worse than not having one.

**The fit pass stands down**, from the bar's computed position rather
than from a width. With the bar out of flow there is no content box to
measure children against, and a pass that ran would collapse the
wordmark on every resize and report success about a 1px box.

**top-bar-fit.spec.ts keeps 390 and asserts the stronger property.**
"Nothing hangs out of the bar" is trivially true of a bar with no row
and would pass on a build that merely broke it, so what that width asks
now is that the content starts where the row above it ends. Measuring
against the window instead would have been asserting "and no background
job is running", which that spec is not about and cannot arrange.

Closes #57
This commit is contained in:
logan
2026-08-20 20:03:20 -04:00
parent ac8f86eb00
commit 1ab767a317
4 changed files with 451 additions and 64 deletions
Download Patch File Download Diff File Expand all files Collapse all files
e2e/specs/phone-search.spec.ts
+322
View File
@@ -0,0 +1,322 @@
import { test, expect } from '../support/fixtures.js';
/**
* #57. Below 600px the top bar is not in the layout, and search is a
* button that opens a modal on the pages where searching means
* anything.
*
* **This is the tier that can answer it, with one honest exception.**
* The shell's breakpoints are media queries, which the component tier
* cannot set — so whether the bar is a grid row, and whether a header
* grows a search button, is a question for a real viewport. What this
* tier *cannot* answer is the reason the surface is a `wa-dialog`:
* #60 read out of the Web Awesome source that `wa-popup` falls back to
* `position: fixed` where there is no Popover API (Chrome 113, the
* reference device) and that `.main-panel`'s `contain: paint` clips a
* fixed descendant. Chromium and WebKit here both have the Popover API,
* so a popup is top-layered and correct, and **an assertion that the
* modal is not clipped would pass on the broken build.** The mechanism
* is asserted in `frontend/test/components/search-dialog.test.ts`
* instead, where "is there a native <dialog>" is a question a browser
* can answer without lying.
*
* **And it is measured per element.** `layout-overflow.spec.ts` asks
* whether the *shell* needs sideways scrolling and was green throughout
* the defect it is named for; the win this issue is for is vertical and
* belongs to one element, so it is that element's box that is read.
*/
type Page = import('@playwright/test').Page;
/** The reference device's own viewport, and a common small phone. */
const DEVICE = { width: 424, height: 439 };
const PHONE = { width: 390, height: 780 };
/**
* Where the top bar is, and how much of the screen it costs.
*
* `contentTop` is measured against the *jobs band* rather than against
* the window, because that band is a real grid row whenever work is in
* flight (#62) and the app under these specs is long-lived — a job
* staged by another file is still in the store. Measuring against zero
* makes this assertion say "and no background job is running", which is
* not what it is for and is not something it can arrange.
*/
const barBox = (page: Page) =>
page.evaluate(() => {
const bar = document.querySelector<HTMLElement>('header.top-bar')!;
const main = document.querySelector<HTMLElement>('.main-panel')!;
const band = document.querySelector<HTMLElement>('job-band');
const cs = getComputedStyle(bar);
return {
position: cs.position,
height: Math.round(bar.getBoundingClientRect().height),
/** Where the content starts, and where the row above it ends. */
contentTop: Math.round(main.getBoundingClientRect().top),
aboveBottom: Math.round(band?.getBoundingClientRect().bottom ?? 0),
};
});
test.describe('the phone has no top bar', () => {
test.beforeEach(async ({ app }) => {
await app.setViewportSize(DEVICE);
});
test.afterEach(async ({ app }) => {
await app.setViewportSize({ width: 1440, height: 900 });
});
/**
* The vertical win, measured rather than asserted by the absence of
* an element: `display: none` on the header would satisfy "the bar is
* hidden" while leaving a 3.25em grid row exactly where it was.
*/
test('gives the row back to the content', async ({ app }) => {
const box = await barBox(app);
// Out of flow, so it takes no row — and 1px rather than 0, because
// it still carries the document's h1.
expect(box.position).toBe('absolute');
expect(box.height).toBeLessThanOrEqual(1);
// The content starts where the row above it ends, and there is no
// row above it but the jobs band. On `main` at the time of writing
// the content started 52px down from that point.
expect(box.contentTop).toBe(box.aboveBottom);
});
/**
* The wordmark yields its width and not its existence, which is the
* rule `top-bar-fit.ts` already lives by one band up: with the bar
* gone, `display: none` would take this document from one top-level
* heading to none on every page whose own header has no h1 —
* Settings has no `page-header` at all.
*/
test('still has a top-level heading', async ({ app }) => {
await expect(
app.getByRole('heading', { name: 'YellowJacket', level: 1 }),
).toHaveCount(1);
});
/**
* And its four controls are gone from the tab order, not merely from
* sight. A visually-hidden container is still focusable, and tabbing
* into a search box nobody can see is worse than not having one.
*/
test('leaves nothing in the bar to tab into', async ({ app }) => {
for (const tag of [
'nav-history',
'library-filter',
'search-bar',
'job-indicator',
]) {
await expect(app.locator(`header.top-bar ${tag}`)).toBeHidden();
}
const focusable = await app.evaluate(
() =>
document
.querySelector('header.top-bar')!
.querySelectorAll('input, select, button, a[href]').length,
);
// Nothing in the bar is *rendered*, so nothing in it can be
// focused; the controls are display:none, which takes their own
// shadow content with them.
expect(focusable).toBe(0);
});
});
test.describe('search on a phone', () => {
test.beforeEach(async ({ app }) => {
await app.setViewportSize(PHONE);
});
test.afterEach(async ({ app }) => {
await app.setViewportSize({ width: 1440, height: 900 });
});
test('is a button in the view that can be searched', async ({ app }) => {
await app.getByTestId('tab-tracks').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'tracks',
);
// Scoped to the view: every cached primary view holds a
// `page-header`, and an unscoped testid is `bottom-nav`'s
// "resolved to 2 elements" trap again.
const trigger = app.locator('track-list page-header search-trigger button');
await expect(trigger).toBeVisible();
await expect(trigger).toHaveAttribute('aria-label', 'Search tracks');
});
/**
* The whole journey, which is the thing the issue asks for: a button,
* a modal, and the results on the page behind it saying what they are
* showing.
*/
test('opens a modal, filters the page, and says so', async ({ app }) => {
await app.getByTestId('tab-tracks').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'tracks',
);
await app.locator('track-list page-header search-trigger button').click();
const dialog = app.getByTestId('search-dialog');
// Attached, not visible: `wa-dialog`'s host is `display: contents`,
// so the element carrying the testid always reports hidden — what
// is visible is the native `<dialog>` inside it. That awkwardness
// is written down in CLAUDE.md and is why the assertion that this
// is really up is the role query below.
await expect(dialog).toBeAttached();
// Named, which `getByRole` can answer and the a11y snapshot cannot
// — the snapshot never prints a dialog's name, named or not. This
// is also the assertion that the dialog is genuinely showing.
await expect(
app.getByRole('dialog', { name: 'Search tracks' }),
).toBeVisible();
// Scoped: the header's own box is still in the document, hidden.
// This is the one moment there are two `search-input`s.
await dialog.getByTestId('search-input').fill('aurora');
// Enter hands the screen back, because the results are the page.
await app.keyboard.press('Enter');
await expect(dialog).not.toBeAttached();
// Polled: the box debounces by 150ms, so reading the page once
// straight after closing the dialog can capture the state before
// the term ever reached the store.
await expect
.poll(() =>
app.evaluate(
() =>
document
.querySelector('[data-testid="main-content"] track-list')
?.shadowRoot?.querySelector('page-header')
?.shadowRoot?.querySelector('[data-testid="page-search-scope"]')
?.textContent?.trim() ?? '',
),
)
.toMatch(/matching.*aurora/);
// And the button says the search is on, in its name rather than
// only in its colour.
await expect(
app.locator('track-list page-header search-trigger button'),
).toHaveAttribute('aria-label', /aurora/);
// Leave the app as the next spec expects to find it.
await app.locator('track-list page-header search-trigger button').click();
await app.getByTestId('search-dialog').getByTestId('search-input').fill('');
await app.keyboard.press('Escape');
});
/**
* Two of the seven searchable views have no `page-header` — they are
* detail views that filter on the term and say so in their own
* headers. A trigger placed only in `page-header` would leave them
* with a search they can show and no way to set it, which is #24's
* sentence broken in the band it was written for.
*/
test('reaches the playlist detail view too', async ({ app }) => {
await app.getByTestId('tab-playlists').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'playlists',
);
// `.playlist-item`, which is what the list renders. Asserted to
// exist rather than skipped on: the seed has a playlist, and a
// spec that quietly skips when its selector stops matching is a
// spec that reports success for a renamed class.
const first = app.locator('playlist-view .playlist-item').first();
await expect(first).toBeVisible();
await first.dblclick();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'playlist-details',
);
await expect(
app.locator('playlist-details search-trigger button'),
).toBeVisible();
});
/**
* A button that cannot do anything is worse than none — the rule
* `library-status-indicator` was rewritten on. Home has nothing of
* its own to search and is not in the store's map.
*/
test('offers no button where there is nothing to search', async ({ app }) => {
await app.getByTestId('tab-home').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'home',
);
await expect(
app.locator('home-view page-header search-trigger button'),
).toHaveCount(0);
});
test('offers no button on a desktop, where the header has a box', async ({
app,
}) => {
await app.setViewportSize({ width: 1440, height: 900 });
await app.getByTestId('nav-tracks').click();
await expect(
app.locator('track-list page-header search-trigger button'),
).toHaveCount(0);
await expect(app.locator('header.top-bar search-bar')).toBeVisible();
});
});
/**
* #148, which #57 inherits: `library-filter` is the only control in the
* app that calls `setSelectedLibrary`, and the bar it lived in is gone
* on a phone. #143 refused to hide it as a fit step for exactly this
* reason, so dropping it here would have been the same trade.
*/
test.describe('the library filter has a home that is not the bar', () => {
test.afterEach(async ({ app }) => {
await app.setViewportSize({ width: 1440, height: 900 });
});
test('is in Settings, and is reachable from a phone', async ({ app }) => {
await app.setViewportSize(PHONE);
await app.getByTestId('tab-more').click();
await app.getByTestId('nav-drawer').getByTestId('nav-settings').click();
await expect(app.getByTestId('main-content')).toHaveAttribute(
'data-active-view',
'settings',
);
const filter = app.getByTestId('settings-library-filter');
await expect(filter).toBeVisible();
await expect(filter.locator('select')).toBeVisible();
});
test('and it is the same control at every width', async ({ app }) => {
// Not a phone-only copy: "where do I change which library I am
// browsing" having two answers by viewport is the fault, not the
// fix.
await app.setViewportSize({ width: 1440, height: 900 });
await app.getByTestId('nav-settings').click();
await expect(app.getByTestId('settings-library-filter')).toBeVisible();
await expect(app.locator('header.top-bar library-filter')).toBeVisible();
});
});
e2e/specs/top-bar-fit.spec.ts
+66 -20
View File
@@ -26,10 +26,22 @@ type Page = import('@playwright/test').Page;
* 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.
* enforced minimum.
*
* **390 is kept, and what it asks changed with #57.** There is no bar
* to fit below 600px any more — it is out of the grid and visually
* hidden — so "nothing hangs out of it" is a claim about an element
* with no row, and would pass on a build that had merely broken the
* bar. Dropping the width would be dropping the one place this file
* can still say something true about a phone, so it asserts the
* *stronger* property instead, below: the bar is out of the layout
* altogether, which is the thing #57 wanted and the thing that makes
* fitting moot.
*/
const WIDTHS = [390, 600, 800, 899, 900, 1440];
const WIDTHS = [600, 800, 899, 900, 1440];
/** Where #57 leaves the bar, and where the desktop still has one. */
const PHONE_WIDTH = 390;
/**
* A scan whose title is as long as a real one gets. The label is capped
@@ -90,6 +102,56 @@ const collapsed = (page: Page) =>
}));
test.describe('the top bar fits the window', () => {
/**
* The phone's answer, which is not "it fits" (#57).
*
* The bar has no grid row below 600px, so measuring its children
* against its content box is measuring a 1px box that is already
* invisible — a fit pass would collapse the wordmark every time and
* report success about nothing, which is why `measureTopBarFit`
* declines to run at all when the bar is out of flow. What is worth
* asserting here is that the fit pass has not quietly started
* *undoing* that: a rule that put the bar back in the layout would
* pass every assertion in this file and cost a 439px screen 12% of
* its height.
*/
test(`the bar is out of the layout at ${PHONE_WIDTH}px, with a job running`, async ({
app,
testctl,
}) => {
await app.setViewportSize({ width: PHONE_WIDTH, height: 600 });
await testctl.emit('JobsChanged', [LONG_JOB]);
// Not merely hidden: `display: none` on the header would satisfy
// "invisible" and leave the 3.25em row exactly where it was. So
// the assertion is that the content starts where the row above it
// ends -- and with a job staged, the row above it is the jobs
// band, which is the whole reason this row could go.
await expect
.poll(() =>
app.evaluate(() => {
const bar = document.querySelector<HTMLElement>('header.top-bar')!;
const main = document.querySelector<HTMLElement>('.main-panel')!;
const band = document.querySelector<HTMLElement>('job-band')!;
return {
position: getComputedStyle(bar).position,
gap:
Math.round(main.getBoundingClientRect().top) -
Math.round(band.getBoundingClientRect().bottom),
};
}),
)
.toEqual({ position: 'absolute', gap: 0 });
// And the work is still visible, in the band that replaced the
// indicator (#62) — which is what made this row removable at all.
await expect(app.locator('job-indicator')).toBeHidden();
await expect(app.locator('job-band').locator('job-row')).toHaveCount(1);
await app.setViewportSize({ width: 1440, height: 900 });
});
for (const width of WIDTHS) {
test(`no control sits outside the bar at ${width}px, idle`, async ({
app,
@@ -108,23 +170,7 @@ test.describe('the top bar fits the window', () => {
// The indicator has to actually be up, or this test passes by
// measuring the idle case under another name.
//
// Below 600px there is deliberately no indicator to measure:
// #62 stands it down and puts the rows in `<job-band>` instead,
// in the layout under the bar. So at 390 the assertion is that
// it *is* away and the bar still fits -- which is the same
// property (the bar has nothing hanging out of it) reached by the
// other branch of the same rule, rather than a width quietly
// dropped from the list.
const phone = width < 600;
await expect(app.locator('job-indicator'))[
phone ? 'toBeHidden' : 'toBeVisible'
]();
if (phone) {
await expect(app.locator('job-band').locator('job-row')).toHaveCount(1);
}
await expect(app.locator('job-indicator')).toBeVisible();
await expect.poll(() => overflowingChildren(app)).toEqual([]);
});
frontend/index.css
+53 -44
View File
@@ -412,8 +412,17 @@ body div.sidebar {
=================================================================== */
@media (max-width: 599px) {
body {
/* **There is no top-bar row here (#57).** Every one of the five
things that bar held has somewhere else to be below 600px:
`nav-history` is the platform's own gesture (gone from 899
down), the job indicator is `<job-band>` (#62), the search
box is a modal opened from the view's own header
(`search-trigger`), the library filter is Settings ->
Libraries (#148), and the wordmark is below. That is 3.25em
of a 439 CSS px viewport -- the single biggest vertical win
available on the reference device, which is why #57 asks for
the row rather than for a smaller bar. */
grid-template:
"top-bar" 3.25em
"jobs-band" auto
"main-panel" 1fr
"bottom-bar" auto
@@ -433,46 +442,55 @@ body div.sidebar {
grid-area: bottom-nav;
}
/* The 2em gutters are half a thumb each at this width, and the
subtitle is already gone from 900 down.
/* The bar is out of the layout, and out of it the way the *wordmark*
already goes at desktop widths: visually hidden rather than
`display: none`, because that `h1` is the document's top-level
heading and this app would otherwise have none on the pages whose
own header is empty by design (`page-header` renders no `h1` when
`heading` is '', and Settings has no `page-header` at all).
`min-width: 0` is the load-bearing half. A grid item's implicit
minimum is `auto` -- its content -- so a header whose children
ask for 580px makes the *body* 580px wide inside a 360px
viewport, and `overflow-x: hidden` then hides the right-hand
third of the app rather than fitting it. Every box between the
viewport and the content that must shrink needs this. */
Its four *controls* are `display: none` below, which is what
keeps them out of the tab order -- a visually-hidden container is
still focusable, and tabbing into a search box nobody can see is
worse than not having one.
This is `styles/sr-only.css.ts`'s recipe again, written out
because that one is a `CSSResult` for shadow roots and this is
the light DOM. `position: absolute` is also what tells
`services/top-bar-fit.ts` there is no row to fit into. */
.top-bar {
padding-left: 0.75em;
padding-right: 0.75em;
gap: 0.5em;
min-width: 0;
position: absolute;
width: 1px;
height: 1px;
padding: 0;
margin: -1px;
overflow: hidden;
clip-path: inset(50%);
white-space: nowrap;
border: 0;
gap: 0;
min-width: 0;
}
.top-bar nav-history,
.top-bar library-filter,
.top-bar search-bar,
.top-bar job-indicator {
display: none;
}
/* `min-width: 0` is load-bearing wherever a box sits between the
viewport and content that must shrink. A grid item's implicit
minimum is `auto` -- its content -- so one child insisting on
580px makes the *body* 580px wide inside a 360px viewport, and
`overflow-x: hidden` then hides the right-hand third of the app
rather than fitting it. */
.content-area,
.main-panel,
.bottom-bar {
min-width: 0;
}
.title {
font-size: 1.1em;
}
/* The search box is the one header control worth its width; the
library filter is a rarely-changed setting and reachable from
the drawer's Settings.
`nav-history` is already gone from 899 down. It would belong
here anyway and for a stronger reason than width: the phone has
Back as a gesture or a button the OS owns, and this app hooks it
(`popstate`), so a second Back in the chrome duplicates a
control the platform provides. */
.top-bar library-filter {
display: none;
}
/* The full-screen now-playing view *is* the transport, so the bar
repeating it underneath is 4em of a small screen spent saying
the same thing twice -- visible in a screenshot, invisible to
@@ -486,11 +504,6 @@ body div.sidebar {
body:has(#main-content[data-active-view="now-playing"]) .bottom-bar {
display: none;
}
.top-bar search-bar {
flex: 1 1 auto;
min-width: 0;
}
}
@media (max-width: 599px) {
@@ -543,16 +556,12 @@ body job-band {
deletes this bar entirely and is blocked on the indicator having
somewhere else to live -- this is that somewhere.
`display: none` rather than a fit step: `services/top-bar-fit.ts`
already skips children whose computed display is none, so the bar's
measurement simply sees one fewer child, and `[compact]` toggling on
a hidden element costs nothing. */
#57 has since done exactly that, so the indicator's own rule now
lives with the other three in the phone block above, where the bar
goes out of the layout in one statement rather than four. What stays
here is the band, and the argument for it. */
@media (max-width: 599px) {
.top-bar job-indicator {
display: none;
}
/* ...and its rows appear here, in the grid row above the content.
/* The indicator's rows appear here, in the grid row above the content.
In flow rather than over it: a fixed band reads fine in a
screenshot and is unusable, because at 424x439 a compact panel
is ~200px of a 439px screen and it *covers* what is under it.
frontend/src/services/top-bar-fit.ts
+10
View File
@@ -119,6 +119,16 @@ export const FIT_STEPS: readonly FitStep[] = [
* @returns the ids collapsed, in the order they were given up.
*/
export function measureTopBarFit(bar: HTMLElement): string[] {
// Below 600px there is no bar to fit (#57): `index.css` takes it
// out of the grid and leaves it visually hidden at 1px, carrying
// nothing but the document's `h1`. Measuring that reports the
// wordmark as overflowing 1px of content box and collapses it every
// time -- true, and about nothing, since the whole bar is already
// invisible. Asking the *computed position* rather than the
// viewport width is what keeps this file free of a breakpoint the
// stylesheet already owns.
if (getComputedStyle(bar).position === 'absolute') return [];
const fits = () => {
const style = getComputedStyle(bar);
const box = bar.getBoundingClientRect();