Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7699fcd9dc |
@@ -3961,3 +3961,51 @@ The general rule for this repo's fixture library: it is deliberately
|
|||||||
full of edge cases (untagged, unicode, duplicates, extremes), so a spec
|
full of edge cases (untagged, unicode, duplicates, extremes), so a spec
|
||||||
that wants an *ordinary* track has to **say so** — filter on the
|
that wants an *ordinary* track has to **say so** — filter on the
|
||||||
property it depends on rather than slicing.
|
property it depends on rather than slicing.
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
|||||||
@@ -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).
|
* The phone shell (plan 016 B2, phase 1).
|
||||||
@@ -138,6 +138,78 @@ test.describe('the shell on a phone', () => {
|
|||||||
.toHaveAttribute('data-active-view', 'tracks');
|
.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 }) => {
|
test('offers no way in on a desktop, where the bar is whole', async ({ app }) => {
|
||||||
await app.setViewportSize({ width: 1440, height: 900 });
|
await app.setViewportSize({ width: 1440, height: 900 });
|
||||||
|
|
||||||
|
|||||||
@@ -188,6 +188,27 @@ export class NowPlaying extends LitElement {
|
|||||||
cursor: pointer;
|
cursor: pointer;
|
||||||
/* The art shows through; this is a target, not a picture. */
|
/* The art shows through; this is a target, not a picture. */
|
||||||
color: transparent;
|
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 {
|
.expand:focus-visible {
|
||||||
|
|||||||
Reference in New Issue
Block a user