Compare commits
5
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
af4b28b0d7 | ||
|
|
4ee5b4b473 | ||
|
|
a70a7ed9eb | ||
|
|
de2cb2693a | ||
|
|
880adff12c |
@@ -4429,3 +4429,61 @@ the content starts where the **row above it** ends, which is true with a
|
|||||||
job running and without one. An assertion against an absolute
|
job running and without one. An assertion against an absolute
|
||||||
coordinate was quietly also asserting "and no background job exists",
|
coordinate was quietly also asserting "and no background job exists",
|
||||||
which is not something that spec is about or can arrange.
|
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.
|
||||||
|
|||||||
@@ -1775,6 +1775,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
|
only while the overlay is up — it is a dismissal, not a shortcut, which
|
||||||
is why it is not a panel-scoped binding.
|
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:
|
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
|
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
|
the queue *closed*. That is #69, and it cannot be fixed in
|
||||||
|
|||||||
@@ -0,0 +1,270 @@
|
|||||||
|
import { test, expect } 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 };
|
||||||
|
|
||||||
|
const activeView = (page: Page) => page.getByTestId('main-content');
|
||||||
|
const queue = (page: Page) => page.locator('#queue-panel');
|
||||||
|
const toggle = (page: Page) => page.locator('#queue-button');
|
||||||
|
|
||||||
|
async function expectQueue(page: Page, open: boolean): Promise<void> {
|
||||||
|
await expect(toggle(page)).toHaveAttribute(
|
||||||
|
'aria-expanded',
|
||||||
|
String(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');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('back closes the queue and leaves the page where it was', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await expect(queue(app)).toHaveAttribute('overlay', '');
|
||||||
|
|
||||||
|
await toggle(app).click();
|
||||||
|
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', 'albums');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('costs exactly one entry, so the next press navigates', async ({
|
||||||
|
app,
|
||||||
|
}) => {
|
||||||
|
await toggle(app).click();
|
||||||
|
await expectQueue(app, true);
|
||||||
|
|
||||||
|
await app.goBack();
|
||||||
|
await expectQueue(app, false);
|
||||||
|
|
||||||
|
await app.goBack();
|
||||||
|
|
||||||
|
// Whatever the launch page is, it is not Albums — the point is that
|
||||||
|
// this press moved the app rather than being swallowed by a queue
|
||||||
|
// that had already closed.
|
||||||
|
await expect(activeView(app)).not.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');
|
||||||
|
},
|
||||||
|
],
|
||||||
|
[
|
||||||
|
'the toggle it was opened from',
|
||||||
|
async (app: Page) => {
|
||||||
|
await toggle(app).click();
|
||||||
|
},
|
||||||
|
],
|
||||||
|
] as Array<[string, (app: Page) => Promise<void>]>) {
|
||||||
|
test(`${name} leaves no entry behind`, async ({ app }) => {
|
||||||
|
await toggle(app).click();
|
||||||
|
await expectQueue(app, true);
|
||||||
|
|
||||||
|
await dismiss(app);
|
||||||
|
await expectQueue(app, false);
|
||||||
|
|
||||||
|
await app.goBack();
|
||||||
|
|
||||||
|
await expect(activeView(app)).not.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.
|
||||||
|
*/
|
||||||
|
test('leaves the tab it was opened from highlighted', async ({ app }) => {
|
||||||
|
await expect(
|
||||||
|
app.getByRole('button', { name: 'Albums', exact: true }),
|
||||||
|
).toHaveAttribute('aria-current', 'page');
|
||||||
|
|
||||||
|
await toggle(app).click();
|
||||||
|
await expectQueue(app, true);
|
||||||
|
|
||||||
|
await expect(
|
||||||
|
app.getByRole('button', { name: 'Albums', exact: true }),
|
||||||
|
).toHaveAttribute('aria-current', 'page');
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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 toggle(app).click();
|
||||||
|
|
||||||
|
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 toggle(app).click();
|
||||||
|
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/);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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);
|
||||||
|
});
|
||||||
|
});
|
||||||
+89
-6
@@ -67,6 +67,7 @@ import '@store/theme-store';
|
|||||||
import './src/services/keyboard-shortcut-service';
|
import './src/services/keyboard-shortcut-service';
|
||||||
import { activateView, deactivateView } from '@utils/view-lifecycle';
|
import { activateView, deactivateView } from '@utils/view-lifecycle';
|
||||||
import { installLongPressContextMenu } from '@utils/long-press';
|
import { installLongPressContextMenu } from '@utils/long-press';
|
||||||
|
import { openQueue, queuePanelElement } from '@utils/open-queue';
|
||||||
import { installTopBarFit } from './src/services/top-bar-fit';
|
import { installTopBarFit } from './src/services/top-bar-fit';
|
||||||
import {
|
import {
|
||||||
hasTrackPayload,
|
hasTrackPayload,
|
||||||
@@ -336,6 +337,33 @@ window.addEventListener('popstate', (e: PopStateEvent) => {
|
|||||||
void handleNavigate({ ...nav, _isBack: true });
|
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(
|
async function handleNavigate(
|
||||||
detail: { view: string; [key: string]: any },
|
detail: { view: string; [key: string]: any },
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
@@ -347,6 +375,25 @@ async function handleNavigate(
|
|||||||
|
|
||||||
if (!detail._isBack) recordNavigation(detail);
|
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
|
// Bookkeeping stays synchronous with the click: the search box's
|
||||||
// scope and the active-view attribute describe the navigation that
|
// scope and the active-view attribute describe the navigation that
|
||||||
// was *asked for*, and are what the rest of the app and the e2e
|
// 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) {
|
if (queueButton && queuePanel) {
|
||||||
queueButton.addEventListener('click', () => {
|
queueButton.addEventListener('click', () => {
|
||||||
const isOpen = queuePanel.hasAttribute('open');
|
if (queuePanel.hasAttribute('open')) {
|
||||||
|
// Closing goes through the panel either way; where the queue
|
||||||
if (isOpen) {
|
// 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');
|
queuePanel.removeAttribute('open');
|
||||||
} else {
|
|
||||||
queuePanel.setAttribute('open', '');
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
openQueue();
|
||||||
});
|
});
|
||||||
|
|
||||||
// The button says whether the panel is open, and it learns that
|
// 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,
|
attributes: true,
|
||||||
attributeFilter: ['open'],
|
attributeFilter: ['open'],
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -15,6 +15,7 @@ import { FavoritesController } from '@store/controllers/favorites-controller';
|
|||||||
import { designTokens } from '../../styles/tokens.css';
|
import { designTokens } from '../../styles/tokens.css';
|
||||||
import { srOnly } from '../../styles/sr-only.css';
|
import { srOnly } from '../../styles/sr-only.css';
|
||||||
import { ICON_QUEUE } from '@utils/icon-language';
|
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,
|
* What is playing, at the size a phone has room for (plan 016 B2,
|
||||||
@@ -226,13 +227,15 @@ export class NowPlayingView extends LitElement {
|
|||||||
*
|
*
|
||||||
* This view hides the bottom bar (index.css), and the bar is where
|
* This view hides the bottom bar (index.css), and the bar is where
|
||||||
* the queue button lives -- so without this, going full-screen
|
* 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
|
* `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
|
* 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() {
|
private openQueue() {
|
||||||
document.getElementById('queue-panel')?.setAttribute('open', '');
|
showQueue();
|
||||||
}
|
}
|
||||||
|
|
||||||
private toggleFavorite() {
|
private toggleFavorite() {
|
||||||
|
|||||||
@@ -410,6 +410,22 @@ export class QueuePanel
|
|||||||
:host([overlay]) .panel-content {
|
:host([overlay]) .panel-content {
|
||||||
width: 100%;
|
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 {
|
.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,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