From ea3edde69705fb01ce682634e25bebc74f10ec2b Mon Sep 17 00:00:00 2001 From: Logan Date: Fri, 21 Aug 2026 04:40:35 -0400 Subject: [PATCH] fix(explore): scroll the album page as one on a phone `explore-album-details` was a fixed header over a scrolling tracklist, which is the desktop arrangement. At the reference device's 424x439 the header owned 253 of the panel's 318px and the list scrolled inside the 64 that were left, and the header's flex row squeezed `.album-info` to 112px beside a 200px cover -- so the title drew as one ellipsised glyph and two of the album's three primary actions were clipped by the component's own `overflow: hidden`: "Shuffle album" ended at x=443 in a 424px box, reachable by no gesture. Below 600px the host is the scroller and `.content` stops being one, so the header scrolls away and the page moves together; the header stacks art over info, so the info column has the row's whole width. The tracklist is plain DOM rather than a virtualizer, so nothing inside wants a scroll window of its own. Another `min-width: 0` was not the fix and the issue's own measurement says so: `.album-info` carries one and was shrinking as asked. Nor could `layout-overflow.spec.ts` see any of this -- `body.scrollWidth` equalled the viewport throughout, because the overflow was inside a component -- so the new spec measures each header control against the host's own box, which is `top-bar-fit.spec.ts`'s shape for the same reason. The phone block is last in the stylesheet on `index.css`'s rule: a media query adds no specificity, so above the rules it overrides every declaration in it would be silently dead. Closes #66 --- CLAUDE.md | 25 +++ e2e/specs/phone-album-page.spec.ts | 209 ++++++++++++++++++ .../explore-album-details.ts | 61 +++++ 3 files changed, 295 insertions(+) create mode 100644 e2e/specs/phone-album-page.spec.ts diff --git a/CLAUDE.md b/CLAUDE.md index 87510e7..904af8c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -2489,6 +2489,31 @@ missing half; `catalogFailed` is the only route to `unavailable` now, and the timer is a 60 s backstop for a genuine hang rather than the verdict. +**On a phone that page is one scroll container, and the header is in +it** (#66). It was built as a fixed header over a scrolling tracklist, +which is the desktop arrangement: at the reference device's 424×439 the +header owned **253 of the panel's 318px** and the list scrolled inside +the 64 that were left. Below 600px the *host* is the scroller and +`.content` stops being one, so the whole page moves together — which is +only available because this tracklist is plain DOM rather than a +virtualizer, and because `.main-panel > *` already gives the host a +definite height. + +Three things about it are load-bearing. **Another `min-width: 0` was +not the fix**: `.album-info` carries one and was shrinking exactly as +asked, to 112px beside a 200px cover — so the title drew as `G…` and +"Shuffle album" ended at x=443 inside a 424px box, clipped by the +component's own `overflow: hidden` and reachable by no gesture. A row +with a fixed-size sibling has to **stack** at that width, or the column +that must shrink has nothing to be wide with. **`layout-overflow.spec.ts` +cannot see any of this** — `body.scrollWidth` equalled the viewport +throughout, because the overflow was *inside* a component; the spec +measures each header control against the host's own box, which is +`top-bar-fit.spec.ts`'s shape for the same reason. And **the phone block +is last in the stylesheet**, on `index.css`'s rule: a media query adds +no specificity, so written above the plain rules it overrides every +declaration in it is silently dead. + **Activating a row plays the list the row is in, from that row.** A double-click — and Play on a single row's context menu — queues the list as *displayed* with `startIndex` on that row, not a queue of one diff --git a/e2e/specs/phone-album-page.spec.ts b/e2e/specs/phone-album-page.spec.ts new file mode 100644 index 0000000..7889541 --- /dev/null +++ b/e2e/specs/phone-album-page.spec.ts @@ -0,0 +1,209 @@ +import { test, expect } from '../support/fixtures.js'; +import type { Page } from '@playwright/test'; + +/** + * The album page on a phone (#66). + * + * Two faults, and neither was visible to `layout-overflow.spec.ts`: + * that spec asserts the *shell* needs no sideways scrolling, and the + * shell was correct throughout — `body.scrollWidth === clientWidth` + * while `explore-album-details` itself measured 443 inside a 424px box + * and clipped two of the album's three primary actions with its own + * `overflow: hidden`. So the measurement here is **per control against + * the component's box**, which is the same shape `top-bar-fit.spec.ts` + * needed for the same reason. + * + * The other half is the scroll: the page was a fixed header over a + * scrolling tracklist, so at the reference device's 424x439 the header + * owned 253 of the panel's 318px and the list scrolled in the 64 that + * were left. It is one scroll container below 600px, which is a + * property of the *host* rather than of `.content`. + * + * The engine is the caveat this tier cannot close: the reference device + * renders in Chrome 113 and this is Chromium/WebKit. A flex direction + * and a scroll container are nowhere near that engine's documented gaps + * (relaxed nesting, the Popover API, `light-dark()`), but "it renders + * at that size in Chromium" is not evidence about the phone. + */ + +/** The phone this was measured on, in CSS pixels. */ +const DEVICE = { width: 424, height: 439 }; + +const details = (page: Page) => page.locator('explore-album-details'); + +/** The page's own boxes, read from inside its shadow root. */ +const geometry = (page: Page) => + page.evaluate(() => { + const host = document.querySelector('explore-album-details'); + const sr = host?.shadowRoot; + + if (!host || !sr) return null; + + const box = (sel: string) => { + const el = sr.querySelector(sel); + + if (!el) return null; + + const r = el.getBoundingClientRect(); + + return { width: Math.round(r.width), right: Math.round(r.right) }; + }; + + const content = sr.querySelector('.content'); + + return { + hostWidth: host.clientWidth, + hostScrollWidth: host.scrollWidth, + // The host is the scroller below 600px, so the page is taller + // than its box rather than the tracklist being a window inside it. + hostScrolls: host.scrollHeight > host.clientHeight, + contentScrolls: content + ? content.scrollHeight > content.clientHeight + : null, + header: box('.album-header'), + play: box('[data-testid="album-play"]'), + shuffle: box('[data-testid="album-shuffle"]'), + queue: box('[data-testid="album-queue"]'), + title: (() => { + const el = sr.querySelector('.album-title-text'); + + return el ? el.scrollWidth <= el.clientWidth + 1 : null; + })(), + }; + }); + +test.describe('the album page on a phone', () => { + test.beforeEach(async ({ app }) => { + await app.setViewportSize(DEVICE); + await openFirstAlbum(app); + }); + + test.afterEach(async ({ app }) => { + await app.setViewportSize({ width: 1440, height: 900 }); + await app.getByTestId('nav-tracks').click(); + }); + + test('keeps every action inside its own box', async ({ app }) => { + const geo = await geometry(app); + + expect(geo).not.toBeNull(); + // "Shuffle album" ended at x=443 in a 424px component and could not + // be reached by any gesture; "Add to queue" at 440. + for (const action of ['play', 'shuffle', 'queue'] as const) { + expect( + geo?.[action], + `${action} is rendered`, + ).not.toBeNull(); + expect( + geo?.[action]?.right ?? 0, + `${action} ends inside the page`, + ).toBeLessThanOrEqual(geo?.hostWidth ?? 0); + } + + expect(geo?.hostScrollWidth).toBe(geo?.hostWidth); + expect(geo?.header?.width).toBe(geo?.hostWidth); + }); + + test('gives the title the row rather than one glyph of it', async ({ + app, + }) => { + // `.album-info` was squeezed to 112px beside the art, so an album + // called *Glass Harbour* drew as `G…`. It carries `min-width: 0` + // and was shrinking as asked — the row had to stack. + expect(await geometry(app).then((g) => g?.title)).toBe(true); + }); + + test('scrolls as one page, with the header scrolling away', async ({ + app, + }) => { + const before = await geometry(app); + + expect(before?.hostScrolls).toBe(true); + expect(before?.contentScrolls).toBe(false); + + const headerTop = () => + app.evaluate( + () => + document + .querySelector('explore-album-details') + ?.shadowRoot?.querySelector('.album-header') + ?.getBoundingClientRect().top ?? 0, + ); + + expect(await headerTop()).toBeGreaterThanOrEqual(0); + + // A wheel gesture, not `scrollTop`: `overflow: hidden` still permits + // programmatic scrolling, so a probe that assigns it passes on the + // build this exists to fail. + await details(app).hover(); + await app.mouse.wheel(0, 250); + + await expect.poll(headerTop).toBeLessThan(-100); + }); + + test('is the desktop arrangement again above the breakpoint', async ({ + app, + }) => { + await app.setViewportSize({ width: 1024, height: 800 }); + + // The same element, re-laid-out: one component with two + // arrangements, not a phone-only copy. + await expect + .poll(async () => (await geometry(app))?.hostScrolls) + .toBe(false); + + const arrangement = await app.evaluate(() => { + const sr = document.querySelector('explore-album-details')?.shadowRoot; + const header = sr?.querySelector('.album-header'); + const content = sr?.querySelector('.content'); + + return { + direction: header ? getComputedStyle(header).flexDirection : null, + contentOverflow: content ? getComputedStyle(content).overflowY : null, + }; + }); + + expect(arrangement.direction).toBe('row'); + expect(arrangement.contentOverflow).toBe('auto'); + }); +}); + +/** Albums → the second card, which navigates to the album page. */ +async function openFirstAlbum(app: Page): Promise { + // Below 600px the sidebar is gone; the tab bar is the navigation. + await app.getByTestId('tab-albums').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'albums', + ); + + await expect.poll(() => cardCount(app)).toBeGreaterThan(1); + + // Dispatched rather than clicked: the card lives in a virtualizer + // inside a shadow root, and Enter expands the dropdown instead. + await app.evaluate(() => { + document + .querySelector('cover-grid') + ?.shadowRoot?.querySelectorAll('.album-card')[1] + ?.dispatchEvent( + new MouseEvent('click', { bubbles: true, composed: true }), + ); + }); + + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'explore-album-details', + ); + await expect( + details(app).locator('[data-testid="album-play"]'), + ).toBeVisible(); +} + +async function cardCount(app: Page): Promise { + return app.evaluate( + () => + document + .querySelector('cover-grid') + ?.shadowRoot?.querySelectorAll('.album-card').length ?? 0, + ); +} diff --git a/frontend/src/components/explore-album-details/explore-album-details.ts b/frontend/src/components/explore-album-details/explore-album-details.ts index 71bc799..e6e3758 100644 --- a/frontend/src/components/explore-album-details/explore-album-details.ts +++ b/frontend/src/components/explore-album-details/explore-album-details.ts @@ -892,6 +892,67 @@ export class ExploreAlbumDetails extends LitElement implements ContextMenuHost { .track-row .track-request { flex-shrink: 0; } + + /* ── The phone (#66) ── + * + * **This block is last on purpose**, for index.css's + * reason: a media query adds no specificity, so a rule + * written above the plain one it overrides loses to it and + * every declaration here is silently dead. + * + * Two faults, one shape. The page is a fixed header over a + * scrolling tracklist — the desktop arrangement — so at the + * reference device's 424x439 the header owned 253 of the + * panel's 318px and the tracklist scrolled inside the 64px + * that were left. And the header's flex row squeezed + * .album-info to 112px, so the title drew as one ellipsised + * glyph and two of the album's three primary actions were + * clipped by the host's own overflow: Shuffle album ended + * at x=443 in a 424px box, unreachable by any gesture. + * + * .album-info carries min-width: 0 and was shrinking as + * asked, so another one is not the fix — the row has to + * stack, or the info column has nothing to be wide with. + * + * The scroller moves to the host and .content stops being + * one, which is what makes the header scroll away; the + * tracklist is plain DOM rather than a virtualizer, so + * nothing inside wants a scroll window of its own. */ + @media (max-width: 599px) { + :host { + overflow-y: auto; + } + + .album-header { + flex-direction: column; + align-items: flex-start; + gap: 12px; + padding: 12px 16px; + } + + /* Stacked, the art is the whole of the header's width + * budget and its 200px square is 45% of the reference + * device's height. It is still what identifies the + * album, so it shrinks rather than going. */ + .cover-art-container { + width: 140px; + height: 140px; + } + + /* A column flex item takes its content's width from + * align-items: flex-start above, which would leave the + * actions wrapping inside a box narrower than the row + * they now have to themselves. */ + .album-info { + align-self: stretch; + } + + .content { + flex: 0 0 auto; + overflow-y: visible; + padding: 16px 16px 24px; + } + } `, ];