fix(player): put the phone's Now Playing button above the artwork
`.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:
@@ -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 });
|
||||
|
||||
|
||||
Reference in New Issue
Block a user