Show background jobs in the phone's layout, not a popover #166

Merged
logan merged 3 commits from feat/62-jobs-as-a-notification into main 2026-08-20 22:32:47 +00:00
10 changed files with 591 additions and 8 deletions
+48
View File
@@ -4302,3 +4302,51 @@ compiles against HEAD with a single shim
(`SetPlaybackFinishedHandler` gained a `srcErr error` parameter), which
makes "did the backend fix cause this" a ten-minute question instead of
a full checkout.
## An overlay band is not a notification, it is a lid (measured 2026-08-20)
#62 asks for background jobs to become "a notification" on the phone,
and the app has exactly one notification surface, so the first version
of the fix put `<job-panel>` in `notification-host`'s band — which is
`position: fixed` under the header. It renders correctly, it is on top,
it is inside the viewport, and it is unusable.
At the device's 424x439 viewport a **compact** panel showing two active
jobs is ~216px — half the screen — drawn over the content, with
`pointer-events: auto` so it swallows every tap underneath. Nothing in
the component tier could see it. The e2e suite could: four specs failed,
and *none* of them was about jobs — two `phone-shell` journeys into the
full-screen Now Playing and `header-action-overflow`'s phone case, all
three because the band was intercepting taps meant for the app.
`<job-band>` is in the shell's grid instead, as a row between the top
bar and the main panel, so it **pushes**. That is #24's one sentence
("no action is ever unreachable at any supported size") deciding a
layout question: a band that hides the app in order to say the app is
busy has traded the popover's fault for a worse one.
Two things fell out of it worth keeping:
- **A finished row in flow is furniture.** The overlay could afford to
keep terminal jobs around; a row that holds the content down after
the work is done cannot. `job-panel` grew `active-only` for the band,
and Settings keeps finished rows because that is where "did the last
scan work" is asked.
- **`job-row` already had the right density.** `variant="compact"` is
described in its own source as "the popover density", which is
exactly what the band is replacing — 216px against 259px for the
same two jobs, and no per-job statistics that a phone has no room
for.
## The e2e suite is the tier that sees a shell regression (2026-08-20)
Worth stating because it decided how #62 was verified. The change is
one component, one stylesheet and one line of `index.html`; `make
ui-test` (955 tests) passed on the broken overlay version and so did
`tsc`, `lint` and the whole Go suite. The failure was three specs that
have nothing to do with jobs, failing on `click()` timeouts.
The corollary for anything that draws over the shell: **run the whole
e2e suite, not the spec you wrote.** A spec written for a feature
asserts the feature works; what a new overlay breaks is everything
else, and only the suite is looking at that.
+31 -4
View File
@@ -593,11 +593,38 @@ rather than renaming them.
`ClearFinishedJobs` is global — a Clear under Libraries would discard
the index build's history too; a finished row dismisses itself.
The header `job-indicator` is untouched and is still the one view of
everything at once, from every page. One consequence worth knowing
before writing a spec: a section holding a `job-panel` also holds a
`job-details-drawer`, whose own header carries `.header` — so
The header `job-indicator` is still the one view of everything at
once, from every page**on a desktop.** One consequence worth
knowing before writing a spec: a section holding a `job-panel` also
holds a `job-details-drawer`, whose own header carries `.header` — so
`config-section .header` is ambiguous the moment a job exists.
**Below 600px that indicator stands down and `<job-band>` takes
over** (#62), because a popover is a *disclosure* and background work
is the one thing a phone should not make you open something to see —
and because #57 deletes the bar it is anchored to and is blocked on
it having somewhere else to live. The band is the same `job-panel`,
so `applyJobControl` and its index-build confirmation come along
rather than being reimplemented; `kinds="*"` is how it says "every
kind", which is what the indicator was for.
Three things about it are load-bearing. **It is in the layout, not
over it**, as its own grid row above the main panel: the first
version put it in `notification-host`'s fixed band, which reads fine
in a screenshot and is unusable — at 424×439 a compact panel is
~200px of a 439px screen and it *covers* what is under it, which four
e2e specs caught by failing on taps it was intercepting. **It shows
active work only** (`active-only`), because in flow a finished row is
furniture that keeps the content pushed down after the work is done;
finished rows stay where the work was started, which is #27's rule.
And **it renders nothing above 600px**, from `matchMedia` rather than
a media query, because that decides whether the element *exists*
Settings already holds four `job-panel`s and a fifth answering for
every kind is `bottom-nav`'s "resolved to 2 elements" trap again.
`index.css` keeps it `display: none` outside the phone for a second
reason: an in-flow grid child with no named area is auto-placed into
one of the shell's rows, which is what the skip link is absolutely
positioned to avoid.
- `config` — TOML-based settings. Settings page uses HTMX + templ for server-rendered HTML fragments.
- `playlist` / `smartplaylist` — Playlist CRUD and rule-based smart playlists.
- `mediacontrols` — OS media controls behind one `Handler`: MPRIS over
+204
View File
@@ -0,0 +1,204 @@
import { test, expect } from '../support/fixtures.js';
/**
* #62. On a phone, background work is shown in the notification band
* and the header indicator stands down.
*
* The report was that the indicator's popover "is obscured by other UI,
* so it cannot be read while jobs run". Worth saying plainly: **that
* symptom did not reproduce in this tier.** Measured at the device's
* own 424x439 viewport, the popover was neither clipped nor covered —
* `elementFromPoint` at its centre returned the indicator at every
* width tried. So this is not a fix for a stacking bug, and a spec
* asserting one would be a spec asserting something that was never
* true here.
*
* What is true regardless, and is what these assert:
*
* - a popover is a **disclosure**, and it is anchored to a bar 3.25em
* tall on a screen 439px tall. Background work is the one thing a
* phone should not make you open something to see.
* - #57 deletes that bar and is *blocked on this issue*, because the
* indicator needs somewhere else to live first. Somewhere else is
* the band, and the test that matters for #57 is that the bar no
* longer holds the indicator at all.
*
* This is the media-query tier by necessity: a query inside a shadow
* root is answered by the viewport, and `notification-host` decides
* whether the panel *exists* from `matchMedia`. The component tier
* cannot set either.
*/
type Page = import('@playwright/test').Page;
const JOBS = [
{
id: 'phone:scan',
kind: 'library-scan',
state: 'running',
title: 'Scanning Music',
current: 40,
total: 100,
caps: { pausable: true, cancellable: true },
},
{
id: 'phone:idx',
kind: 'index-build',
state: 'running',
title: 'Building the search index',
current: 2,
total: 9,
caps: { pausable: true, cancellable: true },
},
];
/** The panel the band renders. Playwright's CSS engine pierces open
* shadow roots, which is what keeps this one line. */
const bandPanel = (page: Page) => page.locator('job-band').locator('job-panel');
const PHONE = { width: 424, height: 439 };
const DESKTOP = { width: 1100, height: 800 };
test.describe('background jobs on a phone', () => {
test('are shown in the band, without opening anything', async ({
app,
testctl,
}) => {
await app.setViewportSize(PHONE);
await testctl.emit('JobsChanged', JOBS);
await expect(bandPanel(app)).toBeVisible();
// Both jobs, drawn by real `job-row`s -- asking the rows what they
// hold rather than reading the panel's text, which would pass
// whether or not a row rendered. Playwright's CSS engine pierces
// open shadow roots, which is what makes this one line;
// `querySelectorAll` does not, and stops at `job-panel`.
await expect(bandPanel(app).locator('job-row')).toHaveCount(2);
await expect(
bandPanel(app).locator('job-row').first(),
).toContainText('Scanning Music');
});
/**
* The #57 assertion. Not "the indicator is invisible" — that could be
* true because the bar overflowed — but that the shell's own rule
* puts it away at this width.
*/
test('leave the top bar, which is what #57 is waiting for', async ({
app,
testctl,
}) => {
await app.setViewportSize(PHONE);
await testctl.emit('JobsChanged', JOBS);
await expect(bandPanel(app)).toBeVisible();
await expect(app.locator('job-indicator')).toBeHidden();
});
/**
* The property the first attempt at this got wrong, so it is the one
* worth pinning: the band is **in the layout**, not over it.
*
* A fixed band reads fine in a screenshot and is unusable -- at
* 424x439 a compact panel is ~200px of a 439px screen and it covers
* what is under it. Four specs failed on that version, two
* phone-shell journeys and the header's action menu, because the
* panel was intercepting the taps. So: nothing of the app is
* underneath it, and the main panel starts below it.
*/
test('push the content down rather than covering it', async ({
app,
testctl,
}) => {
await app.setViewportSize(PHONE);
const before = await app
.getByTestId('main-content')
.evaluate((el) => el.getBoundingClientRect().top);
await testctl.emit('JobsChanged', JOBS);
await expect(bandPanel(app)).toBeVisible();
const after = await app.evaluate(() => {
const band = document.querySelector('job-band') as HTMLElement;
const main = document.querySelector(
'[data-testid="main-content"]',
) as HTMLElement;
const b = band.getBoundingClientRect();
const m = main.getBoundingClientRect();
// What the browser reports at the band's own centre. If this is
// anything but the band, the band is sitting on top of it.
const hit = document.elementFromPoint(
Math.round(b.x + b.width / 2),
Math.round(b.y + b.height / 2),
);
return {
mainTop: m.top,
bandBottom: b.bottom,
withinViewport: b.bottom <= window.innerHeight + 0.5,
hit: hit?.tagName.toLowerCase() ?? null,
};
});
expect({
pushed: after.mainTop > before,
mainClearsBand: after.mainTop >= after.bandBottom - 0.5,
withinViewport: after.withinViewport,
hit: after.hit,
}).toEqual({
pushed: true,
mainClearsBand: true,
withinViewport: true,
hit: 'job-band',
});
});
/**
* A running job repaints several times a second. The stack it sits
* beside is `role="status" aria-live="polite"`, and a progress bar
* inside a live region is a screen reader reading a number out over
* and over — so the two are siblings in the band rather than one
* list, and this is what says so.
*/
test('are not inside the live region they sit beside', async ({
app,
testctl,
}) => {
await app.setViewportSize(PHONE);
await testctl.emit('JobsChanged', JOBS);
await expect(bandPanel(app)).toBeVisible();
const insideLiveRegion = await app.evaluate(() => {
const band = document.querySelector('job-band');
// Neither the band itself nor anything it is nested in may be a
// live region -- `closest` answers both at once.
return !!band?.closest('[aria-live]') || band?.hasAttribute('aria-live');
});
expect(insideLiveRegion).toBe(false);
});
/**
* `bottom-nav` rendering its duplicate `<app-sidebar>` unconditionally
* broke 30 specs with "resolved to 2 elements" on a viewport where it
* was not even visible. Settings already holds four `job-panel`s, so
* a fifth that answers for *every* kind is the same trap — which is
* why the band decides from `matchMedia` whether the element exists
* rather than hiding it with CSS.
*/
test('do not leave a second panel behind on a desktop', async ({
app,
testctl,
}) => {
await app.setViewportSize(DESKTOP);
await testctl.emit('JobsChanged', JOBS);
await expect(app.locator('job-indicator')).toBeVisible();
await expect(bandPanel(app)).toHaveCount(0);
});
});
+17 -1
View File
@@ -108,7 +108,23 @@ 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.
await expect(app.locator('job-indicator')).toBeVisible();
//
// 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.poll(() => overflowingChildren(app)).toEqual([]);
});
+43
View File
@@ -414,6 +414,7 @@ body div.sidebar {
body {
grid-template:
"top-bar" 3.25em
"jobs-band" auto
"main-panel" 1fr
"bottom-bar" auto
"bottom-nav" auto
@@ -522,3 +523,45 @@ body div.sidebar {
display: none;
}
}
/* Out of the desktop grid entirely. `job-band` renders nothing above
600px anyway, but an in-flow grid child with no named area is
auto-placed into a row of the shell -- the same trap the skip link is
absolutely positioned to avoid. */
body job-band {
display: none;
}
/* #62. The job indicator stands down on the phone, and its work is
shown in the notification band instead (notification-host).
Three reasons, and the first is the report: its popover is anchored
to the top bar, which is 3.25em here on a viewport 439 CSS px tall,
and it was reported as unreadable behind other UI. The second is
that a popover is a disclosure, and background work is the one thing
a phone should not make you disclose. The third is #57, which
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. */
@media (max-width: 599px) {
.top-bar job-indicator {
display: none;
}
/* ...and its 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.
Measured, not assumed -- four e2e specs failed on that version,
two phone-shell journeys and the header's action menu, because
the panel was intercepting the taps. */
body job-band {
display: block;
grid-area: jobs-band;
background-color: var(--yj-bg-elevated, #343a40);
}
}
+8
View File
@@ -30,6 +30,14 @@
<search-bar></search-bar>
<job-indicator></job-indicator>
</header>
<!-- The phone's view of background work (#62): below 600px the
indicator above stands down and its rows appear here instead,
in the layout rather than over it. `display: none` above that
width in index.css, which is also what keeps it out of the
desktop grid -- an in-flow child with no named area is
auto-placed into one of the shell's rows, which is the trap the
skip link is absolutely positioned to avoid. -->
<job-band></job-band>
<div class="sidebar">
<app-sidebar></app-sidebar>
</div>
+5
View File
@@ -38,6 +38,11 @@ import '@components/confirm-dialog/confirm-dialog.ts';
// not know what is going on. It costs a dialog and a table.
import '@components/shortcuts-overlay/shortcuts-overlay.ts';
import '@components/jobs/job-indicator.ts';
// The phone's half of the same thing (#62). Eager because it is part
// of the shell's first paint below 600px, and because a band that has
// to fetch a chunk before it can say the app is busy is late by
// exactly the interval it exists to explain.
import '@components/jobs/job-band.ts';
import '@awesome.me/webawesome/dist/styles/themes/default.css';
import '@awesome.me/webawesome/dist/components/icon/icon.js';
import { setBasePath } from '@awesome.me/webawesome/dist/webawesome.js';
+132
View File
@@ -0,0 +1,132 @@
/**
* The phone's view of background work (#62).
*
* The header `job-indicator` is a *popover*, anchored to a bar 3.25em
* tall on a screen 439 CSS px tall, and it was reported as unreadable
* behind other UI. Two things are wrong with it there regardless of
* that symptom: a popover is a **disclosure**, and background work is
* the one thing a phone should not make you open something to see; and
* #57 deletes the bar it is anchored to, and is blocked on this issue
* precisely because the indicator needs somewhere else to live first.
*
* This is that somewhere. Below 600px the indicator stands down
* (`index.css`) and its work appears here instead.
*
* Four things about it are load-bearing.
*
* **It is the existing `job-panel`, not a second job UI.** Pause,
* cancel, Details and the log all come along — and, more to the point,
* so does `applyJobControl`, which is what carries the "you will
* discard hours of downloading" confirmation for an index build. A
* host drawing its own buttons drops that silently, which is the trap
* #27 already named.
*
* **It is in the layout, not over it**, and that was measured rather
* than assumed. The first version of this put the panel in
* `notification-host`'s fixed band, which reads fine in a screenshot
* and is unusable: at 424x439 a compact panel is ~200px of a 439px
* screen, and it *covers* what is under it. Four e2e specs failed —
* two phone-shell journeys and the header's action menu — because the
* panel was intercepting the taps. A band that hides the app to tell
* you the app is busy is worse than the popover it replaced. In flow
* it pushes instead, so nothing is covered and nothing is unreachable,
* which is #24's one sentence across all three bands.
*
* **It shows active work only.** A finished row that lingers is a
* banner that stays after the work is done, which is the opposite of
* what #62 asks for ("dismissed automatically on completion") and, in
* flow, is furniture that keeps the content pushed down. Finished jobs
* are still shown where the work was started, which is #27's rule and
* unaffected.
*
* **It renders nothing at all above 600px**, from `matchMedia` rather
* than a media query, because this decides whether the element
* *exists*. `bottom-nav` learned that the expensive way: rendering its
* duplicate `<app-sidebar>` unconditionally put a second copy of every
* `nav-*` testid in the DOM and broke 30 specs on a viewport where it
* was not even visible. Settings already holds four `job-panel`s, so a
* fifth answering for *every* kind is the same trap.
*/
import { LitElement, html, css, nothing } from 'lit';
import { customElement, state } from 'lit/decorators.js';
import { jobStore } from '@store/job-store';
import { isTerminal } from '@store/job-store';
import { designTokens } from '../../styles/tokens.css';
import { PHONE_QUERY } from '../../utils/breakpoints';
import './job-panel';
@customElement('job-band')
export class JobBand extends LitElement {
@state() private phone = false;
@state() private active = 0;
private media?: MediaQueryList;
private unsubscribe?: () => void;
static override styles = [
designTokens,
css`
:host {
display: block;
min-width: 0;
}
/* The panel's own margin is for a settings section; here the
band owns the spacing. */
job-panel {
margin-top: 0;
padding: 0 0.5em 0.5em;
}
`,
];
private onMedia = (e: MediaQueryListEvent | MediaQueryList) => {
this.phone = e.matches;
};
private onJobs = () => {
this.active = jobStore.jobs.filter((job) => !isTerminal(job)).length;
};
override connectedCallback(): void {
super.connectedCallback();
this.media = window.matchMedia(PHONE_QUERY);
this.phone = this.media.matches;
this.media.addEventListener('change', this.onMedia);
// The band decides whether to render *at all*, and a panel that
// hides itself cannot tell its host that.
this.unsubscribe = jobStore.subscribe(this.onJobs);
this.onJobs();
void jobStore.init();
}
override disconnectedCallback(): void {
super.disconnectedCallback();
this.unsubscribe?.();
this.media?.removeEventListener('change', this.onMedia);
}
override render() {
// `hidden` rather than an empty render, so the grid row this
// sits in costs nothing at all while there is no work -- the
// rule `job-panel` already follows one layer down.
this.hidden = !(this.phone && this.active > 0);
if (this.hidden) return nothing;
return html`
<job-panel kinds="*" density="compact" active-only></job-panel>
`;
}
}
declare global {
interface HTMLElementTagNameMap {
'job-band': JobBand;
}
}
+41 -3
View File
@@ -51,6 +51,14 @@ export class JobPanel extends LitElement {
* literal in a template, and one of them is inside an HTMX-adjacent
* settings page where a property binding would be one more thing to
* remember.
*
* **`*` means every kind**, which is the phone's band (#62) and
* nothing else: there, this panel is standing in for the header
* indicator, whose whole job was to be the one view of everything
* at once. It is spelled `*` rather than taken as the meaning of an
* empty attribute, because empty is what a typo and a missing
* binding both produce and "show everything" is the wrong thing to
* do by accident. Empty still shows nothing.
*/
@property({ type: String })
kinds = '';
@@ -59,6 +67,31 @@ export class JobPanel extends LitElement {
@property({ type: String })
heading = '';
/**
* Row density, passed to `job-row`.
*
* `full` adds elapsed time and per-job statistics and is what a
* settings section wants, so it stays the default and the four
* existing call sites are unchanged. `compact` is what `job-row`
* itself calls "the popover density", and it is what the phone's
* band uses (#62) — there this panel *is* the popover, on a screen
* 439 CSS px tall, and the full density spent 259 of them.
*/
@property({ type: String })
density: 'compact' | 'full' = 'full';
/**
* Drop finished rows.
*
* For the phone's band (#62), which is *in the layout*: a finished
* row there is a banner that stays after the work is done and keeps
* the content pushed down. Settings keeps them, because that is
* where "did the last scan work" is asked, and a finished row there
* dismisses itself.
*/
@property({ type: Boolean, attribute: 'active-only' })
activeOnly = false;
@state()
private jobs: Job[] = [];
@@ -162,9 +195,14 @@ export class JobPanel extends LitElement {
}
private get mine(): Job[] {
const wanted = this.wanted;
const ofKind =
this.kinds.trim() === '*'
? this.jobs
: this.jobs.filter((job) =>
this.wanted.has(job.kind as JobKind),
);
return this.jobs.filter((job) => wanted.has(job.kind as JobKind));
return this.activeOnly ? ofKind.filter((job) => !isTerminal(job)) : ofKind;
}
private openDetails(id: string) {
@@ -196,7 +234,7 @@ export class JobPanel extends LitElement {
<div class="job-entry">
<job-row
.job=${job}
variant="full"
variant=${this.density}
@job-control=${applyJobControl}
></job-row>
<button
@@ -76,6 +76,68 @@ describe('<job-panel>', () => {
expect(titles(el)).toEqual(['Building the index', 'Filling in artists']);
});
/**
* #62. The phone's band has no kinds to name: it is standing in for
* the header indicator, whose whole job was to be the one view of
* everything at once.
*/
it('answers for every kind when asked with a star', async () => {
const el = await fixture<LitElement>('job-panel', { kinds: '*' });
await snapshot([
job({ id: 'scan:1', kind: 'library-scan', title: 'Scanning Music' }),
job({ id: 'idx', kind: 'index-build', title: 'Building the index' }),
job({ id: 'dl:1', kind: 'download', title: 'Downloading Glass Harbour' }),
]);
await el.updateComplete;
expect(titles(el)).toEqual([
'Scanning Music',
'Building the index',
'Downloading Glass Harbour',
]);
});
/**
* The other half of that, and the reason it is a star rather than the
* meaning of an empty attribute: empty is what a typo and a dropped
* binding both produce, and "show everything" is the wrong thing to
* do by accident.
*/
it('still shows nothing when asked for nothing', async () => {
const el = await fixture<LitElement>('job-panel', { kinds: '' });
await snapshot([job({ id: 'scan:1', kind: 'library-scan' })]);
await el.updateComplete;
expect([el.hidden, rows(el)].map(String)).toEqual(['true', '']);
});
/**
* `full` stays the default so the four settings call sites are
* untouched; the band asks for the density `job-row` calls "the
* popover density", because on the phone this panel *is* the popover.
*/
it('passes its density to the rows, defaulting to full', async () => {
const settings = await fixture<LitElement>('job-panel', { kinds: '*' });
await snapshot([job()]);
await settings.updateComplete;
const band = await fixture<LitElement>('job-panel', {
kinds: '*',
density: 'compact',
});
await snapshot([job()]);
await band.updateComplete;
expect([
rows(settings)[0]?.getAttribute('variant'),
rows(band)[0]?.getAttribute('variant'),
]).toEqual(['full', 'compact']);
});
/**
* An idle panel in four places is four pieces of furniture describing
* an absence — and `hidden` rather than an empty render, because the