e2e/specs/jobs-on-a-phone.spec.ts

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

e2e/specs/top-bar-fit.spec.ts

@@ -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([]);
     });

frontend/index.css

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

frontend/index.html

@@ -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>

frontend/index.ts

@@ -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';

frontend/src/components/jobs/job-band.ts

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