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
|
||||
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.
|
||||
|
||||
@@ -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
|
||||
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,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 { 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'],
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
@@ -226,13 +227,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() {
|
||||
|
||||
@@ -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,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