fix(player): put the phone's Now Playing button above the artwork
CI / check (push) Skipped
CI / e2e (push) Skipped
CI / check (pull_request) Successful in 2m27s
CI / e2e (pull_request) Successful in 8m1s

`.expand` is the phone's only route into the full-screen now-playing
view. It is absolutely positioned with `z-index: auto` over
`.cover-art`, which is a *later* sibling with the same z-index, so the
two tie on paint order and the later one wins. An `<img>` costs nothing
there; a track with no artwork renders a placeholder `wa-icon`, which
takes every click aimed at the button underneath it.

So the control did not work whenever the current song had no cover, on
the one platform that has no other way in. Nothing to do with the
fixture: any library has untagged files.

Measured at 390px with elementFromPoint at the button's centre — the
icon with a placeholder, the button with an image, and the button
either way with the z-index. Chosen over `pointer-events: none` on the
art, which would take the cover preview's mouseenter with it, and over
reordering the DOM, which leaves the same tie to be won by the same
accident in the other direction.

This was filed as an e2e flake, and the diagnosis was wrong: it failed
on both engines three times across two branches that could not have
caused it, and passed on re-run each time, because the spec starts the
*first* row of the track list and which track that is depends on the
order the scan inserted rows — the same root cause as #156. The new
spec picks a track *for* having no artwork, and asserts the placeholder
is rendered rather than assuming it, so it cannot quietly go back to
measuring the easy case.

Two things it has to get right, both already documented traps: the
track must be the 90-second one, since a 2-second one finishes before
the assertions run; and `library.Track.CoverArt` is empty for all 31
fixture rows, so "the first track with no cover art" selects nothing in
particular and picked a short one.

Verified by mutation: without the z-index the new spec fails on the
click in 30s, and the pre-existing one beside it passes, which is
exactly how this survived.

