diff --git a/backend/config/window.go b/backend/config/window.go index 96e6e40..03f9876 100644 --- a/backend/config/window.go +++ b/backend/config/window.go @@ -12,9 +12,17 @@ const ( // MinWidth is the smallest allowed window width in pixels. Wails // enforces this at runtime; it is also the floor below which a // reported size is treated as bogus and not persisted. - MinWidth = 512 + // + // 800x600 is where the shell was measured to still work, rather + // than a round number: below ~780 the header's subtitle wraps and + // pushes the title out of the 4em top bar, and below ~600 tall the + // eleven sidebar items no longer fit at once. The previous + // 512x384 was aspirational — at 700x480 the sidebar overflowed + // behind the player bar with no scroll and Settings and Jobs could + // not be reached at all. + MinWidth = 800 // MinHeight is the smallest allowed window height in pixels. - MinHeight = 384 + MinHeight = 600 ) // WindowConfig holds window size preferences. diff --git a/e2e/specs/layout-overflow.spec.ts b/e2e/specs/layout-overflow.spec.ts new file mode 100644 index 0000000..03f9b03 --- /dev/null +++ b/e2e/specs/layout-overflow.spec.ts @@ -0,0 +1,137 @@ +import { test, expect } from '../support/fixtures.js'; + +/** + * H-7 and H-11, which are one finding seen from two distances: the app + * did not fit in its own window. + * + * `computeDefaultWidths` shared out the track list's whole + * `clientWidth` and never subtracted the 24 px favourite column or the + * 2×8 px row padding that `colBoundaryPositions` already knew about, so + * every row was 40 px wider than the box holding it (`scrollWidth 1280` + * against `clientWidth 1240`) and Duration was clipped at every size. + * + * And the enforced minimum window was 512×384, which the layout had + * never supported: at 700×480 the eleven sidebar items needed 406 px of + * a 352 px pane, `overflow: hidden` cut the last two off, nothing + * scrolled, and **Settings and Jobs could not be reached at all**. + * + * The three viewports are the two common ones and the new enforced + * minimum (`backend/config/window.go`). The minimum is the one that + * matters: it is the only size the app is *promising* to work at. + */ + +/** Keep in step with `MinWidth`/`MinHeight` in backend/config/window.go. */ +const MIN_VIEWPORT = { width: 800, height: 600 }; + +const VIEWPORTS = [ + { name: '1440×900', width: 1440, height: 900 }, + { name: '1024×768', width: 1024, height: 768 }, + { name: `the minimum (${MIN_VIEWPORT.width}×${MIN_VIEWPORT.height})`, ...MIN_VIEWPORT }, +]; + +/** + * Rows report their own overflow, which is the symptom a user sees: a + * column rendered past the edge of the row it belongs to. + */ +const rowOverflow = (page: import('@playwright/test').Page) => + page.evaluate(() => { + const list = document.querySelector('track-list'); + const root = list?.shadowRoot; + + if (!root) return { rows: 0, overflowing: [] as string[] }; + + const boxes = [ + ...root.querySelectorAll('.header-row, .track-row'), + ]; + + return { + rows: boxes.length, + overflowing: boxes + .filter((b) => b.scrollWidth > b.clientWidth) + .map((b) => `${b.className}: ${b.scrollWidth} > ${b.clientWidth}`), + }; + }); + +test.describe('the app fits in its own window', () => { + for (const vp of VIEWPORTS) { + test(`no track row overflows at ${vp.name}`, async ({ app }) => { + await app.setViewportSize({ width: vp.width, height: vp.height }); + await app.getByTestId('nav-tracks').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'tracks', + ); + + // The columns are recomputed by a ResizeObserver, so poll rather + // than read once: a single read races the resize and passes on + // the widths from the previous viewport. + await expect.poll(async () => (await rowOverflow(app)).rows).toBeGreaterThan(1); + await expect + .poll(async () => (await rowOverflow(app)).overflowing) + .toEqual([]); + }); + } + + test('every destination stays reachable at the minimum size', async ({ + app, + }) => { + await app.setViewportSize(MIN_VIEWPORT); + + // Below the breakpoint the sidebar collapses to icons; the labels + // go, the destinations do not. + const sidebar = app.locator('app-sidebar'); + + await expect + .poll(() => + sidebar.evaluate((el) => el.classList.contains('collapsed')), + ) + .toBe(true); + + // Settings and Jobs are the two that were unreachable: they are + // last in the nav, and the pane used to clip rather than scroll. + for (const view of ['jobs', 'settings'] as const) { + const item = app.getByTestId(`nav-${view}`); + + await item.scrollIntoViewIfNeeded(); + await item.click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + view, + ); + } + + // Leave the app where the next spec expects to find it. + await app.getByTestId('nav-tracks').click(); + await expect(app.getByTestId('main-content')).toHaveAttribute( + 'data-active-view', + 'tracks', + ); + }); + + test('the sidebar scrolls rather than hiding what does not fit', async ({ + app, + }) => { + await app.setViewportSize({ width: 700, height: 480 }); + + // Smaller than the enforced minimum on purpose: the window cannot + // be dragged here, but a scaled display or a large system font can + // still land the layout in it, and clipping the nav with no scroll + // is the failure that made Settings unreachable. + const reachable = await app.locator('app-sidebar').evaluate((el) => { + const settings = el.shadowRoot?.querySelector( + '[data-testid="nav-settings"]', + ); + + if (!settings) return null; + + el.scrollTop = el.scrollHeight; + + const item = settings.getBoundingClientRect(); + const pane = el.getBoundingClientRect(); + + return item.bottom <= Math.ceil(pane.bottom) && item.top >= Math.floor(pane.top); + }); + + expect(reachable).toBe(true); + }); +}); diff --git a/frontend/index.css b/frontend/index.css index b571bdb..83f5f60 100644 --- a/frontend/index.css +++ b/frontend/index.css @@ -54,6 +54,16 @@ ul { margin-top: 0; } +/* The same breakpoint the sidebar collapses at (AUTO_COLLAPSE_VIEWPORT + in app-sidebar.ts). The subtitle wrapped to two lines below ~780 px, + which made the title block 98 px tall inside a 4em bar and pushed it + down into the nav (H-11). */ +@media (max-width: 899px) { + .subtitle { + display: none; + } +} + body div.sidebar { grid-area: sidebar; background-color: var(--yj-bg-surface, #212529); diff --git a/frontend/src/components/sidebar/app-sidebar.ts b/frontend/src/components/sidebar/app-sidebar.ts index a051fca..41ca0ea 100644 --- a/frontend/src/components/sidebar/app-sidebar.ts +++ b/frontend/src/components/sidebar/app-sidebar.ts @@ -18,6 +18,14 @@ const MAX_WIDTH = 400; const DEFAULT_WIDTH = 200; const COLLAPSE_WIDTH = 142; +/** + * Below this viewport width the sidebar collapses itself to icons. + * `.collapsed` existed and only a manual drag ever reached it (H-11), + * so a small window kept a 200 px sidebar it could not afford and the + * content pane wore the whole loss. + */ +const AUTO_COLLAPSE_VIEWPORT = 900; + @customElement('app-sidebar') export class AppSidebar extends LitElement { static override styles = [designTokens, css` @@ -28,6 +36,14 @@ export class AppSidebar extends LitElement { background-color: var(--yj-bg-surface, #212529); min-width: ${MIN_WIDTH}px; max-width: ${MAX_WIDTH}px; + /* Eleven items need ~406 px and the pane is whatever the + window leaves it — 352 px at 700x480, which clipped Jobs + and Settings behind the player bar with no way to reach + them (H-11). Collapsing to icons does not help: it is a + width mode, and this is the height. */ + overflow-y: auto; + overflow-x: hidden; + scrollbar-width: thin; } .resize-handle { @@ -147,6 +163,12 @@ export class AppSidebar extends LitElement { @state() private collapsed = false; + /** The width the user chose, restored when the window grows back. */ + private userWidth = DEFAULT_WIDTH; + + private narrowViewport: MediaQueryList | null = + null; + /** Whether a track drag is in progress somewhere in the app. */ @state() private trackDragActive = false; @@ -176,6 +198,14 @@ export class AppSidebar extends LitElement { override connectedCallback() { super.connectedCallback(); this.style.width = `${DEFAULT_WIDTH}px`; + this.narrowViewport = window.matchMedia( + `(max-width: ${AUTO_COLLAPSE_VIEWPORT - 1}px)`, + ); + this.narrowViewport.addEventListener( + 'change', + this.onViewportChange, + ); + this.applyViewportWidth(); document.addEventListener( 'mousemove', this.handleMouseMove, @@ -192,6 +222,11 @@ export class AppSidebar extends LitElement { override disconnectedCallback() { super.disconnectedCallback(); + this.narrowViewport?.removeEventListener( + 'change', + this.onViewportChange, + ); + this.narrowViewport = null; document.removeEventListener( 'mousemove', this.handleMouseMove, @@ -284,8 +319,29 @@ export class AppSidebar extends LitElement { this.style.width = `${clampedWidth}px`; this.collapsed = clampedWidth < COLLAPSE_WIDTH; + this.userWidth = clampedWidth; }; + private onViewportChange = () => { + this.applyViewportWidth(); + }; + + /** + * Icons below the breakpoint, the user's own width above it. The + * width is inline (set here and by the drag handle), so this cannot + * be a media query in the stylesheet. + */ + private applyViewportWidth() { + const narrow = + this.narrowViewport?.matches ?? false; + const width = narrow + ? MIN_WIDTH + : this.userWidth; + + this.style.width = `${width}px`; + this.collapsed = width < COLLAPSE_WIDTH; + } + private handleMouseUp = () => { this.isDragging = false; }; diff --git a/frontend/src/components/track-list/track-list.ts b/frontend/src/components/track-list/track-list.ts index d6f53a1..8bf5949 100644 --- a/frontend/src/components/track-list/track-list.ts +++ b/frontend/src/components/track-list/track-list.ts @@ -70,6 +70,18 @@ const SORT_DIR_KEY = 'track-list-sort-direction'; const MIN_COLUMN_WIDTH = 50; const DEFAULT_FIXED_WIDTH = 80; +/** + * Chrome the resizable columns cannot use: the fixed favourite column + * and the row's own horizontal padding. Both numbers also appear in + * the CSS below — the `24px` first track of `--grid-cols` and + * `.track-row`/`.header-row`'s `padding: 8px` — so they live here and + * are read from both places rather than written out four times. + */ +const FAV_COL_WIDTH = 24; +const ROW_PADDING_X = 8; +const ROW_CHROME_WIDTH = + FAV_COL_WIDTH + ROW_PADDING_X * 2; + // Inline SVG paths for favorite icons — eliminates wa-icon shadow DOM // overhead (30-50 shadow roots during scroll). Font Awesome 6 paths. const FAV_ICONS = { @@ -448,7 +460,7 @@ export class TrackList private get gridTemplateColumns(): string { const cols = this.activeColumns; - const favCol = '24px'; + const favCol = `${FAV_COL_WIDTH}px`; if (this.columnWidths.length === 0 || this.columnWidths.length !== cols.length) { return ( @@ -472,10 +484,9 @@ export class TrackList private get colBoundaryPositions(): number[] { if (this.columnWidths.length === 0) return []; - const padding = 8; - const favColWidth = 24; const positions: number[] = []; - let cumulative = padding + favColWidth; + let cumulative = + ROW_PADDING_X + FAV_COL_WIDTH; for ( let i = 0; @@ -502,8 +513,18 @@ export class TrackList this.computeDefaultWidths(); } + /** + * Width the resizable columns may share. H-7: this was the raw + * `clientWidth`, which the columns then summed to exactly — so + * every row was `ROW_CHROME_WIDTH` (40 px) wider than the box it + * had to fit in and the last column was always clipped. + */ + private get availableColumnWidth(): number { + return this.clientWidth - ROW_CHROME_WIDTH; + } + private computeDefaultWidths() { - const totalWidth = this.clientWidth; + const totalWidth = this.availableColumnWidth; if (totalWidth <= 0) return; @@ -631,13 +652,14 @@ export class TrackList } /** - * Scale widths so they sum to exactly the container width. + * Scale widths so they sum to exactly the width available to the + * columns (the host minus the row chrome). * Every column is guaranteed at least MIN_COLUMN_WIDTH. */ private normalizeWidths( widths: number[], ): number[] { - const container = this.clientWidth; + const container = this.availableColumnWidth; if (container <= 0 || widths.length === 0) { return widths; @@ -731,7 +753,7 @@ export class TrackList private onColResizeMove = (e: MouseEvent) => { if (this.resizingColumn === null) return; - const container = this.clientWidth; + const container = this.availableColumnWidth; if (container <= 0) return; diff --git a/frontend/test/components/track-list-width.test.ts b/frontend/test/components/track-list-width.test.ts new file mode 100644 index 0000000..ea31629 --- /dev/null +++ b/frontend/test/components/track-list-width.test.ts @@ -0,0 +1,111 @@ +/** + * `H-7`: `computeDefaultWidths` distributed the host's whole + * `clientWidth` across the resizable columns, while every row spends + * 24 px on the favourite column and 2×8 px on its own padding before + * the first one starts. So the grid was always exactly 40 px wider + * than the box it had to fit in, and the last column was clipped on + * every row at every window size — measured in the running app at + * `scrollWidth 1280` against `clientWidth 1240`. + * + * The e2e spec (`e2e/specs/layout-overflow.spec.ts`) asserts it in the + * real shell at three viewports. This is the cheap guard on the + * arithmetic itself, because the failure is silent: the row still + * renders, nothing throws, and the only symptom is a column you cannot + * read. + */ +import { describe, expect, it, beforeEach } from 'vitest'; +import type { LitElement } from 'lit'; + +import '@components/track-list/track-list'; +import { fixture, shadow, shadowAll } from '@test/support/render'; + +/** The chrome a row spends before the first resizable column. */ +const FAV_COL = 24; +const ROW_PADDING_X = 8; + +const TRACKS = Array.from({ length: 40 }, (_, i) => ({ + FilePath: `/music/track-${i}.mp3`, + TrackName: `Track ${i}`, + ArtistName: 'An Artist', + Album: 'An Album', + Duration: 180, +})) as never[]; + +/** Sum the px tracks of a `grid-template-columns` value. */ +function trackWidths(el: Element | null): number[] { + if (el === null) throw new Error('no element to measure'); + + return getComputedStyle(el) + .gridTemplateColumns.split(/\s+/) + .filter(Boolean) + .map((t) => parseFloat(t)) + .filter((n) => !Number.isNaN(n)); +} + +describe(' column arithmetic', () => { + let el: LitElement; + /** What the columns may spend: the host, minus the row's padding. */ + let budget: number; + + beforeEach(async () => { + // A saved layout is normalised against the same budget, so the + // default path is the one worth pinning. + localStorage.removeItem('track-list-column-widths'); + + el = await fixture('track-list', { + externalTracks: TRACKS, + }); + el.style.height = '600px'; + await el.updateComplete; + await new Promise((r) => setTimeout(r, 60)); + + budget = el.clientWidth - ROW_PADDING_X * 2; + }); + + it('leaves room for the favourite column and the row padding', () => { + const widths = trackWidths(shadow(el, '.header-row')); + const total = widths.reduce((a, b) => a + b, 0); + + expect(total).toBeLessThanOrEqual(budget); + }); + + it('spends the whole budget and no more', () => { + // Not merely "does not overflow": a fix that under-filled would + // pass that and leave a gap down the right of every row. + const widths = trackWidths(shadow(el, '.header-row')); + const total = widths.reduce((a, b) => a + b, 0); + + expect(total).toBe(budget); + expect(widths[0]).toBe(FAV_COL); + }); + + it('renders no row wider than the row itself', () => { + const rows = shadowAll(el, '.track-row'); + + expect(rows.length).toBeGreaterThan(0); + expect( + rows.filter((r) => r.scrollWidth > r.clientWidth), + ).toHaveLength(0); + }); + + it('keeps the header aligned with the rows it heads', () => { + // The regression this catches is the one a screenshot found twice + // in this plan: columns that no longer line up with their header. + expect(trackWidths(shadow(el, '.header-row'))).toEqual( + trackWidths(shadow(el, '.track-row')), + ); + }); + + it('places the resize handles on the boundaries it just measured', () => { + // `colBoundaryPositions` starts from the same two numbers + // `computeDefaultWidths` subtracts. They were written out + // separately, which is how they came to disagree. + const widths = trackWidths(shadow(el, '.header-row')); + const handles = shadowAll(el, '.col-resize-handle'); + + expect(handles).toHaveLength(widths.length - 2); + expect(parseFloat(handles[0]?.style.left ?? '0')).toBe( + ROW_PADDING_X + FAV_COL + (widths[1] ?? 0), + ); + }); +});