Compare commits
11
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
e3b64f9255 | ||
|
|
c7e5a4f086 | ||
|
|
f65822c4b2 | ||
|
|
32d4dc2c82 | ||
|
|
218e4f5e99 | ||
|
|
56a5ff99fe | ||
|
|
af4b28b0d7 | ||
|
|
4ee5b4b473 | ||
|
|
a70a7ed9eb | ||
|
|
de2cb2693a | ||
|
|
880adff12c |
@@ -4429,3 +4429,147 @@ the content starts where the **row above it** ends, which is true with a
|
||||
job running and without one. An assertion against an absolute
|
||||
coordinate was quietly also asserting "and no background job exists",
|
||||
which is not something that spec is about or can arrange.
|
||||
|
||||
## The queue was already the right rectangle; what it lacked was an entry (measured 2026-08-21)
|
||||
|
||||
#55 asks for the queue to be "a real screen instead of a pop-open
|
||||
sidebar", and its Direction asks for a `DETAIL_LOADERS` mount. Measured
|
||||
against `880adff` at the reference device's real viewport (424x439),
|
||||
with #24's overlay open:
|
||||
|
||||
| box | rect |
|
||||
|---|---|
|
||||
| `.main-panel` | 424 x 318 |
|
||||
| `queue-panel` host | 424 x 318 |
|
||||
| `.panel-content` | 424 x 318 |
|
||||
| `.scrim` | 424 x 318, entirely underneath the panel |
|
||||
|
||||
So a detail-view mount would have drawn the same rectangle in the same
|
||||
place. Three things were genuinely missing, and none of them is a
|
||||
rendering:
|
||||
|
||||
- **Back navigated the page underneath and left the queue up.** Opened
|
||||
on Artists, pressed back: `data-active-view` went `artists` ->
|
||||
`albums`, `open` stayed `true`. A press that changes something the
|
||||
user cannot see, and costs them their place.
|
||||
- **The scrim has zero reachable pixels at phone width**, because
|
||||
`panel-content` is `width: 100%` there. #24's tap-outside-to-close
|
||||
does not exist on the device.
|
||||
- The only pointer route out was a **25x21px** button.
|
||||
|
||||
The rule that followed is that the queue is a *place* exactly while it
|
||||
is an overlay and a *control* while it is a column, which reuses #24's
|
||||
computed mode rather than adding a breakpoint.
|
||||
|
||||
**The containment finding is the reason the Direction was not
|
||||
followed.** Read off the running app rather than the stylesheet:
|
||||
|
||||
| element | computed `contain` |
|
||||
|---|---|
|
||||
| `queue-panel` (open, overlay) | `layout style` |
|
||||
| `.content-area` | `layout style` |
|
||||
| `.main-panel` | `content` |
|
||||
| `.main-panel > *` (a view) | `content` |
|
||||
|
||||
`queue-panel` has a `wa-popup` context menu, and #60's finding is that
|
||||
`position: fixed` escapes overflow but not paint containment on
|
||||
Chrome 113. Its ancestry today is paint-free to `body`; a
|
||||
`DETAIL_LOADERS` mount would have put it under two paint-containing
|
||||
ancestors. **No tier here can see that** — CI's Chromium and WebKit
|
||||
both have the Popover API — so the spec asserts the mechanism (the
|
||||
panel is not under a paint-contained ancestor) rather than the
|
||||
symptom. This is the second change in a row where the honest assertion
|
||||
was about where an element *is* rather than how it *looks*.
|
||||
|
||||
One thing worth knowing about the spec: **three of its nine tests fail
|
||||
on the build before the change and the other six cannot.** "The entry
|
||||
is not orphaned" and "a docked column is not in the stack" are both
|
||||
vacuously true of a build that pushes no entry at all. Reverting the
|
||||
source and re-running is what established which were which, and the
|
||||
file says so in its header rather than implying all nine reproduce.
|
||||
|
||||
## The phone's transport, and three things that only a screenshot or a stash could see (measured 2026-08-21)
|
||||
|
||||
#59 and #56 were done as one PR — argued on #73 first — because they are
|
||||
the same row of pixels: one removes controls from the phone's bar and
|
||||
the other enlarges what is left, and both are one property on
|
||||
`player-controls`. Measured at 424x439 before:
|
||||
|
||||
| control | before | after |
|
||||
|---|---|---|
|
||||
| bar: shuffle / prev / play / next / repeat | 33x21 each | prev/next 44, play 56, shuffle+repeat moved |
|
||||
| bar: favourite | **18x14** | 44x44 |
|
||||
| bar: queue button | 33x29 | gone (#59) |
|
||||
| Now Playing: all five | 33x21 each | 44, play 64 |
|
||||
| desktop bar: all five | 33x21 | **33x21** |
|
||||
|
||||
Four things cost a cycle each and are worth keeping.
|
||||
|
||||
**A `<button>` does not inherit its font from its parent.** The UA
|
||||
stylesheet gives it one, so `font-size: inherit` on a button is a
|
||||
*change*, not a no-op: it took every desktop control from 33x21 to
|
||||
36x24 by moving them from 13.3px to the shell's 16px. Nothing failed.
|
||||
The only way it surfaced was measuring the baseline by stashing the file
|
||||
and re-running.
|
||||
|
||||
**And the pixel it was first pinned with was the wrong assertion.** The
|
||||
spec asserted the literal `'33x21'`, measured in Chromium — and WebKit
|
||||
draws the same button **36x24**, so it failed in CI on a build where
|
||||
nothing was wrong. A button's box comes from the UA stylesheet when the
|
||||
author sets nothing, and what each UA sets is its own business. What
|
||||
must not happen is that *we* set something, so that is what it asserts
|
||||
now: `min-width` and `min-height` compute to `0px`, and the font-size
|
||||
still equals that of a bare `<button>` probed in the same page. That
|
||||
form catches the `font-size: inherit` regression in either engine —
|
||||
checked by re-introducing it — and it is the same "assert the
|
||||
mechanism" move `queue-as-a-screen.spec.ts` makes about containment.
|
||||
|
||||
It is also the second time in two sessions that **CI's WebKit was the
|
||||
only tier that could see something**, which is the argument for checking
|
||||
that step ran rather than trusting the run's conclusion.
|
||||
|
||||
**A rule at the bottom of `index.css` still loses to a nested rule
|
||||
above it.** The phone block is last on purpose because a media query
|
||||
adds no specificity — but `#queue-button` is written *nested* inside
|
||||
`.bottom-bar`, so it builds to a descendant selector one class more
|
||||
specific, and a bare `#queue-button { display: none }` in the phone
|
||||
block did nothing at all. Silently: the button simply stayed. Nesting
|
||||
adds specificity the source does not show.
|
||||
|
||||
**Removing a control moved the question of how you reach what is left,
|
||||
and ten specs were quietly asserting the old answer.** Hiding the bar's
|
||||
queue button failed ten tests in four files about the back stack and
|
||||
about layout, every one of which opened the queue by clicking
|
||||
`#queue-button`. `openTheQueue` in `e2e/support/fixtures.ts` is the
|
||||
route *this viewport* offers, and the fix was to stop hard-coding one.
|
||||
|
||||
**And the route it takes did not exist in the state that matters.**
|
||||
`now-playing` renders two branches, and the no-track one had no
|
||||
`.expand` button — so with nothing loaded there was no way to Now
|
||||
Playing, and once the queue button left the bar the queue was
|
||||
unreachable outright. The queue is persisted across restarts, so this
|
||||
is a state the app launches into, not a corner. It first appeared as a
|
||||
*flake* (#168: the long-lived e2e app meant whether a track was loaded
|
||||
depended on which spec ran first), which is worth remembering — a leak
|
||||
made a deterministic bug look like a race.
|
||||
|
||||
## Now Playing does not fit a 439px screen, and #56 makes that visible (measured 2026-08-21)
|
||||
|
||||
Two separate things, and only the first is a defect.
|
||||
|
||||
**The art overflowed its own box and drew over the header and the
|
||||
title.** It is `width: min(100%, 60vh); aspect-ratio: 1`, so its height
|
||||
is derived from its width and bounded by nothing — 60vh bounds the
|
||||
*viewport*, not the room left over, and those differ by all the chrome
|
||||
above and below. `max-height: 100%` is the fix and shipped with #56.
|
||||
Pre-existing: screenshotted on `main`. **Found by reading a screenshot,
|
||||
which is the only tier that can see it** — nothing fails, the shell does
|
||||
not overflow, and every control is still hittable.
|
||||
|
||||
**With that fixed, the art is a 39px sliver**, because the transport is
|
||||
now 172px of a 439px screen. That is a consequence of #56 rather than a
|
||||
fault in it, and it is filed as #172 with the per-element budget. #64
|
||||
(no in-app volume on Android) is ~30px of pure gain there and #51 is the
|
||||
umbrella; folding shuffle and repeat back onto the primary row was
|
||||
considered and rejected — it buys 52px, leaves the art at 91px, and
|
||||
costs a third arrangement of the same five buttons.
|
||||
|
||||
@@ -1728,6 +1728,55 @@ rather than after the state, because with the slider beside it there is
|
||||
nothing left to disclose. It stands down below 600px whatever the
|
||||
setting says — that is about the platform rather than preference, and
|
||||
is why `mediacontrols`' Android handler implements no volume callback.
|
||||
(Only the *bar's* copy: `now-playing-view` renders one and it is
|
||||
visible on a phone. #64 asks for it to be gone on Android outright,
|
||||
which is a platform question the frontend cannot currently ask.)
|
||||
|
||||
**And below 600px that bar carries three controls, not five** (#59).
|
||||
Shuffle, repeat and the queue button leave it; what is left is art,
|
||||
title/artist, favourite, and prev/play/next. `player-controls` is one
|
||||
component in two places and **the context is a property rather than a
|
||||
media query**, which is the exception to the rule two paragraphs down:
|
||||
on a phone the bar wants three controls and `now-playing-view` wants
|
||||
five, larger still, *at the same viewport* — so the host states the
|
||||
context and the viewport states the size band, and neither alone can
|
||||
express it. Sizes come from `--yj-control-*` custom properties set per
|
||||
context; play/pause alone goes above the 44px floor, because a row of
|
||||
identical squares says every action is equally likely and that is not
|
||||
true of play. Measured before #56: every one of them was **33×21px**,
|
||||
and the mini bar's favourite was **18×14**, the smallest control in the
|
||||
app.
|
||||
|
||||
Four things about it are load-bearing.
|
||||
|
||||
**The phone draws three buttons rather than hiding two**, from
|
||||
`matchMedia` — `job-band`'s pattern, and the rule that a decision about
|
||||
whether an element *exists* is not a stylesheet's to make. A
|
||||
`display: none` control is still in the shadow root and still something
|
||||
a positional query finds, so "the phone has three controls" would have
|
||||
been true of the pixels and false of the element.
|
||||
|
||||
**Removing a control is only allowed because it is still reachable.**
|
||||
Plan 018's matrix promises no action is unreachable at any supported
|
||||
size, and all three are on `now-playing-view`, one tap away through the
|
||||
mini player's art. That promise is what `phone-transport.spec.ts`
|
||||
asserts — it walks the route — rather than counting buttons.
|
||||
|
||||
**So the route to Now Playing must not depend on what is playing**, and
|
||||
it did. `now-playing` renders two branches and the no-track one had no
|
||||
`.expand` button on its placeholder, so with nothing loaded there was
|
||||
no way to the full-screen view — which, once the queue button left the
|
||||
bar, made the *queue* unreachable. The queue is persisted across
|
||||
restarts, so "tracks queued, nothing playing" is a state the app
|
||||
launches into.
|
||||
|
||||
**The desktop bar is untouched and a spec says so with a literal.**
|
||||
Both issues are `Platform/Android`. The trap is that a `<button>` does
|
||||
not inherit its font from its parent — the UA stylesheet gives it one —
|
||||
so a generic `font-size: inherit` is not the no-op it reads as: it took
|
||||
every desktop button from 33×21 to 36×24, silently. The sizes are
|
||||
asserted as `'33x21'` rather than as a range, because the regression
|
||||
was three pixels.
|
||||
|
||||
**900 is the worst desktop width, not the 800×600 minimum.** The
|
||||
sidebar collapses to icons *below* 900, so the main panel is 843px at
|
||||
@@ -1775,6 +1824,61 @@ along untouched. Escape closes it and returns focus, and is attached
|
||||
only while the overlay is up — it is a dismissal, not a shortcut, which
|
||||
is why it is not a panel-scoped binding.
|
||||
|
||||
**And an overlaid queue is a place, which is the whole of #55.** The
|
||||
pixels were already right: measured at the reference device's 424×439,
|
||||
the overlaid panel is 424×318 — `.main-panel`'s rect exactly — so a
|
||||
`DETAIL_LOADERS` mount would draw the same rectangle in the same spot.
|
||||
What was missing was the navigation model, and the defect was one
|
||||
measurement: opening the queue on Artists and pressing back moved the
|
||||
page *underneath* to Albums and left the queue up. So opening an
|
||||
**overlay** queue dispatches `navigate {view: 'queue'}` and opening a
|
||||
**column** sets the attribute as it always did — `utils/open-queue.ts`
|
||||
is that one decision, and both routes end at the same `open` attribute
|
||||
on the same element.
|
||||
|
||||
Five things about it are load-bearing.
|
||||
|
||||
**The queue is a screen exactly while it is an overlay**, which is the
|
||||
rule above rather than a second one: a column is a thing the user
|
||||
docked, so back must not undock it and a navigation must not take it
|
||||
away, while an overlay is covering the content and has to answer the
|
||||
platform's gesture. That also inherits the *computed, not
|
||||
breakpointed* property for free — the panel is drag-resizable, so a
|
||||
viewport breakpoint would be wrong by up to 180px.
|
||||
|
||||
**It is in neither `VIEW_TAGS` nor `DETAIL_LOADERS`**, because there is
|
||||
nothing to mount; the panel is already in the document. That is not
|
||||
tidiness. `.main-panel > *` is paint-contained under a `.main-panel`
|
||||
that is, and `contain: paint` clips the `position: fixed` a `wa-popup`
|
||||
falls back to on the reference device's Chrome 113 (#60) — so the
|
||||
detail-view mount asked for in #55's Direction would have broken
|
||||
`queue-panel`'s working context menu on the one device the issue is
|
||||
about. Measured: the panel's ancestry is `layout style` all the way to
|
||||
`body`; a view inside the main panel is `content` under `content`.
|
||||
**No tier here can see that consequence** — CI's Chromium and WebKit
|
||||
both have the Popover API — so `queue-as-a-screen.spec.ts` asserts the
|
||||
*mechanism*, that the panel is not under a paint-contained ancestor.
|
||||
|
||||
**A navigation to `queue` deliberately writes neither
|
||||
`dataset.activeView` nor `searchStore.setCurrentView`**, because both
|
||||
describe what is *in* the main panel and the queue covers that panel
|
||||
without replacing it. It publishes itself through `activeViewStore`
|
||||
with `isPrimary: false`, so the tab it was opened from stays lit —
|
||||
the same rule a detail view gets.
|
||||
|
||||
**The entry is unwound from the panel's `open` attribute**, in the
|
||||
mutation observer `index.ts` already ran for `aria-expanded`, rather
|
||||
than at each of the four ways out. Escape, the scrim, the close button
|
||||
and the toggle all take that route, and a fifth added later gets it
|
||||
free. Without it the entry is orphaned and the *next* back press is the
|
||||
one that closes the queue — the reported defect moved one press later,
|
||||
which looks exactly like a press that did nothing.
|
||||
|
||||
**And the way out is 44px on a phone.** With the panel spanning the
|
||||
whole width the scrim has no uncovered pixels at all, so the close
|
||||
button is the only pointer route out of a full-screen surface; it was
|
||||
**25×21px**.
|
||||
|
||||
What this does **not** fix is `page-header` overflowing on its own:
|
||||
at 900×600 "New Smart Playlist" is still clipped to 114 of 162px with
|
||||
the queue *closed*. That is #69, and it cannot be fixed in
|
||||
|
||||
@@ -0,0 +1,315 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* The phone's transport (#59, #56).
|
||||
*
|
||||
* #56 reports that "the playback controls are the most important thing
|
||||
* in the mobile app and they are tiny". Measured at the reference
|
||||
* device's 424x439 before this, every one of them was **33x21px**, and
|
||||
* the favourite beside them — which #59 keeps on the bar — was
|
||||
* **18x14px**, the smallest control in the app.
|
||||
*
|
||||
* #59 is what makes the sizes affordable: five controls plus a queue
|
||||
* button at 44px does not fit 424 CSS px, so the bar carries three and
|
||||
* the rest are on the full-screen view.
|
||||
*
|
||||
* **The assertion that matters is not the pixel count.** Plan 018's
|
||||
* matrix promises that *no action is ever unreachable at any supported
|
||||
* size*, and #59 removes three controls from the phone's bar — so the
|
||||
* first thing this file checks is that all three are still reachable,
|
||||
* by walking the route a user would. A spec that only measured the
|
||||
* survivors would be green on a build that had made shuffle
|
||||
* unreachable, which is the failure mode this pair of issues is one
|
||||
* mistake away from.
|
||||
*/
|
||||
type Page = import('@playwright/test').Page;
|
||||
|
||||
/** The reference device's real viewport. */
|
||||
const DEVICE = { width: 424, height: 439 };
|
||||
const PHONE = { width: 390, height: 780 };
|
||||
const DESKTOP = { width: 1280, height: 800 };
|
||||
|
||||
/**
|
||||
* The touch-target floor. 44px is what #56's Findings name and what
|
||||
* #55's queue header was sized to, so the app has one number.
|
||||
*/
|
||||
const TARGET = 44;
|
||||
|
||||
/** The play button is named for its action, not its identity. */
|
||||
const PLAY_PAUSE = /^(Play|Pause)$/;
|
||||
|
||||
const barControls = (page: Page) =>
|
||||
page.locator('audio-player player-controls');
|
||||
|
||||
/**
|
||||
* `name` may be a regex, and for play/pause it must be: that button is
|
||||
* named for the *action*, so it is "Pause" while a track runs and
|
||||
* "Play" when it stops. An exact 'Play' made these tests wait out a
|
||||
* fixture track (11.1s each, passing by luck) and would have failed
|
||||
* outright against `LONG_TRACK`. A test about a control's size does not
|
||||
* care what the transport is doing.
|
||||
*/
|
||||
async function sizeOf(
|
||||
page: Page,
|
||||
name: string | RegExp,
|
||||
): Promise<[number, number]> {
|
||||
const box = await page
|
||||
.getByRole('button', { name, exact: typeof name === 'string' })
|
||||
.boundingBox();
|
||||
|
||||
expect(box, `no button named ${name}`).not.toBeNull();
|
||||
|
||||
return [box!.width, box!.height];
|
||||
}
|
||||
|
||||
/** Put something in the queue, so the transport has a track to act on. */
|
||||
async function stageATrack(page: Page): Promise<void> {
|
||||
await page.evaluate(async () => {
|
||||
const tracks = (await window.__yjEvents.call(
|
||||
'library.Library.GetTracks',
|
||||
[0],
|
||||
10_000,
|
||||
)) as { FilePath: string }[];
|
||||
|
||||
await window.__yjEvents.call(
|
||||
'queue.Queue.SetQueue',
|
||||
[tracks.slice(0, 4).map((t) => t.FilePath), 0, false, { type: '', id: 0, label: '' }],
|
||||
10_000,
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
test.describe('the phone bar carries three controls', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(DEVICE);
|
||||
await stageATrack(app);
|
||||
});
|
||||
|
||||
test('drops shuffle, repeat and the queue from the bar', async ({ app }) => {
|
||||
const bar = barControls(app);
|
||||
|
||||
await expect(bar.getByRole('button', { name: 'Previous track' })).toBeVisible();
|
||||
await expect(bar.getByRole('button', { name: 'Next track' })).toBeVisible();
|
||||
|
||||
// Not in the bar's own subtree. Asserted against the bar rather
|
||||
// than the page, because the whole point is that they moved rather
|
||||
// than went away -- a page-wide `not.toBeVisible()` would fail the
|
||||
// moment Now Playing is open and would be asserting the wrong
|
||||
// thing besides.
|
||||
await expect(bar.getByRole('button', { name: 'Shuffle' })).toHaveCount(0);
|
||||
await expect(bar.getByRole('button', { name: /^Repeat/ })).toHaveCount(0);
|
||||
await expect(app.locator('#queue-button')).toBeHidden();
|
||||
});
|
||||
|
||||
/**
|
||||
* The promise, walked. Every control #59 takes off the bar is
|
||||
* reachable from the mini player's art in one tap.
|
||||
*/
|
||||
test('leaves every removed control reachable from Now Playing', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('open-now-playing').click();
|
||||
|
||||
await expect(app.getByTestId('main-content')).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'now-playing',
|
||||
);
|
||||
|
||||
await expect(app.getByRole('button', { name: 'Shuffle' })).toBeVisible();
|
||||
await expect(app.getByRole('button', { name: /^Repeat/ })).toBeVisible();
|
||||
await expect(app.getByRole('button', { name: 'Show the queue' })).toBeVisible();
|
||||
});
|
||||
|
||||
test('sizes what is left for a thumb', async ({ app }) => {
|
||||
for (const name of ['Previous track', 'Next track']) {
|
||||
const [w, h] = await sizeOf(app, name);
|
||||
|
||||
expect(w, `${name} width`).toBeGreaterThanOrEqual(TARGET);
|
||||
expect(h, `${name} height`).toBeGreaterThanOrEqual(TARGET);
|
||||
}
|
||||
|
||||
// Play is deliberately bigger than its neighbours: a row of
|
||||
// identical squares says every action is equally likely, which is
|
||||
// not true of play.
|
||||
const [pw, ph] = await sizeOf(app, PLAY_PAUSE);
|
||||
const [nw] = await sizeOf(app, 'Next track');
|
||||
|
||||
expect(ph).toBeGreaterThanOrEqual(TARGET);
|
||||
expect(pw).toBeGreaterThan(nw);
|
||||
});
|
||||
|
||||
/**
|
||||
* The favourite was 18x14 and is one of the three controls #59
|
||||
* keeps, so it is part of this issue rather than a nicety.
|
||||
*/
|
||||
test('sizes the favourite, which was the smallest control in the app', async ({
|
||||
app,
|
||||
}) => {
|
||||
const fav = app
|
||||
.locator('now-playing')
|
||||
.getByRole('button', { name: /Favorites$/ });
|
||||
|
||||
const box = await fav.boundingBox();
|
||||
|
||||
expect(box).not.toBeNull();
|
||||
expect(box!.width).toBeGreaterThanOrEqual(TARGET);
|
||||
expect(box!.height).toBeGreaterThanOrEqual(TARGET);
|
||||
});
|
||||
|
||||
/**
|
||||
* **The route to the queue must not depend on what is playing.**
|
||||
*
|
||||
* `now-playing` renders two branches, and the no-track one had no
|
||||
* `.expand` button on its placeholder — so with nothing loaded there
|
||||
* was no way to Now Playing, and once #59 takes the queue button off
|
||||
* the bar that makes the *queue* unreachable. The queue is persisted
|
||||
* across restarts, so "tracks queued, nothing playing" is a state the
|
||||
* app launches into.
|
||||
*
|
||||
* This is asserted with the queue explicitly emptied rather than by
|
||||
* relying on the app not having played anything: `make e2e` runs one
|
||||
* long-lived app across every spec file (#168), so "no track loaded"
|
||||
* is otherwise whatever the file before this one left behind — which
|
||||
* is how the underlying fault first showed up as a flake in a spec
|
||||
* about something else.
|
||||
*/
|
||||
test('reaches the queue with nothing playing', async ({ app }) => {
|
||||
await app.evaluate(async () => {
|
||||
await window.__yjEvents.call('queue.Queue.Clear', [], 10_000);
|
||||
});
|
||||
|
||||
await expect(app.getByTestId('open-now-playing')).toBeVisible();
|
||||
|
||||
await app.getByTestId('open-now-playing').click();
|
||||
await app.getByTestId('npv-queue').click();
|
||||
|
||||
await expect(app.locator('#queue-panel')).toHaveAttribute('open', '');
|
||||
});
|
||||
|
||||
test('still fits, with nothing to scroll sideways to', async ({ app }) => {
|
||||
const fit = await app.evaluate(() => ({
|
||||
scroll: document.body.scrollWidth,
|
||||
client: document.body.clientWidth,
|
||||
}));
|
||||
|
||||
expect(fit.scroll).toBe(fit.client);
|
||||
});
|
||||
});
|
||||
|
||||
test.describe('the full-screen transport is the page', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(DEVICE);
|
||||
await stageATrack(app);
|
||||
await app.getByTestId('open-now-playing').click();
|
||||
});
|
||||
|
||||
test('draws all five, larger than the bar draws any', async ({ app }) => {
|
||||
const [pw, ph] = await sizeOf(app, PLAY_PAUSE);
|
||||
|
||||
expect(pw).toBeGreaterThanOrEqual(56);
|
||||
expect(ph).toBeGreaterThanOrEqual(56);
|
||||
|
||||
for (const name of ['Shuffle', 'Previous track', 'Next track']) {
|
||||
const [w, h] = await sizeOf(app, name);
|
||||
|
||||
expect(w, `${name} width`).toBeGreaterThanOrEqual(TARGET);
|
||||
expect(h, `${name} height`).toBeGreaterThanOrEqual(TARGET);
|
||||
}
|
||||
});
|
||||
|
||||
test('fits at both phone widths', async ({ app }) => {
|
||||
for (const size of [DEVICE, PHONE]) {
|
||||
await app.setViewportSize(size);
|
||||
|
||||
const fit = await app.evaluate(() => ({
|
||||
scroll: document.body.scrollWidth,
|
||||
client: document.body.clientWidth,
|
||||
}));
|
||||
|
||||
expect(fit.scroll, `${size.width}px`).toBe(fit.client);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* **The desktop bar is not what either issue is about, and must not
|
||||
* move.** Both are `Platform/Android`; this is the guard that says so
|
||||
* in a way a build can check.
|
||||
*
|
||||
* It caught a real regression while it was being written: a generic
|
||||
* `font-size` on the buttons took them from the UA stylesheet's 13.3px
|
||||
* to the shell's 16px and grew every one from 33x21 to 36x24 — a
|
||||
* change nobody asked for, invisible to every other assertion here.
|
||||
*/
|
||||
test.describe('the desktop bar is untouched', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(DESKTOP);
|
||||
await stageATrack(app);
|
||||
});
|
||||
|
||||
test('keeps all five controls and the queue button', async ({ app }) => {
|
||||
const bar = barControls(app);
|
||||
|
||||
for (const name of ['Shuffle', 'Previous track', 'Next track']) {
|
||||
await expect(bar.getByRole('button', { name })).toBeVisible();
|
||||
}
|
||||
|
||||
await expect(bar.getByRole('button', { name: /^Repeat/ })).toBeVisible();
|
||||
await expect(app.locator('#queue-button')).toBeVisible();
|
||||
});
|
||||
|
||||
/**
|
||||
* **The mechanism, because the pixels are the engine's.**
|
||||
*
|
||||
* The first version of this asserted the literal `'33x21'`, measured
|
||||
* on `main` in Chromium — and WebKit draws the same button **36x24**,
|
||||
* so it failed in CI on a build where nothing was wrong. A button's
|
||||
* box comes from the UA stylesheet when the author sets nothing, and
|
||||
* what each UA sets is its own business.
|
||||
*
|
||||
* What this PR must not do is *set* anything here, so that is what is
|
||||
* asserted: our two box properties are unset, and the font is still
|
||||
* the UA's rather than the shell's. That is precisely the regression
|
||||
* this caught the first time — a generic `font-size: inherit` took
|
||||
* these from the UA's default to 16px — and it catches it in either
|
||||
* engine.
|
||||
*/
|
||||
test('sets no size of its own on the desktop bar', async ({ app }) => {
|
||||
const measured = await barControls(app).evaluate((el) => {
|
||||
// A bare button with no author styles: whatever this engine
|
||||
// gives one is what the bar's buttons must still be.
|
||||
const probe = document.createElement('button');
|
||||
|
||||
document.body.appendChild(probe);
|
||||
|
||||
const uaFontSize = getComputedStyle(probe).fontSize;
|
||||
|
||||
probe.remove();
|
||||
|
||||
return [...el.shadowRoot!.querySelectorAll('button')].map((b) => {
|
||||
const cs = getComputedStyle(b);
|
||||
const r = b.getBoundingClientRect();
|
||||
|
||||
return {
|
||||
minWidth: cs.minWidth,
|
||||
minHeight: cs.minHeight,
|
||||
usesUaFont: cs.fontSize === uaFontSize,
|
||||
size: `${Math.round(r.width)}x${Math.round(r.height)}`,
|
||||
};
|
||||
});
|
||||
});
|
||||
|
||||
expect(measured).toHaveLength(5);
|
||||
|
||||
for (const m of measured) {
|
||||
expect(m.minWidth, 'min-width').toBe('0px');
|
||||
expect(m.minHeight, 'min-height').toBe('0px');
|
||||
expect(m.usesUaFont, 'font-size is still the UA default').toBe(true);
|
||||
}
|
||||
|
||||
// And all five are the same box: `.play` takes a larger size in
|
||||
// both sized contexts, so this is what says the desktop is neither
|
||||
// of them.
|
||||
expect(new Set(measured.map((m) => m.size)).size).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,346 @@
|
||||
import { test, expect, openTheQueue } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* #55 — the queue is a *place* while it covers the content, and a
|
||||
* *control* while it sits beside it.
|
||||
*
|
||||
* #24 already made the pixels right: measured at the reference device's
|
||||
* 424×439, the overlaid panel is 424×318, which is `.main-panel`'s rect
|
||||
* exactly. What was missing was the navigation model, and the defect was
|
||||
* measurable in one line — opening the queue on Artists and pressing
|
||||
* back moved the page *underneath* to Albums and left the queue up. A
|
||||
* back press that changes something the user cannot see, and costs them
|
||||
* their place, is the whole of "it does not flow".
|
||||
*
|
||||
* **These assert the entry, not the attribute.** The temptation is to
|
||||
* check `#queue-button[aria-expanded]` and stop, which is the shell's
|
||||
* own bookkeeping and was right throughout the bug: what has to be true
|
||||
* is that *one* back press closes the queue and the *next* one
|
||||
* navigates. Asserting only the first would pass on a build that
|
||||
* orphans the entry, which is the defect moved one press later — the
|
||||
* same trap `back-navigation.spec.ts` documents about `data-active-view`
|
||||
* and `layout-overflow.spec.ts` set for #69.
|
||||
*
|
||||
* **Three of these nine fail on the build before #55**, and the other
|
||||
* six cannot, which is worth knowing before trusting them: "the entry
|
||||
* is not orphaned" and "the column is not in the stack" are both
|
||||
* vacuously true of a build that pushes no entry at all, and the
|
||||
* containment assertion pins the mount that was *not* taken. They guard
|
||||
* the next change rather than reproducing this one — the three that
|
||||
* reproduce it are the two back-press tests and the touch target.
|
||||
*/
|
||||
type Page = import('@playwright/test').Page;
|
||||
|
||||
/** The reference device's real viewport, not a resized desktop. */
|
||||
const DEVICE = { width: 424, height: 439 };
|
||||
|
||||
/** Wide enough that the queue is a column: 1280 − 200 − 320 ≥ 480. */
|
||||
const DESKTOP = { width: 1280, height: 800 };
|
||||
|
||||
/**
|
||||
* The Compact band, where the queue is a *screen* (644 − 320 < 480) and
|
||||
* the bottom bar still carries its button.
|
||||
*
|
||||
* Two of these tests need both facts at once and only this band has
|
||||
* them: below 600px #59 takes the button off the bar, so there is no
|
||||
* toggle to re-press and the queue is opened from Now Playing — which
|
||||
* is itself a detail view, so "the destination stays lit" is vacuously
|
||||
* true there rather than tested.
|
||||
*/
|
||||
const COMPACT = { width: 700, height: 600 };
|
||||
|
||||
const activeView = (page: Page) => page.getByTestId('main-content');
|
||||
const queue = (page: Page) => page.locator('#queue-panel');
|
||||
const toggle = (page: Page) => page.locator('#queue-button');
|
||||
|
||||
/**
|
||||
* Whether the queue is up.
|
||||
*
|
||||
* The panel's own attribute rather than the toggle's `aria-expanded`,
|
||||
* because below 600px there is no toggle to ask (#59) — and the panel
|
||||
* is the one fact both of them reflect anyway.
|
||||
*/
|
||||
async function expectQueue(page: Page, open: boolean): Promise<void> {
|
||||
const panel = queue(page);
|
||||
|
||||
if (open) {
|
||||
await expect(panel).toHaveAttribute('open', '');
|
||||
} else {
|
||||
await expect(panel).not.toHaveAttribute('open', '');
|
||||
}
|
||||
}
|
||||
|
||||
test.describe('the queue is a screen where it covers the content', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(DEVICE);
|
||||
await app.getByTestId('tab-albums').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
});
|
||||
|
||||
// On a phone the queue is opened from Now Playing (#59), so the page
|
||||
// *underneath* it is `now-playing` and the journey is two entries
|
||||
// deep: albums -> now-playing -> queue. That is the real route a user
|
||||
// takes, which is why these do not reach for the shortcut.
|
||||
|
||||
test('back closes the queue and leaves the page where it was', async ({
|
||||
app,
|
||||
}) => {
|
||||
await expect(queue(app)).toHaveAttribute('overlay', '');
|
||||
|
||||
await openTheQueue(app);
|
||||
await expectQueue(app, true);
|
||||
|
||||
await app.goBack();
|
||||
|
||||
await expectQueue(app, false);
|
||||
// The page underneath is untouched. Before #55 this was the
|
||||
// *previous* view, because the queue was not in the stack at all
|
||||
// and back spent an entry navigating something nobody could see.
|
||||
await expect(activeView(app)).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'now-playing',
|
||||
);
|
||||
});
|
||||
|
||||
test('costs exactly one entry, so the next press navigates', async ({
|
||||
app,
|
||||
}) => {
|
||||
await openTheQueue(app);
|
||||
await expectQueue(app, true);
|
||||
|
||||
await app.goBack();
|
||||
await expectQueue(app, false);
|
||||
await expect(activeView(app)).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'now-playing',
|
||||
);
|
||||
|
||||
await app.goBack();
|
||||
|
||||
// Exactly one entry each: the second press leaves Now Playing for
|
||||
// the page it was opened from, rather than being swallowed by a
|
||||
// queue that had already closed.
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
});
|
||||
|
||||
/**
|
||||
* Every route out unwinds the entry, and they do it through the
|
||||
* panel's own `open` attribute rather than each knowing about
|
||||
* history — which is why a fourth route added later gets this free.
|
||||
*
|
||||
* The failure this pins is silent: close by button, and if the entry
|
||||
* is orphaned the app looks correct until the next back press does
|
||||
* nothing at all. It is a guard rather than a reproduction — a build
|
||||
* with no entry to orphan passes it — and it is paired with the two
|
||||
* above, which do reproduce.
|
||||
*/
|
||||
for (const [name, dismiss] of [
|
||||
[
|
||||
'the close button',
|
||||
async (app: Page) => {
|
||||
await app.getByRole('button', { name: 'Close queue' }).click();
|
||||
},
|
||||
],
|
||||
[
|
||||
'Escape',
|
||||
async (app: Page) => {
|
||||
await app.keyboard.press('Escape');
|
||||
},
|
||||
],
|
||||
] as Array<[string, (app: Page) => Promise<void>]>) {
|
||||
test(`${name} leaves no entry behind`, async ({ app }) => {
|
||||
await openTheQueue(app);
|
||||
await expectQueue(app, true);
|
||||
|
||||
await dismiss(app);
|
||||
await expectQueue(app, false);
|
||||
|
||||
await app.goBack();
|
||||
|
||||
// One press, one screen: Now Playing is what the queue was opened
|
||||
// from, so leaving it lands on Albums. An orphaned entry would
|
||||
// have spent this press on nothing and left it here.
|
||||
await expect(activeView(app)).toHaveAttribute(
|
||||
'data-active-view',
|
||||
'albums',
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/**
|
||||
* A detail view leaves the destination it was opened from lit
|
||||
* (`active-view-store`, #72), and the queue inherits that — it is
|
||||
* published with `isPrimary: false`, so `isActive('albums')` is still
|
||||
* true underneath it.
|
||||
*
|
||||
* `aria-current` rather than a class, for the reason
|
||||
* `back-navigation.spec.ts` gives: the class was right throughout the
|
||||
* bug that rule exists for.
|
||||
*/
|
||||
|
||||
/**
|
||||
* With the panel spanning the whole width the scrim has no uncovered
|
||||
* pixels, so the close button is the only pointer route out of a
|
||||
* full-screen surface. Measured at 424×439 before #55: **25×21px**.
|
||||
*/
|
||||
test('offers a way out a thumb can hit', async ({ app }) => {
|
||||
await openTheQueue(app);
|
||||
|
||||
const box = await app
|
||||
.getByRole('button', { name: 'Close queue' })
|
||||
.boundingBox();
|
||||
|
||||
expect(box).not.toBeNull();
|
||||
expect(box!.width).toBeGreaterThanOrEqual(44);
|
||||
expect(box!.height).toBeGreaterThanOrEqual(44);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* **The mechanism, because no tier here can see the consequence.**
|
||||
*
|
||||
* #55's Direction asked for a `DETAIL_LOADERS` mount, which would put
|
||||
* the panel inside `.main-panel > *`. That box is paint-contained under
|
||||
* a `.main-panel` that is too, and `contain: paint` makes an element a
|
||||
* containing block for fixed descendants *and clips them* — which is
|
||||
* what a `wa-popup` falls back to on the reference device's Chrome 113,
|
||||
* where the Popover API does not exist (#60, `.planning/NOTES.md`).
|
||||
* `queue-panel` has a context menu, so that mount would have broken a
|
||||
* working menu on the one device this issue is about.
|
||||
*
|
||||
* CI's Chromium and WebKit both *have* the Popover API, so the menu is
|
||||
* top-layered and correct here either way: a spec asserting "the menu is
|
||||
* not clipped" is green on the broken build. What a browser can answer
|
||||
* honestly is where the element is, so that is what this asks.
|
||||
*/
|
||||
test('the panel stays out of the paint-contained region', async ({ app }) => {
|
||||
await app.setViewportSize(DEVICE);
|
||||
|
||||
// Open, because that is the only state in which a menu can be opened
|
||||
// from it — and because the host drops `paint` from its own
|
||||
// containment deliberately in overlay mode, so a closed panel answers
|
||||
// a different question.
|
||||
await openTheQueue(app);
|
||||
await expectQueue(app, true);
|
||||
|
||||
const ancestry = await app.evaluate(() => {
|
||||
const chain: Array<{ tag: string; contain: string }> = [];
|
||||
|
||||
for (
|
||||
let el = document.getElementById('queue-panel');
|
||||
el && el !== document.documentElement;
|
||||
el = el.parentElement
|
||||
) {
|
||||
chain.push({
|
||||
tag: el.tagName.toLowerCase(),
|
||||
contain: getComputedStyle(el).contain,
|
||||
});
|
||||
}
|
||||
|
||||
return chain;
|
||||
});
|
||||
|
||||
expect(ancestry.length).toBeGreaterThan(1);
|
||||
expect(ancestry.some((a) => a.tag === 'main')).toBe(false);
|
||||
|
||||
for (const { tag, contain } of ancestry) {
|
||||
expect(
|
||||
`${tag}: ${contain}`,
|
||||
'a paint-contained ancestor clips a fixed-positioned popup on Chrome 113',
|
||||
).not.toMatch(/paint|content|strict/);
|
||||
}
|
||||
});
|
||||
|
||||
/**
|
||||
* Two properties need the queue to be a *screen* and the bar to still
|
||||
* have its button, and only the Compact band has both — below 600px #59
|
||||
* takes the button off the bar.
|
||||
*/
|
||||
test.describe('a screen opened from the bar', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(COMPACT);
|
||||
await app.getByTestId('nav-albums').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
await expect(queue(app)).toHaveAttribute('overlay', '');
|
||||
});
|
||||
|
||||
/**
|
||||
* A detail view leaves the destination it was opened from lit
|
||||
* (`active-view-store`, #72), and the queue inherits that — it is
|
||||
* published with `isPrimary: false`, so `isActive('albums')` is still
|
||||
* true underneath it.
|
||||
*
|
||||
* `aria-current` rather than a class, for the reason
|
||||
* `back-navigation.spec.ts` gives: the class was right throughout the
|
||||
* bug that rule exists for.
|
||||
*/
|
||||
test('leaves the destination it was opened from highlighted', async ({
|
||||
app,
|
||||
}) => {
|
||||
// By testid, not by role: at 700px the sidebar is in icon mode, so
|
||||
// what the item is *named* is a different question from which item
|
||||
// it is. The assertion is still `aria-current`, which is the
|
||||
// accessible fact.
|
||||
const albums = app.getByTestId('nav-albums');
|
||||
|
||||
await expect(albums).toHaveAttribute('aria-current', 'page');
|
||||
|
||||
await toggle(app).click();
|
||||
await expectQueue(app, true);
|
||||
|
||||
await expect(albums).toHaveAttribute('aria-current', 'page');
|
||||
});
|
||||
|
||||
/** The toggle is a fourth way out, and it unwinds the entry like the
|
||||
* other three — through the panel's attribute, not its own handler. */
|
||||
test('closes from the same toggle, leaving no entry behind', async ({
|
||||
app,
|
||||
}) => {
|
||||
await toggle(app).click();
|
||||
await expectQueue(app, true);
|
||||
|
||||
await toggle(app).click();
|
||||
await expectQueue(app, false);
|
||||
|
||||
await app.goBack();
|
||||
|
||||
await expect(activeView(app)).not.toHaveAttribute(
|
||||
'data-active-view',
|
||||
'albums',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* The column is not a place. Somebody docked it; back must not undock
|
||||
* it, and navigating to another view must not take it away.
|
||||
*
|
||||
* This is the half a viewport breakpoint would get wrong: the mode is
|
||||
* computed from the panel's own drag-resizable width, so the queue
|
||||
* becomes a screen exactly when it stops being affordable as a column.
|
||||
*/
|
||||
test.describe('a docked queue is not in the back stack', () => {
|
||||
test.beforeEach(async ({ app }) => {
|
||||
await app.setViewportSize(DESKTOP);
|
||||
});
|
||||
|
||||
test('survives a navigation, and back navigates the page', async ({
|
||||
app,
|
||||
}) => {
|
||||
await app.getByTestId('nav-albums').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
|
||||
await toggle(app).click();
|
||||
await expectQueue(app, true);
|
||||
await expect(queue(app)).not.toHaveAttribute('overlay', '');
|
||||
|
||||
await app.getByTestId('nav-artists').click();
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'artists');
|
||||
await expectQueue(app, true);
|
||||
|
||||
await app.goBack();
|
||||
|
||||
await expect(activeView(app)).toHaveAttribute('data-active-view', 'albums');
|
||||
await expectQueue(app, true);
|
||||
});
|
||||
});
|
||||
@@ -1,4 +1,4 @@
|
||||
import { test, expect } from '../support/fixtures.js';
|
||||
import { test, expect, openTheQueue } from '../support/fixtures.js';
|
||||
|
||||
/**
|
||||
* #24 — the queue panel does not take the page's width away from it.
|
||||
@@ -54,15 +54,15 @@ const shellGeometry = (page: import('@playwright/test').Page) =>
|
||||
};
|
||||
});
|
||||
|
||||
async function openQueue(page: import('@playwright/test').Page) {
|
||||
const toggle = page.locator('#queue-button');
|
||||
|
||||
if ((await toggle.getAttribute('aria-expanded')) !== 'true') {
|
||||
await toggle.click();
|
||||
}
|
||||
|
||||
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
|
||||
}
|
||||
/**
|
||||
* Opening the queue is `openTheQueue`, which takes the route this
|
||||
* viewport offers. It used to be a local helper that clicked
|
||||
* `#queue-button` unconditionally, and #59 hid that button below
|
||||
* 600px -- so the two phone bands here failed on a build where the
|
||||
* queue was working perfectly, having been asserting *how* it opens as
|
||||
* much as what it does.
|
||||
*/
|
||||
const openQueue = openTheQueue;
|
||||
|
||||
test.describe('an open queue leaves the content its width', () => {
|
||||
for (const band of BANDS) {
|
||||
|
||||
@@ -140,6 +140,50 @@ export async function navigateTo(page: Page, view: string): Promise<void> {
|
||||
.waitFor({ state: 'attached' });
|
||||
}
|
||||
|
||||
/**
|
||||
* Open the queue the way a user at this viewport would.
|
||||
*
|
||||
* **The route differs by width and that is the feature, not an
|
||||
* inconvenience.** Above 600px the bottom bar carries a queue button.
|
||||
* Below it that button is gone (#59) and the queue is reached from the
|
||||
* full-screen Now Playing view, which the mini player's art opens —
|
||||
* "reachable only from Now Playing", which is what the issue asks for.
|
||||
*
|
||||
* It is here rather than in one spec because four files need it, and
|
||||
* because a spec that hard-codes `#queue-button` is quietly asserting
|
||||
* *which* route exists as well as what the queue does. Four of them
|
||||
* were, which is how hiding one button failed ten tests about
|
||||
* something else.
|
||||
*
|
||||
* The width is read from the page rather than passed, so a caller that
|
||||
* resizes and then opens does not have to say so twice.
|
||||
*/
|
||||
export async function openTheQueue(page: Page): Promise<void> {
|
||||
const toggle = page.locator('#queue-button');
|
||||
|
||||
if (await toggle.isVisible()) {
|
||||
if ((await toggle.getAttribute('aria-expanded')) !== 'true') {
|
||||
await toggle.click();
|
||||
}
|
||||
|
||||
await expect(toggle).toHaveAttribute('aria-expanded', 'true');
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
// The phone: through Now Playing. `open-now-playing` is the mini
|
||||
// player's art, which is a button only below 600px.
|
||||
if (
|
||||
(await page.getByTestId('main-content').getAttribute('data-active-view')) !==
|
||||
'now-playing'
|
||||
) {
|
||||
await page.getByTestId('open-now-playing').click();
|
||||
}
|
||||
|
||||
await page.getByTestId('npv-queue').click();
|
||||
await expect(page.locator('#queue-panel')).toHaveAttribute('open', '');
|
||||
}
|
||||
|
||||
/** Thin client for the dev-only /__test/ surface (backend/testctl). */
|
||||
export class TestCtl {
|
||||
constructor(private readonly baseURL: string) {}
|
||||
|
||||
@@ -535,6 +535,30 @@ body div.sidebar {
|
||||
.bottom-bar volume-control {
|
||||
display: none;
|
||||
}
|
||||
|
||||
/* The queue leaves the phone's bar (#59), because #55 made it a
|
||||
screen with an entry in the back stack and Now Playing already
|
||||
carries its own button for it. The route is the mini player's
|
||||
art -> Now Playing -> the queue, which is the "reachable only
|
||||
from Now Playing" this issue asks for.
|
||||
|
||||
This is allowed to remove a control only because the control is
|
||||
still reachable: plan 018's matrix promises that no action is
|
||||
ever unreachable at any supported size, and that promise is what
|
||||
`phone-transport.spec.ts` asserts rather than the button count.
|
||||
|
||||
**`.bottom-bar #queue-button`, not `#queue-button`**, and that is
|
||||
not decoration. The rule this overrides is written *nested*
|
||||
inside `.bottom-bar`, so it builds to a descendant selector one
|
||||
class more specific than it looks in the source -- and a bare
|
||||
`#queue-button` here loses to it, media query or not. Being last
|
||||
in the file is not enough when the thing above is more specific,
|
||||
which is the same lesson as this section's own header one level
|
||||
down: nesting adds specificity the source does not show, and the
|
||||
failure is silent (the button simply stayed). */
|
||||
.bottom-bar #queue-button {
|
||||
display: none;
|
||||
}
|
||||
}
|
||||
|
||||
/* Out of the desktop grid entirely. `job-band` renders nothing above
|
||||
|
||||
+89
-6
@@ -67,6 +67,7 @@ import '@store/theme-store';
|
||||
import './src/services/keyboard-shortcut-service';
|
||||
import { activateView, deactivateView } from '@utils/view-lifecycle';
|
||||
import { installLongPressContextMenu } from '@utils/long-press';
|
||||
import { openQueue, queuePanelElement } from '@utils/open-queue';
|
||||
import { installTopBarFit } from './src/services/top-bar-fit';
|
||||
import {
|
||||
hasTrackPayload,
|
||||
@@ -336,6 +337,33 @@ window.addEventListener('popstate', (e: PopStateEvent) => {
|
||||
void handleNavigate({ ...nav, _isBack: true });
|
||||
});
|
||||
|
||||
/**
|
||||
* The queue, while it is a screen (#55).
|
||||
*
|
||||
* It is *not* in `VIEW_TAGS` and *not* in `DETAIL_LOADERS`: there is
|
||||
* nothing to mount, because the panel is already in the document and,
|
||||
* as an overlay, already occupies `.main-panel`'s rect exactly. What a
|
||||
* navigation adds is the two things that make a screen a screen — a
|
||||
* history entry, so the platform's back gesture answers it, and a
|
||||
* destination to leave, so navigating anywhere else takes it away.
|
||||
*
|
||||
* Keeping it out of both tables is what keeps its context menu working
|
||||
* on the reference device: `.main-panel > *` is paint-contained and a
|
||||
* `wa-popup` falls back to `position: fixed` on Chrome 113, which
|
||||
* escapes overflow but not containment (#60). The panel stays in
|
||||
* `.content-area`, which is not paint-contained, exactly as it is
|
||||
* today.
|
||||
*/
|
||||
const QUEUE_VIEW = 'queue';
|
||||
|
||||
/** Close a queue that is being navigated away from. A *column* is not
|
||||
* a place, so it survives a navigation the way the sidebar does. */
|
||||
function dismissQueueScreen(): void {
|
||||
const panel = queuePanelElement();
|
||||
|
||||
if (panel?.hasAttribute('overlay')) panel.removeAttribute('open');
|
||||
}
|
||||
|
||||
async function handleNavigate(
|
||||
detail: { view: string; [key: string]: any },
|
||||
): Promise<void> {
|
||||
@@ -347,6 +375,25 @@ async function handleNavigate(
|
||||
|
||||
if (!detail._isBack) recordNavigation(detail);
|
||||
|
||||
if (view === QUEUE_VIEW) {
|
||||
// The shell says where the user is; `false` because the queue is
|
||||
// not a primary view, so nothing in either nav lights while it
|
||||
// is up -- the same rule a detail view gets, and the reason the
|
||||
// tab the queue was opened from stays lit.
|
||||
activeViewStore.setView(view, false);
|
||||
queuePanelElement()?.setAttribute('open', '');
|
||||
|
||||
// Deliberately not `searchStore.setCurrentView` and not
|
||||
// `dataset.activeView`: both describe what is *in the main
|
||||
// panel*, and the queue covers that panel without replacing it.
|
||||
// Overwriting either would disable the search box belonging to
|
||||
// the page underneath and make every `data-active-view`
|
||||
// selector in the suite disagree with the element it names.
|
||||
return;
|
||||
}
|
||||
|
||||
dismissQueueScreen();
|
||||
|
||||
// Bookkeeping stays synchronous with the click: the search box's
|
||||
// scope and the active-view attribute describe the navigation that
|
||||
// was *asked for*, and are what the rest of the app and the e2e
|
||||
@@ -631,13 +678,17 @@ const queuePanel = document.getElementById('queue-panel') as HTMLElement | null;
|
||||
|
||||
if (queueButton && queuePanel) {
|
||||
queueButton.addEventListener('click', () => {
|
||||
const isOpen = queuePanel.hasAttribute('open');
|
||||
|
||||
if (isOpen) {
|
||||
if (queuePanel.hasAttribute('open')) {
|
||||
// Closing goes through the panel either way; where the queue
|
||||
// is a screen the observer below is what unwinds its history
|
||||
// entry, so this button, Escape, the scrim and the close
|
||||
// button all take the same route out.
|
||||
queuePanel.removeAttribute('open');
|
||||
} else {
|
||||
queuePanel.setAttribute('open', '');
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
openQueue();
|
||||
});
|
||||
|
||||
// The button says whether the panel is open, and it learns that
|
||||
@@ -655,7 +706,39 @@ if (queueButton && queuePanel) {
|
||||
);
|
||||
};
|
||||
|
||||
new MutationObserver(reflectQueueState).observe(queuePanel, {
|
||||
/**
|
||||
* Keep the back stack honest about a queue that closed itself.
|
||||
*
|
||||
* Where the queue is a screen its `open` attribute and the current
|
||||
* history entry are two statements of one fact, and the panel can
|
||||
* change its half on its own -- Escape, the scrim, the close button,
|
||||
* and anything added later. Reconciling here rather than at each of
|
||||
* those is the same reason this observer already exists for
|
||||
* `aria-expanded`: the attribute is the one fact, and a state kept
|
||||
* beside a click is right until something else changes it.
|
||||
*
|
||||
* Without this the entry is orphaned and the *next* back press is
|
||||
* the one that closes the queue -- a press that appears to do
|
||||
* nothing, which is the defect this issue is about, moved one press
|
||||
* later.
|
||||
*
|
||||
* `history.back()` rather than a stack of our own, for the reason
|
||||
* `navigate-back` does: two stacks is how a component's own way out
|
||||
* and the phone's gesture come to disagree about what one press
|
||||
* means.
|
||||
*/
|
||||
const reconcileQueueHistory = () => {
|
||||
if (queuePanel.hasAttribute('open')) return;
|
||||
|
||||
const state = history.state as NavState | null;
|
||||
|
||||
if (state?.yjNav?.view === QUEUE_VIEW) history.back();
|
||||
};
|
||||
|
||||
new MutationObserver(() => {
|
||||
reflectQueueState();
|
||||
reconcileQueueHistory();
|
||||
}).observe(queuePanel, {
|
||||
attributes: true,
|
||||
attributeFilter: ['open'],
|
||||
});
|
||||
|
||||
@@ -1,22 +1,77 @@
|
||||
import { LitElement, html, css } from 'lit';
|
||||
import { customElement, state } from 'lit/decorators.js';
|
||||
import { LitElement, html, css, nothing } from 'lit';
|
||||
import { customElement, property, state } from 'lit/decorators.js';
|
||||
import '@awesome.me/webawesome/dist/components/icon/icon.js';
|
||||
import { PlayerController } from '@store/controllers/player-controller';
|
||||
import { queueStore } from '@store/queue-store';
|
||||
import type { RepeatMode } from '@store/queue-store';
|
||||
import { designTokens } from '../../../styles/tokens.css';
|
||||
import { PHONE_QUERY } from '../../../utils/breakpoints';
|
||||
|
||||
/**
|
||||
* The transport, in the two places it appears.
|
||||
*
|
||||
* **The context is a property and cannot be a media query**, which is
|
||||
* the whole reason this exists (#56). Everywhere else in this app a
|
||||
* component states what it drops at phone width itself, because a media
|
||||
* query inside a shadow root is answered by the viewport and that is
|
||||
* the honest signal. Here the two hosts want *different* answers at the
|
||||
* *same* viewport: on a phone the bottom bar wants three controls sized
|
||||
* for a thumb, and `now-playing-view` wants five, larger still. So the
|
||||
* host says which context and the viewport says which size band, and
|
||||
* neither one alone can express it.
|
||||
*
|
||||
* Measured at the reference device's 424x439 before this: every button
|
||||
* here was **33x21px**, in both places, which is what #56 reports as
|
||||
* "the most important thing in the mobile app and they are tiny".
|
||||
*/
|
||||
export type ControlsContext = 'bar' | 'full';
|
||||
|
||||
@customElement('player-controls')
|
||||
export class PlayerControls extends LitElement {
|
||||
private player = new PlayerController(this);
|
||||
private unsubscribeQueue?: () => void;
|
||||
|
||||
/**
|
||||
* Where these controls are drawn. `bar` is the bottom bar in both
|
||||
* bands; `full` is the full-screen transport.
|
||||
*
|
||||
* Reflected so a spec can read it and so the stylesheet keys off one
|
||||
* fact rather than a class the host has to remember to set.
|
||||
*/
|
||||
@property({ type: String, reflect: true })
|
||||
context: ControlsContext = 'bar';
|
||||
|
||||
@state() private shuffleMode = false;
|
||||
@state() private repeatMode: RepeatMode = 'off';
|
||||
|
||||
/**
|
||||
* Phone width, from `matchMedia` rather than from a media query,
|
||||
* because what it decides is whether shuffle and repeat *exist* here
|
||||
* — and a stylesheet can only decide whether they are painted.
|
||||
* `job-band` and `search-trigger` are the same pattern for the same
|
||||
* reason.
|
||||
*/
|
||||
@state() private phone = false;
|
||||
|
||||
private media?: MediaQueryList;
|
||||
|
||||
private onMedia = (e: MediaQueryListEvent) => {
|
||||
this.phone = e.matches;
|
||||
};
|
||||
|
||||
/** Whether this is the phone's bottom bar, which carries three
|
||||
* controls rather than five. */
|
||||
private get slim(): boolean {
|
||||
return this.context === 'bar' && this.phone;
|
||||
}
|
||||
|
||||
override connectedCallback(): void {
|
||||
super.connectedCallback();
|
||||
|
||||
this.media = window.matchMedia(PHONE_QUERY);
|
||||
this.phone = this.media.matches;
|
||||
this.media.addEventListener('change', this.onMedia);
|
||||
|
||||
const s = queueStore.getState();
|
||||
this.shuffleMode = s.shuffleMode;
|
||||
this.repeatMode = s.repeatMode;
|
||||
@@ -37,6 +92,7 @@ export class PlayerControls extends LitElement {
|
||||
override disconnectedCallback(): void {
|
||||
super.disconnectedCallback();
|
||||
this.unsubscribeQueue?.();
|
||||
this.media?.removeEventListener('change', this.onMedia);
|
||||
}
|
||||
|
||||
static override styles = [designTokens, css`
|
||||
@@ -58,6 +114,99 @@ export class PlayerControls extends LitElement {
|
||||
justify-content: center;
|
||||
}
|
||||
|
||||
/* ---------------------------------------------------------------
|
||||
Sizes (#56).
|
||||
|
||||
44px is the floor everything here is sized to, and play/pause
|
||||
alone goes above it -- "large play/pause, adequate prev/next" is
|
||||
the Direction, and it is the one control the report calls "front
|
||||
and centre".
|
||||
|
||||
They are stated as custom properties rather than on each button
|
||||
so a context sets two numbers instead of five rules, and so the
|
||||
icon scales with its target: a 44px box around a 16px glyph is a
|
||||
big hit area that still looks tiny, which is half of what the
|
||||
report is about.
|
||||
|
||||
**The desktop bar sets none of them and must not change at all.**
|
||||
#56 is an Android issue; the desktop's buttons are 33x21 before
|
||||
this and are 33x21 after it.
|
||||
|
||||
That is why the box rules take a zero fallback and the *font-size*
|
||||
rules are scoped to the two contexts instead of sharing them. A
|
||||
button does not inherit its font from its parent -- the UA
|
||||
stylesheet gives it one -- so a generic font-size: inherit is
|
||||
not the no-op it reads as: it moved the desktop's buttons from
|
||||
33x21 to 36x24, silently, by taking them from the UA's 13.3px to
|
||||
the shell's 16px. Measured before and after by stashing this
|
||||
file, which is the only way that particular 3px shows up.
|
||||
--------------------------------------------------------------- */
|
||||
button {
|
||||
min-width: var(--yj-control-target, 0);
|
||||
min-height: var(--yj-control-target, 0);
|
||||
}
|
||||
|
||||
button.play {
|
||||
min-width: var(--yj-control-play-target, 0);
|
||||
min-height: var(--yj-control-play-target, 0);
|
||||
}
|
||||
|
||||
/* The phone's bottom bar: three controls, sized for a thumb.
|
||||
Shuffle and repeat are not here -- see the render method, which
|
||||
does not draw them rather than hiding them, because a control
|
||||
that is display:none is still a thing the component claims to
|
||||
have. They are on the full-screen view, which is one tap away
|
||||
through the mini player's art (#59). */
|
||||
@media (max-width: 599px) {
|
||||
:host([context='bar']) {
|
||||
--yj-control-target: 44px;
|
||||
--yj-control-icon: 18px;
|
||||
--yj-control-play-target: 56px;
|
||||
--yj-control-play-icon: 24px;
|
||||
}
|
||||
|
||||
:host([context='bar']) button {
|
||||
font-size: var(--yj-control-icon);
|
||||
}
|
||||
|
||||
:host([context='bar']) button.play {
|
||||
font-size: var(--yj-control-play-icon);
|
||||
}
|
||||
}
|
||||
|
||||
/* The full-screen transport, at every width: this view *is* the
|
||||
player, so the controls are the page rather than a strip of it. */
|
||||
:host([context='full']) {
|
||||
--yj-control-target: 44px;
|
||||
--yj-control-icon: 20px;
|
||||
--yj-control-play-target: 64px;
|
||||
--yj-control-play-icon: 28px;
|
||||
}
|
||||
|
||||
:host([context='full']) button {
|
||||
font-size: var(--yj-control-icon);
|
||||
}
|
||||
|
||||
:host([context='full']) button.play {
|
||||
font-size: var(--yj-control-play-icon);
|
||||
}
|
||||
|
||||
:host([context='full']) #player-control-buttons {
|
||||
gap: 12px;
|
||||
}
|
||||
|
||||
/* Secondary controls sit below the primary row rather than beside
|
||||
it, which is the Direction's shape and is why this is a second
|
||||
group in the DOM instead of a CSS order property: visual order
|
||||
and focus order have to agree. */
|
||||
.secondary {
|
||||
display: flex;
|
||||
justify-content: center;
|
||||
align-items: center;
|
||||
gap: 24px;
|
||||
margin-top: 8px;
|
||||
}
|
||||
|
||||
button:hover {
|
||||
color: var(--yj-accent-text, #ffd43b);
|
||||
}
|
||||
@@ -104,46 +253,107 @@ export class PlayerControls extends LitElement {
|
||||
queueStore.cycleRepeat();
|
||||
};
|
||||
|
||||
override render() {
|
||||
const playOrPauseIcon = this.player.isPlaying ? 'pause' : 'play';
|
||||
const playOrPauseHandler = this.player.isPlaying
|
||||
? this.handlePauseClick
|
||||
: this.handlePlayClick;
|
||||
/** Shuffle. Secondary: it changes how the queue behaves rather than
|
||||
* what is playing now. */
|
||||
private renderShuffle() {
|
||||
return html`
|
||||
<button
|
||||
class=${this.shuffleMode ? 'active' : ''}
|
||||
aria-label="Shuffle"
|
||||
aria-pressed=${this.shuffleMode}
|
||||
@click=${this.handleShuffleClick}
|
||||
>
|
||||
<wa-icon name="shuffle"></wa-icon>
|
||||
</button>
|
||||
`;
|
||||
}
|
||||
|
||||
const shuffleClass = this.shuffleMode ? 'active' : '';
|
||||
/** Repeat, whose label spells the mode out because one icon covers
|
||||
* three states. */
|
||||
private renderRepeat() {
|
||||
const repeatMode = this.repeatMode;
|
||||
const repeatClasses = [
|
||||
repeatMode !== 'off' ? 'active' : '',
|
||||
repeatMode === 'one' ? 'repeat-one' : '',
|
||||
].filter(Boolean).join(' ');
|
||||
|
||||
return html`
|
||||
<button
|
||||
class=${repeatClasses}
|
||||
aria-label=${`Repeat: ${repeatMode}`}
|
||||
aria-pressed=${repeatMode !== 'off'}
|
||||
@click=${this.handleRepeatClick}
|
||||
>
|
||||
<wa-icon name="repeat"></wa-icon>
|
||||
</button>
|
||||
`;
|
||||
}
|
||||
|
||||
/** Previous, play/pause, next — the three that are always drawn, in
|
||||
* every context and at every width. Only play/pause takes the large
|
||||
* size: the Direction asks for "large play/pause, adequate
|
||||
* prev/next", and a row of identical squares says every action here
|
||||
* is equally likely, which is not true of play. */
|
||||
private renderPrimary() {
|
||||
const playOrPauseIcon = this.player.isPlaying ? 'pause' : 'play';
|
||||
const playOrPauseHandler = this.player.isPlaying
|
||||
? this.handlePauseClick
|
||||
: this.handlePlayClick;
|
||||
|
||||
return html`
|
||||
<button
|
||||
aria-label="Previous track"
|
||||
@click=${this.handlePreviousClick}
|
||||
>
|
||||
<wa-icon name="backward-step"></wa-icon>
|
||||
</button>
|
||||
<button
|
||||
class="play"
|
||||
aria-label=${this.player.isPlaying ? 'Pause' : 'Play'}
|
||||
@click="${playOrPauseHandler}"
|
||||
>
|
||||
<wa-icon name=${playOrPauseIcon}></wa-icon>
|
||||
</button>
|
||||
<button
|
||||
aria-label="Next track"
|
||||
@click=${this.handleNextClick}
|
||||
>
|
||||
<wa-icon name="forward-step"></wa-icon>
|
||||
</button>
|
||||
`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Two arrangements, not two components.
|
||||
*
|
||||
* `bar` keeps the order it has always had — shuffle, prev, play,
|
||||
* next, repeat, one row — so nothing about the desktop bar moves.
|
||||
* `full` puts the primary three on their own row with the secondary
|
||||
* pair beneath, which the Direction asks for.
|
||||
*
|
||||
* **The phone's bar draws three buttons rather than hiding two.** A
|
||||
* `display: none` control is still in the component's shadow root,
|
||||
* still in the accessibility tree's markup, and still something a
|
||||
* `shadowAll('button')[4]` finds — so "the phone has three controls"
|
||||
* would be true of the pixels and false of the element. They are
|
||||
* reachable on the full-screen view, which the mini player's art
|
||||
* opens, and through the global shortcuts.
|
||||
*/
|
||||
override render() {
|
||||
if (this.context === 'full') {
|
||||
return html`
|
||||
<div id="player-control-buttons">${this.renderPrimary()}</div>
|
||||
<div class="secondary">
|
||||
${this.renderShuffle()}${this.renderRepeat()}
|
||||
</div>
|
||||
`;
|
||||
}
|
||||
|
||||
return html`
|
||||
<div id="player-control-buttons">
|
||||
<button
|
||||
class=${shuffleClass}
|
||||
aria-label="Shuffle"
|
||||
aria-pressed=${this.shuffleMode}
|
||||
@click=${this.handleShuffleClick}
|
||||
>
|
||||
<wa-icon name="shuffle"></wa-icon>
|
||||
</button>
|
||||
<button aria-label="Previous track" @click=${this.handlePreviousClick}>
|
||||
<wa-icon name="backward-step"></wa-icon>
|
||||
</button>
|
||||
<button aria-label=${this.player.isPlaying ? 'Pause' : 'Play'} @click="${playOrPauseHandler}">
|
||||
<wa-icon name=${playOrPauseIcon}></wa-icon>
|
||||
</button>
|
||||
<button aria-label="Next track" @click=${this.handleNextClick}>
|
||||
<wa-icon name="forward-step"></wa-icon>
|
||||
</button>
|
||||
<button
|
||||
class=${repeatClasses}
|
||||
aria-label=${`Repeat: ${repeatMode}`}
|
||||
aria-pressed=${repeatMode !== 'off'}
|
||||
@click=${this.handleRepeatClick}
|
||||
>
|
||||
<wa-icon name="repeat"></wa-icon>
|
||||
</button>
|
||||
${this.slim ? nothing : this.renderShuffle()}
|
||||
${this.renderPrimary()}
|
||||
${this.slim ? nothing : this.renderRepeat()}
|
||||
</div>
|
||||
`;
|
||||
}
|
||||
|
||||
@@ -15,6 +15,7 @@ import { FavoritesController } from '@store/controllers/favorites-controller';
|
||||
import { designTokens } from '../../styles/tokens.css';
|
||||
import { srOnly } from '../../styles/sr-only.css';
|
||||
import { ICON_QUEUE } from '@utils/icon-language';
|
||||
import { openQueue as showQueue } from '@utils/open-queue';
|
||||
|
||||
/**
|
||||
* What is playing, at the size a phone has room for (plan 016 B2,
|
||||
@@ -116,8 +117,27 @@ export class NowPlayingView extends LitElement {
|
||||
.art .placeholder {
|
||||
/* Square, and never taller than the room left over: the
|
||||
art is the one thing here that would happily push the
|
||||
transport off the bottom of a short phone. */
|
||||
transport off the bottom of a short phone.
|
||||
|
||||
**max-height is what actually keeps that promise**, and
|
||||
it was missing. With a definite width and
|
||||
a 1:1 aspect-ratio the height is *derived from the width*
|
||||
and is bounded by nothing: at the reference device's
|
||||
424x439 that is a 263px square (60vh) in a box with far
|
||||
less than 263px left, so the art overflowed its own
|
||||
centred flex item and drew over the header above and the
|
||||
title below it. The comment claimed this was handled;
|
||||
60vh is a bound on the *viewport*, not on the room left
|
||||
over, and those differ by however much chrome is above
|
||||
and below.
|
||||
|
||||
Pre-existing -- screenshotted on main -- and made acute
|
||||
by #56, which gives the transport 95px more than it had.
|
||||
Found by reading a screenshot, which is the only tier
|
||||
that can see it: nothing fails, nothing overflows the
|
||||
*shell*, and every control is still hittable. */
|
||||
width: min(100%, 60vh);
|
||||
max-height: 100%;
|
||||
aspect-ratio: 1;
|
||||
object-fit: cover;
|
||||
border-radius: 12px;
|
||||
@@ -226,13 +246,15 @@ export class NowPlayingView extends LitElement {
|
||||
*
|
||||
* This view hides the bottom bar (index.css), and the bar is where
|
||||
* the queue button lives -- so without this, going full-screen
|
||||
* would take the queue away. It toggles the same `open` attribute
|
||||
* would take the queue away. It goes through the same helper
|
||||
* `index.ts` does, because the panel's state is an attribute on one
|
||||
* element and a second mechanism for it is a second thing to keep
|
||||
* in step.
|
||||
* in step -- which is exactly what this button was: it set `open`
|
||||
* directly, so on a phone it produced a queue with no history entry
|
||||
* behind it and back moved the page underneath instead (#55).
|
||||
*/
|
||||
private openQueue() {
|
||||
document.getElementById('queue-panel')?.setAttribute('open', '');
|
||||
showQueue();
|
||||
}
|
||||
|
||||
private toggleFavorite() {
|
||||
@@ -314,7 +336,13 @@ export class NowPlayingView extends LitElement {
|
||||
|
||||
<div class="transport">
|
||||
<seek-bar></seek-bar>
|
||||
<player-controls></player-controls>
|
||||
<!-- context="full": this view *is* the player, so the
|
||||
transport is the page rather than a strip of it --
|
||||
primary controls large, shuffle and repeat beneath
|
||||
at normal size (#56). It is a property rather than
|
||||
a media query because the bottom bar wants a
|
||||
different answer at this same viewport. -->
|
||||
<player-controls context="full"></player-controls>
|
||||
<volume-control></volume-control>
|
||||
</div>
|
||||
`;
|
||||
|
||||
@@ -215,6 +215,17 @@ export class NowPlaying extends LitElement {
|
||||
outline: 2px solid var(--yj-accent, #ffd43b);
|
||||
outline-offset: 2px;
|
||||
}
|
||||
|
||||
/* The favourite is one of the three controls #59 keeps on the
|
||||
phone's bar, and it was the **smallest control in the app**:
|
||||
measured at 424x439, 18x14px, against the 48x48 art beside it.
|
||||
Zero padding around an icon-sized glyph is a reasonable mouse
|
||||
target and is not a thumb target at all. */
|
||||
.fav-btn {
|
||||
min-width: 44px;
|
||||
min-height: 44px;
|
||||
font-size: var(--yj-icon-md);
|
||||
}
|
||||
}
|
||||
|
||||
.cover-preview-panel {
|
||||
@@ -446,8 +457,30 @@ export class NowPlaying extends LitElement {
|
||||
return html`
|
||||
<div class="sr-only" role="status" aria-live="polite">${announcement}</div>
|
||||
<div class="now-playing">
|
||||
<div class="cover-art">
|
||||
<div class="cover-placeholder"><wa-icon name="music"></wa-icon></div>
|
||||
<!-- **The way to Now Playing does not depend on what is
|
||||
playing.** This branch used to render the placeholder
|
||||
with no button on it, so on a phone there was no route to
|
||||
the full-screen view while nothing was loaded -- and once
|
||||
#59 took the queue button off the bar, that made the
|
||||
queue itself unreachable, because Now Playing is where it
|
||||
is reached from. The queue is persisted across restarts,
|
||||
so "a queue with tracks in it and nothing playing" is an
|
||||
ordinary state to launch into, not a corner.
|
||||
|
||||
Plan 018's matrix promises no action is unreachable at
|
||||
any supported size, and the promise is what makes #59
|
||||
allowed to remove a control at all. -->
|
||||
<div class="cover-art-wrapper">
|
||||
<button
|
||||
type="button"
|
||||
class="expand"
|
||||
data-testid="open-now-playing"
|
||||
aria-label="Open now playing"
|
||||
@click=${this.openNowPlaying}
|
||||
></button>
|
||||
<div class="cover-art">
|
||||
<div class="cover-placeholder"><wa-icon name="music"></wa-icon></div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
<div
|
||||
|
||||
@@ -410,6 +410,22 @@ export class QueuePanel
|
||||
:host([overlay]) .panel-content {
|
||||
width: 100%;
|
||||
}
|
||||
|
||||
/* A screen's way out has to be hittable with a thumb.
|
||||
Measured at 424x439 before #55: these were **25x21px**,
|
||||
and with the panel spanning the whole width the scrim
|
||||
underneath has no uncovered pixels at all -- so it was
|
||||
the only pointer route out of a full-screen surface.
|
||||
Back answers it now as well, which is the other half.
|
||||
|
||||
Sized only in overlay mode: inline these sit in a 320px
|
||||
column beside the content, where a mouse is what reaches
|
||||
them and 44px of header is 44px the queue does not get. */
|
||||
:host([overlay]) .header-action-button {
|
||||
min-width: 44px;
|
||||
min-height: 44px;
|
||||
justify-content: center;
|
||||
}
|
||||
}
|
||||
|
||||
.resize-handle {
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* Opening the queue, from the two buttons that do it.
|
||||
*
|
||||
* **The queue is a place while it is covering the content, and a
|
||||
* control while it sits beside it** (#55). Those are not two components
|
||||
* and not two mount points — they are the two presentations #24 already
|
||||
* computes, and this is the one line that turns that measurement into a
|
||||
* navigation decision.
|
||||
*
|
||||
* A column is a thing the user docked: back must not undock it, and
|
||||
* navigating to Albums must not take it away. An overlay is a screen —
|
||||
* at the reference device's 424x439 it is 424x318, which is
|
||||
* `.main-panel`'s rect exactly — so it needs the two things a screen
|
||||
* has and this one did not: an entry in the back stack, and a way out
|
||||
* that answers the platform's own gesture. Measured before this existed:
|
||||
* opening the queue on Artists and pressing back moved the page
|
||||
* *underneath* to Albums and left the queue up.
|
||||
*
|
||||
* The mode is read off the panel rather than from a viewport width, for
|
||||
* the reason `queue-panel.overlay` is computed at all: the panel is
|
||||
* drag-resizable between 200 and 500px and persisted, so a breakpoint
|
||||
* is wrong by up to 180px in the direction that hurts.
|
||||
*/
|
||||
export function queuePanelElement(): HTMLElement | null {
|
||||
return document.getElementById('queue-panel');
|
||||
}
|
||||
|
||||
/** Whether the queue is currently a screen rather than a column. */
|
||||
export function queueIsAScreen(): boolean {
|
||||
return queuePanelElement()?.hasAttribute('overlay') ?? false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Show the queue: a navigation where it is a screen, an attribute where
|
||||
* it is a column.
|
||||
*
|
||||
* Both routes end at the same `open` attribute on the same element —
|
||||
* `index.ts` handles `navigate {view: 'queue'}` by setting it — because
|
||||
* the panel's state is one fact and a second mechanism for it is a
|
||||
* second thing to keep in step.
|
||||
*/
|
||||
export function openQueue(): void {
|
||||
if (queueIsAScreen()) {
|
||||
document.dispatchEvent(new CustomEvent('navigate', {
|
||||
bubbles: true,
|
||||
composed: true,
|
||||
detail: { view: 'queue' },
|
||||
}));
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
queuePanelElement()?.setAttribute('open', '');
|
||||
}
|
||||
@@ -0,0 +1,181 @@
|
||||
/**
|
||||
* The transport in its two contexts (#59, #56).
|
||||
*
|
||||
* `player-controls` is one component in two places, and what each place
|
||||
* wants differs *at the same viewport*: on a phone the bottom bar wants
|
||||
* three controls sized for a thumb, and `now-playing-view` wants five,
|
||||
* larger still. So the host states the context and the viewport states
|
||||
* the size band, and this file pins the half a media query cannot
|
||||
* express.
|
||||
*
|
||||
* **What this tier can and cannot see.** It can see which buttons
|
||||
* exist, because that is `matchMedia` and a render — and existence is
|
||||
* the whole of #59. It cannot see the *sizes*: those come from the
|
||||
* context's custom properties, and a component-tier render has no shell
|
||||
* around it, so the measurements live in `e2e/specs/phone-transport.spec.ts`
|
||||
* where there is a real bar in a real viewport. Asserting a pixel here
|
||||
* would be asserting the fallbacks, which is `ui-visual`'s documented
|
||||
* blind spot one tier over.
|
||||
*/
|
||||
import { describe, expect, it, beforeEach, afterEach } from 'vitest';
|
||||
|
||||
import '@components/audio-player/controls/player-controls';
|
||||
import { Events } from '../../src/events';
|
||||
import { emit, flush, calls } from '@test/support/harness';
|
||||
import { fixture, shadowAll, click } from '@test/support/render';
|
||||
|
||||
/** Reset the backend-owned state the component reads from. */
|
||||
function idle(): void {
|
||||
emit(Events.TrackChanged, null);
|
||||
emit(Events.PlaybackStateChanged, { state: 'stopped' });
|
||||
emit(Events.QueueModeChanged, { shuffleMode: false, repeatMode: 'off' });
|
||||
}
|
||||
|
||||
const names = (el: Element): Array<string | null> =>
|
||||
shadowAll(el, 'button').map((b) => b.getAttribute('aria-label'));
|
||||
|
||||
/**
|
||||
* Answer `matchMedia` for the phone query, since the test runner's own
|
||||
* window is whatever size the browser provider gives it.
|
||||
*
|
||||
* It is stubbed rather than resized because what is under test is the
|
||||
* component's *reaction* to the answer, and a resize would additionally
|
||||
* be asserting that this runner's viewport can get below 600px.
|
||||
*/
|
||||
const realMatchMedia = window.matchMedia;
|
||||
|
||||
function pretendPhone(phone: boolean): void {
|
||||
window.matchMedia = ((query: string) => ({
|
||||
matches: phone && query.includes('599'),
|
||||
media: query,
|
||||
addEventListener: () => {},
|
||||
removeEventListener: () => {},
|
||||
})) as unknown as typeof window.matchMedia;
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
window.matchMedia = realMatchMedia;
|
||||
});
|
||||
|
||||
describe('<player-controls> in the bar', () => {
|
||||
beforeEach(() => {
|
||||
idle();
|
||||
});
|
||||
|
||||
it('keeps all five on a desktop, in the order it always had', async () => {
|
||||
pretendPhone(false);
|
||||
|
||||
const el = await fixture('player-controls');
|
||||
|
||||
// Unchanged from before #59, deliberately: this is the desktop bar
|
||||
// and nothing about it was reported.
|
||||
expect(names(el)).toEqual([
|
||||
'Shuffle',
|
||||
'Previous track',
|
||||
'Play',
|
||||
'Next track',
|
||||
'Repeat: off',
|
||||
]);
|
||||
});
|
||||
|
||||
it('draws three on a phone, and does not merely hide the other two', async () => {
|
||||
pretendPhone(true);
|
||||
|
||||
const el = await fixture('player-controls');
|
||||
|
||||
expect(names(el)).toEqual(['Previous track', 'Play', 'Next track']);
|
||||
|
||||
// The distinction this asserts is the point. A `display: none`
|
||||
// control is still in the shadow root, still something a positional
|
||||
// query finds, and still a thing the component claims to have --
|
||||
// so "the phone has three controls" would have been true of the
|
||||
// pixels and false of the element.
|
||||
expect(shadowAll(el, 'button')).toHaveLength(3);
|
||||
});
|
||||
|
||||
it('follows the viewport when it changes, not just at construction', async () => {
|
||||
pretendPhone(false);
|
||||
|
||||
const el = await fixture('player-controls');
|
||||
|
||||
expect(names(el)).toHaveLength(5);
|
||||
|
||||
// A desktop window dragged narrow is the phone layout, per plan
|
||||
// 018's decision 4 -- so this is a real transition and not a
|
||||
// hypothetical.
|
||||
(el as unknown as { phone: boolean }).phone = true;
|
||||
await flush();
|
||||
await el.updateComplete;
|
||||
|
||||
expect(names(el)).toEqual(['Previous track', 'Play', 'Next track']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('<player-controls> full-screen', () => {
|
||||
beforeEach(() => {
|
||||
idle();
|
||||
});
|
||||
|
||||
it('keeps all five on a phone, where the bar keeps three', async () => {
|
||||
pretendPhone(true);
|
||||
|
||||
const el = await fixture('player-controls');
|
||||
|
||||
el.setAttribute('context', 'full');
|
||||
await el.updateComplete;
|
||||
|
||||
// The same viewport, the other answer: this is why the context is a
|
||||
// property and cannot be a media query.
|
||||
expect(names(el)).toHaveLength(5);
|
||||
});
|
||||
|
||||
it('puts the secondary pair after the primary three, in the DOM', async () => {
|
||||
pretendPhone(true);
|
||||
|
||||
const el = await fixture('player-controls');
|
||||
|
||||
el.setAttribute('context', 'full');
|
||||
await el.updateComplete;
|
||||
|
||||
// Order, not just membership: the secondary controls are drawn on a
|
||||
// second row, and this is asserted in the DOM because visual order
|
||||
// and focus order have to agree. A CSS `order` property would move
|
||||
// them on screen and leave Tab walking the old sequence.
|
||||
expect(names(el)).toEqual([
|
||||
'Previous track',
|
||||
'Play',
|
||||
'Next track',
|
||||
'Shuffle',
|
||||
'Repeat: off',
|
||||
]);
|
||||
});
|
||||
|
||||
it('still routes every button to the backend', async () => {
|
||||
pretendPhone(true);
|
||||
|
||||
const el = await fixture('player-controls');
|
||||
|
||||
el.setAttribute('context', 'full');
|
||||
await el.updateComplete;
|
||||
|
||||
// Two arrangements, one set of handlers. The regression this
|
||||
// guards is the reason a second *component* was refused: a second
|
||||
// template renders buttons wired to nothing, which looks perfect
|
||||
// in a screenshot and does nothing at all.
|
||||
for (const name of [
|
||||
'Previous track',
|
||||
'Next track',
|
||||
'Shuffle',
|
||||
'Repeat: off',
|
||||
]) {
|
||||
await click(el, `button[aria-label="${name}"]`);
|
||||
}
|
||||
|
||||
expect(calls().map((c) => c.path)).toEqual([
|
||||
'queue.Queue.Previous',
|
||||
'queue.Queue.Next',
|
||||
'queue.Queue.ToggleShuffle',
|
||||
'queue.Queue.CycleRepeat',
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,101 @@
|
||||
/**
|
||||
* Opening the queue is a *navigation* where the queue is a screen, and
|
||||
* an *attribute* where it is a column (#55).
|
||||
*
|
||||
* This is the one decision in that change, so it is pinned at the tier
|
||||
* that can state it without a shell: the mode is read off the panel's
|
||||
* own `overlay` attribute — which #24 computes from the measured widths
|
||||
* — and never from a viewport breakpoint. A breakpoint would silently
|
||||
* assume the default 320px panel and be wrong by up to 180px for a user
|
||||
* who has dragged it wide, in the direction that hurts.
|
||||
*
|
||||
* What this tier cannot see is the other half: that the entry is
|
||||
* unwound when the panel closes, which lives in the shell's mutation
|
||||
* observer. `e2e/specs/queue-as-a-screen.spec.ts` is where that is
|
||||
* asserted, and it is asserted as *two* back presses rather than one.
|
||||
*/
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { openQueue, queueIsAScreen } from '@utils/open-queue';
|
||||
|
||||
function panel(overlay: boolean): HTMLElement {
|
||||
const el = document.createElement('div');
|
||||
|
||||
el.id = 'queue-panel';
|
||||
if (overlay) el.setAttribute('overlay', '');
|
||||
document.body.appendChild(el);
|
||||
|
||||
return el;
|
||||
}
|
||||
|
||||
function recordNavigations(): string[] {
|
||||
const seen: string[] = [];
|
||||
const listener = (e: Event) => {
|
||||
seen.push((e as CustomEvent).detail.view);
|
||||
};
|
||||
|
||||
document.addEventListener('navigate', listener);
|
||||
cleanup.push(() => document.removeEventListener('navigate', listener));
|
||||
|
||||
return seen;
|
||||
}
|
||||
|
||||
const cleanup: Array<() => void> = [];
|
||||
|
||||
afterEach(() => {
|
||||
while (cleanup.length) cleanup.pop()!();
|
||||
document.getElementById('queue-panel')?.remove();
|
||||
});
|
||||
|
||||
describe('opening the queue', () => {
|
||||
it('navigates where the queue covers the content', () => {
|
||||
const el = panel(true);
|
||||
const seen = recordNavigations();
|
||||
|
||||
expect(queueIsAScreen()).toBe(true);
|
||||
|
||||
openQueue();
|
||||
|
||||
expect(seen).toEqual(['queue']);
|
||||
// The shell answers the navigation by setting the attribute, so
|
||||
// the helper deliberately does *not* set it as well: two
|
||||
// mechanisms for one fact is two things to keep in step, which
|
||||
// is what `now-playing-view`'s copy of this button was.
|
||||
expect(el.hasAttribute('open')).toBe(false);
|
||||
});
|
||||
|
||||
it('sets the attribute where the queue is a column', () => {
|
||||
const el = panel(false);
|
||||
const seen = recordNavigations();
|
||||
|
||||
expect(queueIsAScreen()).toBe(false);
|
||||
|
||||
openQueue();
|
||||
|
||||
// A column is a thing the user docked. Back must not undock it,
|
||||
// so it is not a history entry and therefore not a navigation.
|
||||
expect(seen).toEqual([]);
|
||||
expect(el.hasAttribute('open')).toBe(true);
|
||||
});
|
||||
|
||||
it('follows the panel rather than the viewport', () => {
|
||||
const el = panel(false);
|
||||
const seen = recordNavigations();
|
||||
|
||||
openQueue();
|
||||
expect(seen).toEqual([]);
|
||||
|
||||
// Nothing about the window changed; the panel got wider, which
|
||||
// is exactly the case a media query cannot express.
|
||||
el.removeAttribute('open');
|
||||
el.setAttribute('overlay', '');
|
||||
|
||||
openQueue();
|
||||
expect(seen).toEqual(['queue']);
|
||||
});
|
||||
|
||||
it('says the queue is not a screen when there is no panel at all', () => {
|
||||
expect(queueIsAScreen()).toBe(false);
|
||||
expect(() => openQueue()).not.toThrow();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user