Closes #150
This commit is contained in:
2026-08-20 01:33:35 -04:00
parent dc6625d33a
commit ffc9490a32
3 changed files with 142 additions and 1 deletions
+48
View File
@@ -4029,3 +4029,51 @@ The spec that pins this is two assertions, not one, and that split is
deliberate: an uncapped build is *perfectly centred* and fails only the
seek-bar width, so a spec asserting centring alone would have passed
the regression.
## The phone's way into Now Playing was under the artwork (measured 2026-08-20)
`phone-shell.spec.ts`'s "opens the full-screen now playing" failed in CI
on both engines, three times across two branches that could not have
caused it, and passed on re-run each time. It was filed as a flake
(#150). It is not one: **it depends on which track is playing.**
`.expand` — the phone's only route into `<now-playing-view>` — is
`position: absolute; inset: 0` inside `.cover-art-wrapper`, and
`.cover-art` is a **later sibling**. Both have `z-index: auto`, so they
tie on paint order and the later one wins. With an `<img>` that costs
nothing; with no artwork the placeholder `wa-icon` renders and takes
every click aimed at the button underneath it.
Measured at 390px with `elementFromPoint` at the button's centre:
| playing track | hit test |
|---|---|
| has artwork | `button.expand` |
| no artwork | **`wa-icon`** |
| no artwork, with `z-index: 1` | `button.expand` |
So on a phone, the only way into the full-screen player stopped working
whenever the current song had no cover — and this has nothing to do with
the fixture: any library has untagged files.
Three things worth keeping.
**"Flaky in CI" was the wrong diagnosis and it cost three cycles.** The
spec starts the *first* row of the track list, so which track it plays
is the order the scan inserted rows in — the same root cause as #156,
one spec over. A test whose subject is a hit test has to *choose* the
case that breaks it.
**The first two hypotheses were both wrong, and both were plausible.**
A custom element's upgrade replacing its own contents, and the cover
preview's `mouseenter` opening a popup under the pointer. Neither
survived contact with `elementFromPoint`, which took a minute and would
have saved the other two cycles.
**And the spec that pins it needs the 90-second track**, because a
2-second one finishes before the assertions run — the trap
`fixtures.ts` already documents. Note the filter that does *not* work:
`library.Track.CoverArt` is empty for all 31 fixture rows, so "the
first track with no cover art" selects nothing in particular. The
placeholder's presence is asserted instead, which is the property the
test actually depends on.
+73 -1
View File
@@ -1,4 +1,4 @@
import { test, expect } from '../support/fixtures.js';
import { test, expect, LONG_TRACK } from '../support/fixtures.js';
/**
* The phone shell (plan 016 B2, phase 1).
@@ -138,6 +138,78 @@ test.describe('the shell on a phone', () => {
.toHaveAttribute('data-active-view', 'tracks');
});
/**
* The same journey with a track that has **no cover art** (#150).
*
* The test above starts the *first* row of the track list, so which
* track it plays is the order the scan inserted them in — and the
* answer decided whether it passed. A track with artwork renders an
* `<img>`, which is no obstacle; one without renders a placeholder
* `wa-icon`, which took every click aimed at the button beneath it,
* because that button is absolutely positioned with `z-index: auto`
* and the art is a *later* sibling. They tied, and the later one won.
*
* So this picks a track *for* the property that broke it, which is
* the only way the assertion means anything: the version above passes
* on a broken build roughly two runs in three, which is exactly how
* it came to cost three CI cycles across two branches that could not
* have caused it.
*/
test('opens the full-screen now playing for a track with no art', async ({
app,
}) => {
// `LONG_TRACK` by name, and not "the first track with no
// CoverArt": the *library* model reports that field empty for
// every row in this fixture (31 of 31), so filtering on it selects
// nothing in particular and picked a 2-second track, which had
// finished before the assertions ran. The placeholder check below
// is what actually holds the property this test needs.
const started = await app.evaluate(async (longTitle) => {
const tracks = (await window.__yjEvents.call(
'library.Library.GetTracks',
[0],
10_000,
)) as { FilePath: string; TrackName: string }[];
const bare = tracks.find((t) => t.TrackName === longTitle);
if (!bare) return null;
await window.__yjEvents.call(
'queue.Queue.SetQueue',
[[bare.FilePath], 0, false, { type: '', id: 0, label: '' }],
10_000,
);
await window.__yjEvents.call('queue.Queue.Play', [], 5_000);
return bare.TrackName;
}, LONG_TRACK);
expect(started).toBe(LONG_TRACK);
await expect(app.getByTestId('now-playing-title')).not.toBeEmpty();
// **The placeholder is the whole point**, so it is asserted rather
// than assumed: this test is about the thing that renders when
// there is no artwork. If the fixture ever gives this album a
// cover, this fails and says so instead of passing while measuring
// the easy case.
//
// One selector rather than a chain from the host: Playwright's CSS
// engine pierces an open shadow root, and chaining from the host
// element does not reach into it.
await expect(
app.locator('now-playing .cover-placeholder'),
).toBeAttached();
await app.getByTestId('open-now-playing').click();
await expect(app.getByTestId('main-content'))
.toHaveAttribute('data-active-view', 'now-playing');
await app.getByTestId('npv-back').click();
});
test('offers no way in on a desktop, where the bar is whole', async ({ app }) => {
await app.setViewportSize({ width: 1440, height: 900 });
@@ -188,6 +188,27 @@ export class NowPlaying extends LitElement {
cursor: pointer;
/* The art shows through; this is a target, not a picture. */
color: transparent;
/* **Above the art, or it is not a target at all** (#150).
This button is absolutely positioned with z-index auto and
the art is a *later* sibling, so the two tie on paint order
and the later one wins. With an <img> that costs nothing --
an image is not a hit-test obstacle here -- but a track with
no artwork renders a placeholder wa-icon, which is, and it
takes every click aimed at the button underneath it.
The failure is therefore per *track*, not per build: on a
phone the only way into the full-screen now-playing view
stopped working whenever the current song had no cover.
Measured with elementFromPoint at the button's centre --
wa-icon with a placeholder, button.expand with an image, and
button.expand either way once this line exists.
z-index rather than pointer-events: none on the art, which
would take the cover preview's mouseenter with it; and
rather than reordering the DOM, which would leave the same
tie to be won by the same accident in the other direction. */
z-index: 1;
}
.expand:focus-visible {