diff --git a/e2e/specs/queue-overlay.spec.ts b/e2e/specs/queue-overlay.spec.ts new file mode 100644 index 0000000..1000936 --- /dev/null +++ b/e2e/specs/queue-overlay.spec.ts @@ -0,0 +1,187 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * #24 — the queue panel does not take the page's width away from it. + * + * The panel is `flex-shrink: 0` in the flow of `.content-area`, so an + * open queue used to be paid for by the main panel. Measured on + * Playlists before the fix: + * + * | viewport | main panel | + * |---|---| + * | 900×600 | 379px — all three header actions clipped | + * | 390×780 | 69px | + * | 320×600 | **0px** | + * + * **900×600 is the worst desktop case, not the 800×600 minimum**, and + * that is the trap this file exists to keep closed: the sidebar + * collapses to icons *below* 900, so the main panel is 843px at 899 and + * 700px at 900. A spec that checks "the minimum" and stops has not + * checked the worst case — which is what every viewport list in this + * suite did before this. + * + * These assert the *content's* width rather than the panel's mode + * wherever they can, because the mode is the mechanism and the width is + * the complaint. + */ + +/** The bands from plan 018's size matrix, plus the pixel above the collapse. */ +const BANDS = [ + { name: 'a wide desktop (1280×800)', width: 1280, height: 800, inline: true }, + { name: 'the default window (1100×720)', width: 1100, height: 720, inline: true }, + { name: 'a laptop (1024×768)', width: 1024, height: 768, inline: true }, + { name: 'the worst desktop width (900×600)', width: 900, height: 600, inline: false }, + { name: 'the enforced minimum (800×600)', width: 800, height: 600, inline: false }, + { name: 'a phone (390×780)', width: 390, height: 780, inline: false }, + { name: '400% zoom (320×600)', width: 320, height: 600, inline: false }, +]; + +/** + * How much room the content has, and whether the shell needs scrolling + * to reach any of itself. + */ +const shellGeometry = (page: import('@playwright/test').Page) => + page.evaluate(() => { + const main = document.querySelector('#main-content')!.getBoundingClientRect(); + const panel = document.querySelector('#queue-panel')!; + + return { + mainWidth: Math.round(main.width), + overlay: panel.hasAttribute('overlay'), + open: panel.hasAttribute('open'), + bodyScrollWidth: document.body.scrollWidth, + bodyClientWidth: document.body.clientWidth, + }; + }); + +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'); +} + +test.describe('an open queue leaves the content its width', () => { + for (const band of BANDS) { + test(`at ${band.name}`, async ({ app }) => { + await app.setViewportSize({ width: band.width, height: band.height }); + await openQueue(app); + + // The mode is settled by a ResizeObserver, so poll rather than + // read once: a single read races the resize and reports the + // previous viewport's answer. + await expect + .poll(async () => (await shellGeometry(app)).overlay) + .toBe(!band.inline); + + const geo = await shellGeometry(app); + + // The floor is the point of the whole issue. Inline, the queue is + // affordable and the content keeps the rest; as an overlay the + // content keeps *everything*, which is what makes 0px at 320 + // impossible rather than merely unlikely. + expect(geo.mainWidth).toBeGreaterThanOrEqual(320); + + if (!band.inline) { + expect(geo.mainWidth).toBeGreaterThanOrEqual( + Math.min(band.width, 320), + ); + } + + // And opening the queue must not make the shell overflow. + expect(geo.bodyScrollWidth).toBeLessThanOrEqual(geo.bodyClientWidth); + }); + } +}); + +test.describe('an overlaid queue says it is over the content', () => { + test.beforeEach(async ({ app }) => { + await app.setViewportSize({ width: 900, height: 600 }); + }); + + test('draws a scrim and closes when it is clicked', async ({ app }) => { + await openQueue(app); + + const panel = app.locator('#queue-panel'); + + await expect(panel).toHaveAttribute('overlay', ''); + + // The scrim is `aria-hidden` on purpose — it is a dismissal target, + // and the named routes out are the close button and Escape — so it + // is located structurally rather than by role. + await panel.evaluate((el) => + el.shadowRoot!.querySelector('.scrim')!.click(), + ); + + await expect(app.locator('#queue-button')).toHaveAttribute( + 'aria-expanded', + 'false', + ); + }); + + /** + * `getByRole`, not a shadow-root query: this repo has shipped a + * nameless control three times, and a drawer with a scrim is exactly + * the shape that grows a fourth. + */ + test('offers a named close button', async ({ app }) => { + await openQueue(app); + + const close = app.getByRole('button', { name: 'Close queue' }); + + await expect(close).toBeVisible(); + await close.click(); + + await expect(app.locator('#queue-button')).toHaveAttribute( + 'aria-expanded', + 'false', + ); + }); + + test('closes on Escape and gives focus back to the toggle', async ({ + app, + }) => { + const toggle = app.locator('#queue-button'); + + await toggle.focus(); + await toggle.click(); + await expect(toggle).toHaveAttribute('aria-expanded', 'true'); + + await app.keyboard.press('Escape'); + + await expect(toggle).toHaveAttribute('aria-expanded', 'false'); + await expect(toggle).toBeFocused(); + }); +}); + +/** + * The inline panel is the mode that already worked, and the one every + * other queue spec is written against. It keeps its resize handle and + * gains none of the overlay's chrome. + */ +test.describe('a wide window keeps the queue beside the content', () => { + test('no scrim, no close button, and the content is narrower', async ({ + app, + }) => { + await app.setViewportSize({ width: 1280, height: 800 }); + + const widthWithoutQueue = (await shellGeometry(app)).mainWidth; + + await openQueue(app); + + await expect(app.locator('#queue-panel')).not.toHaveAttribute( + 'overlay', + '', + ); + + const geo = await shellGeometry(app); + + expect(geo.mainWidth).toBeLessThan(widthWithoutQueue); + await expect( + app.getByRole('button', { name: 'Close queue' }), + ).toHaveCount(0); + }); +}); diff --git a/frontend/index.css b/frontend/index.css index 7549e03..71b65db 100644 --- a/frontend/index.css +++ b/frontend/index.css @@ -231,6 +231,14 @@ body div.sidebar { display: flex; overflow: hidden; contain: layout style; + + /* The containing block for the queue panel's overlay mode (plan + 018, #24), which spans this box rather than taking width from + the main panel beside it. `contain: layout` already establishes + one; this says so on purpose, so that removing the containment + for a paint reason does not silently reparent the overlay to the + viewport. */ + position: relative; } .main-panel { diff --git a/frontend/src/components/queue-panel/queue-panel.ts b/frontend/src/components/queue-panel/queue-panel.ts index eb8b4fb..397785e 100644 --- a/frontend/src/components/queue-panel/queue-panel.ts +++ b/frontend/src/components/queue-panel/queue-panel.ts @@ -72,6 +72,24 @@ const MIN_WIDTH = 200; const MAX_WIDTH = 500; const DEFAULT_WIDTH = 320; +/** + * The narrowest main panel the queue is allowed to leave behind before + * it stops being a column and becomes an overlay (plan 018, issue #24). + * + * There is no cliff to derive this from, and pretending otherwise would + * be the more dishonest answer: the track list rescales its columns + * continuously (213px down to 124px between main widths of 900 and 544, + * with no row overflow at any of them) and the album grid steps 3 + * columns to 2 without breaking. So this is a judgement, anchored at + * both ends — it keeps the *default* 1100px window inline, because an + * inline queue is a desktop affordance people choose and demoting the + * common case to an overlay would be a regression in feel; and it puts + * every case measured as broken on the overlay side, which is 900x600 + * (main = 379px, where all three of the Playlists header's actions are + * clipped) and every phone width (main = 69px at 390, 0px at 320). + */ +const MAIN_PANEL_FLOOR = 480; + @customElement('queue-panel') export class QueuePanel extends LitElement @@ -85,6 +103,22 @@ export class QueuePanel @property({ type: Boolean, reflect: true }) open = false; + /** + * Whether the panel is covering the content instead of sitting + * beside it. **Computed, never set by a caller** — it is reflected + * so the stylesheet and a spec can both read it. + * + * It is deliberately *not* a media query, which is the whole reason + * this is a property and not a `@media` block. The panel's width is + * user state: drag-resizable between MIN_WIDTH and MAX_WIDTH and + * persisted. A breakpoint at a fixed viewport width silently + * assumes the default 320, so it is wrong by up to 180px for a user + * who has widened the panel — in the direction that hurts, since a + * wider queue is exactly when the content can least afford it. + */ + @property({ type: Boolean, reflect: true }) + overlay = false; + @state() private isDragging = false; @@ -190,6 +224,19 @@ export class QueuePanel private panelWidth = DEFAULT_WIDTH; private scrollbarDragging = false; + /** Watches `.content-area`, which is the viewport minus the sidebar. */ + private spaceObserver?: ResizeObserver; + + /** + * What had focus when the overlay opened, so Escape and the scrim + * can give it back. Focus is only taken back if the panel had it — + * the same rule `MenuKeyboard` follows, for the same reason: the + * queue can also be closed by the button in the bottom bar, and + * yanking focus away from wherever the user actually is would be + * worse than leaving it. + */ + private overlayOpener: HTMLElement | null = null; + // _itemSize is an internal property applied via Object.assign in BaseLayout's // config setter. Setting it to match the actual fixed .track-item height (49px) // prevents lit-virtualizer's scroll error correction from fighting the native @@ -265,6 +312,86 @@ export class QueuePanel border-left: 1px solid var(--yj-border-subtle, #333); } + /* --------------------------------------------------------- + Overlay mode (plan 018, #24). + + In flow the panel takes its width *from the main panel*, + which is the reported bug: at 900x600 that left 379px and + clipped every action in the Playlists header, and at 320px + it left 0px — the content was not degraded but gone. + + Here the host spans the whole content area instead and + stops being a layout participant, so the main panel keeps + its full width and the queue sits over it. The host itself + is transparent and click-through; the scrim and the panel + are what take pointer events. The containment drops paint, + which would otherwise clip the panel's own shadow. + --------------------------------------------------------- */ + :host([overlay]) { + position: absolute; + inset: 0; + width: auto; + background-color: transparent; + overflow: visible; + pointer-events: none; + contain: layout style; + z-index: 20; + } + + /* Closed, an overlay is not there at all. In flow the panel is + width: 0, which is its own way of saying this; absolutely + positioned there is no width to collapse. */ + :host([overlay]:not([open])) { + display: none; + } + + :host([overlay][open]) { + border-left: none; + } + + :host([overlay]) .panel-content { + position: absolute; + top: 0; + right: 0; + bottom: 0; + width: var(--queue-width, ${unsafeCSS(DEFAULT_WIDTH)}px); + max-width: 100%; + box-sizing: border-box; + background-color: var(--yj-bg-surface, #212529); + border-left: 1px solid var(--yj-border-subtle, #333); + box-shadow: -8px 0 24px rgb(0 0 0 / 45%); + pointer-events: auto; + } + + /* Dragging the edge of something that is already covering the + content answers a question nobody asked, and it is a + mouse-only affordance either way. */ + :host([overlay]) .resize-handle { + display: none; + } + + .scrim { + position: absolute; + inset: 0; + background-color: rgb(0 0 0 / 45%); + pointer-events: auto; + border: none; + padding: 0; + margin: 0; + cursor: pointer; + } + + /* The phone gets the whole width: below 600 there is no + "beside" left to be, and this is the shape #55 turns into a + real screen. A media query inside a shadow root is answered + by the viewport, so the component states this itself rather + than the shell reaching in. */ + @media (max-width: 599px) { + :host([overlay]) .panel-content { + width: 100%; + } + } + .resize-handle { position: absolute; top: 0; @@ -656,6 +783,20 @@ export class QueuePanel '--queue-width', `${this.panelWidth}px`, ); + + // The mode is a measurement, so it is observed rather than + // computed once: the parent is `.content-area`, whose width is + // the viewport minus the sidebar — including the sidebar's own + // collapse at 900px, which is what makes 900 the *worst* + // desktop width rather than the minimum. + this.updateOverlayMode(); + + if (this.parentElement) { + this.spaceObserver = new ResizeObserver(() => + this.updateOverlayMode(), + ); + this.spaceObserver.observe(this.parentElement); + } document.addEventListener( 'mousemove', this.handleMouseMove, @@ -690,6 +831,9 @@ export class QueuePanel super.disconnectedCallback(); this.creditsUnsub?.(); this.creditsUnsub = undefined; + this.spaceObserver?.disconnect(); + this.spaceObserver = undefined; + document.removeEventListener('keydown', this.onOverlayKeydown); document.removeEventListener( 'mousemove', this.handleMouseMove, @@ -736,7 +880,79 @@ export class QueuePanel this.delegationAttached = false; } + /** + * Decide whether the queue can afford to be a column. + * + * The parent is `.content-area`, so its width is the viewport minus + * the sidebar and the sum already accounts for the sidebar's own + * collapse. It is stable across the panel's own open/closed state + * in both modes — in flow the panel is a child of that box, and as + * an overlay it is out of flow — so this cannot oscillate. + */ + private updateOverlayMode = () => { + const available = this.parentElement?.clientWidth ?? 0; + + // Before layout there is nothing to measure, and answering 0 by + // flipping to overlay would show the scrim for a frame. + if (available === 0) return; + + this.overlay = available - this.panelWidth < MAIN_PANEL_FLOOR; + }; + + /** + * Escape closes a scrimmed overlay, which is the one keyboard rule + * every dialog in this app already follows. + * + * It is a document listener rather than a panel-scoped binding + * (`services/shortcut-scope.ts`) because it is not a *shortcut*: it + * is the dismissal of something covering the page, and it has to + * work while focus is still behind the scrim. It is attached only + * while the overlay is actually up and removed on close, so it is + * scoped to a state rather than being a permanent global. Nothing + * else binds Escape — the shortcut service only uses it to blur a + * text input. + */ + private onOverlayKeydown = (e: KeyboardEvent) => { + if (e.key !== 'Escape' || !this.open || !this.overlay) return; + + e.stopPropagation(); + this.closeFromOverlay(); + }; + + private closeFromOverlay = () => { + const hadFocus = this.contains( + document.activeElement as Node | null, + ); + + this.open = false; + + if (hadFocus) { + const back = + this.overlayOpener ?? + document.getElementById('queue-button'); + + back?.focus(); + } + + this.overlayOpener = null; + }; + override updated() { + // The overlay owns Escape only while it is up. + if (this.open && this.overlay) { + document.addEventListener('keydown', this.onOverlayKeydown); + + this.overlayOpener ??= + document.activeElement instanceof HTMLElement && + document.activeElement !== document.body + ? document.activeElement + : null; + } else { + document.removeEventListener('keydown', this.onOverlayKeydown); + + if (!this.open) this.overlayOpener = null; + } + // Closed, the panel is `width: 0` — which hides it from the eye // and from nobody else: its Clear and Add buttons still took tab // stops at x=1440 and were still read out (H-5). `inert` is the @@ -1631,6 +1847,11 @@ export class QueuePanel '--queue-width', `${clampedWidth}px`, ); + + // Widening the panel is one of the two ways the content can run + // out of room, and it is the way a viewport-width media query + // cannot see at all. + this.updateOverlayMode(); }; private handleMouseUp = () => { @@ -1714,6 +1935,15 @@ export class QueuePanel const tracks = this.queue.tracks; return html` + ${this.overlay + ? html`` + : nothing}
+ + ` + : nothing}
diff --git a/frontend/test/components/queue-overlay-mode.test.ts b/frontend/test/components/queue-overlay-mode.test.ts new file mode 100644 index 0000000..4c23c4d --- /dev/null +++ b/frontend/test/components/queue-overlay-mode.test.ts @@ -0,0 +1,175 @@ +/** + * #24 — the queue stops being a column when it cannot afford to be one. + * + * In flow the panel is `flex-shrink: 0`, so it takes its width *from + * the main panel* rather than covering it. Measured against the running + * app on the Playlists page, that left 379px of content at 900×600 — + * with all three of the page header's actions clipped — 69px at 390px + * wide, and **0px** at 320px, where the content was not degraded but + * gone. + * + * The rule is `available - panelWidth >= MAIN_PANEL_FLOOR`, and the + * reason it is a computed property rather than a `@media` block is the + * third test here: the panel's width is user state, drag-resizable + * between 200 and 500px and persisted, so a breakpoint on the viewport + * alone is wrong by up to 180px for a user who has widened it — in the + * direction that hurts, since a wider queue is exactly when the content + * can least afford it. + * + * The parent is `.content-area`, i.e. the viewport minus the sidebar, + * which is why these mount into a sized wrapper rather than into + * `document.body`: the width that decides this is the *parent's*, and + * `fixture()` would hand the panel the whole test window. + */ +import { describe, expect, it, afterEach } from 'vitest'; + +import '@components/queue-panel/queue-panel'; +import type { QueuePanel } from '@components/queue-panel/queue-panel'; +import { shadow } from '@test/support/render'; + +const wrappers: HTMLElement[] = []; + +afterEach(() => { + for (const w of wrappers.splice(0)) w.remove(); +}); + +/** + * Mount a panel inside a parent of a stated width. + * + * The wrapper is `position: relative` and `display: flex` because that + * is what `.content-area` is; the mode is measured from + * `parentElement.clientWidth`, so a wrapper that collapses to its + * content would measure the panel rather than the space around it. + */ +async function panelIn(parentWidth: number): Promise { + const wrapper = document.createElement('div'); + + wrapper.style.cssText = `position: relative; display: flex; width: ${parentWidth}px;`; + document.body.append(wrapper); + wrappers.push(wrapper); + + const el = document.createElement('queue-panel') as QueuePanel; + + el.open = true; + wrapper.append(el); + + await el.updateComplete; + await settle(el); + + return el; +} + +/** + * A ResizeObserver delivers on a frame, not a microtask, so the mode + * lands a frame after the width that decides it. + */ +async function settle(el: QueuePanel): Promise { + for (let frame = 0; frame < 4; frame += 1) { + await new Promise((r) => { + requestAnimationFrame(() => r(null)); + }); + await el.updateComplete; + } +} + +/** Drag the resize handle by `dx`, the way a user widens the panel. */ +async function dragHandleBy(el: QueuePanel, dx: number): Promise { + const handle = shadow(el, '.resize-handle'); + const startX = el.getBoundingClientRect().left; + + if (!handle) throw new Error('no resize handle to drag'); + + handle.dispatchEvent( + new MouseEvent('mousedown', { clientX: startX, bubbles: true }), + ); + document.dispatchEvent( + new MouseEvent('mousemove', { clientX: startX - dx, bubbles: true }), + ); + document.dispatchEvent(new MouseEvent('mouseup', { bubbles: true })); + + await settle(el); +} + +describe('the queue panel decides whether it can be a column', () => { + it('stays inline while the content can spare the width', async () => { + const el = await panelIn(1080); + + expect(el.overlay).toBe(false); + expect(el.hasAttribute('overlay')).toBe(false); + }); + + it('becomes an overlay when it cannot', async () => { + const el = await panelIn(700); + + expect(el.overlay).toBe(true); + expect(el.hasAttribute('overlay')).toBe(true); + }); + + /** + * The test the media query could not have passed. The parent does not + * move; only the user's own panel width does. + */ + it('flips to overlay when the user widens the panel, at a fixed width', async () => { + const el = await panelIn(880); + + expect(el.overlay).toBe(false); + + await dragHandleBy(el, 180); + + expect(el.overlay).toBe(true); + }); + + it('gives an overlay a scrim and a named way out, and an inline panel neither', async () => { + const overlaid = await panelIn(700); + + expect(shadow(overlaid, '.scrim')).toBeTruthy(); + + const close = shadow(overlaid, '[data-testid="queue-close"]'); + + expect(close?.getAttribute('aria-label')).toBe('Close queue'); + + const inline = await panelIn(1080); + + expect(inline.shadowRoot?.querySelector('.scrim')).toBeNull(); + expect( + inline.shadowRoot?.querySelector('[data-testid="queue-close"]'), + ).toBeNull(); + }); + + it('closes on the scrim, on the close button and on Escape', async () => { + for (const close of [ + (el: QueuePanel) => shadow(el, '.scrim')?.click(), + (el: QueuePanel) => + shadow(el, '[data-testid="queue-close"]')?.click(), + () => + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), + ), + ]) { + const el = await panelIn(700); + + expect(el.open).toBe(true); + + close(el); + await el.updateComplete; + + expect(el.open).toBe(false); + } + }); + + /** + * Escape belongs to the overlay, not to the queue. An inline panel is + * beside the content rather than over it, so there is nothing to + * dismiss and the key has to reach whatever else wants it. + */ + it('leaves Escape alone while inline', async () => { + const el = await panelIn(1080); + + document.dispatchEvent( + new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }), + ); + await el.updateComplete; + + expect(el.open).toBe(true); + }); +});