From 351798fd66b2a2781bcbcd7700fa639cae5d3fae Mon Sep 17 00:00:00 2001 From: Caleb Allen Date: Mon, 17 Aug 2026 22:09:50 -0400 Subject: [PATCH] fix(ui): spend a row's leftover space on the gaps, not the margins The three card grids -- albums, artists, genres -- laid out with `justify: 'center'` and a fixed 8px gap and padding, which gives the row a fixed width and pushes everything left over to the two margins. Measured on a 1440px window: cards 16px apart inside 78px of nothing down each side. The outside was five times the inside. `utils/grid-spacing.ts` computes one number instead, from what the row could not spend on another card: the same value between two cards, between two rows, and down each edge. That window now reads 30px outside against 34px between, and it holds at any width. The virtualizer has a word for this -- `justify: 'space-evenly'` with `gap: 'auto'` -- and it cannot be used. It fits `floor(width / cardWidth)` columns without reserving the gap it is about to need, so a width one card short of exact leaves seven cards a pixel apart. On the window above it would fit 7 columns with 1px between them. Deciding the column count here is what puts a floor under the spacing. Two consequences. The layout is rebuilt when the container width changes the spacing rather than only when the cover size changes, so each grid observes its own scroller -- keyed on the spacing, or every pixel of a drag rebuilds a layout that comes out the same. And `cover-grid`'s ScrollManager took `GRID_GAP`/`GRID_PADDING` as constants, which stopped describing anything the moment the spacing became elastic: it asks the host for the geometry now, since a scroll position rebuilt from a stale 8px lands in the wrong row. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MeQt5hgXg5YGoNZQ9ozG7L --- .../components/artists-view/artists-view.ts | 62 ++++++++-- .../src/components/cover-grid/cover-grid.ts | 106 +++++++++++++++--- .../components/cover-grid/scroll-manager.ts | 64 ++++++----- .../src/components/genres-view/genres-view.ts | 62 ++++++++-- frontend/src/utils/grid-spacing.ts | 58 ++++++++++ 5 files changed, 292 insertions(+), 60 deletions(-) create mode 100644 frontend/src/utils/grid-spacing.ts diff --git a/frontend/src/components/artists-view/artists-view.ts b/frontend/src/components/artists-view/artists-view.ts index dba2319..209dc49 100644 --- a/frontend/src/components/artists-view/artists-view.ts +++ b/frontend/src/components/artists-view/artists-view.ts @@ -10,6 +10,7 @@ import type { VisibilityChangedEvent, } from '@lit-labs/virtualizer'; import { grid } from '@lit-labs/virtualizer/layouts/grid.js'; +import { gridSpacingFor } from '@utils/grid-spacing'; import { GetAlbumsByArtist, GetFilePathsByAlbums, @@ -147,8 +148,6 @@ export class ArtistsView // ----- Grid spacing constants ----- - private static readonly GRID_GAP = 8; - private static readonly GRID_PADDING = 8; private static readonly CARD_PADDING = 5; private get imageSize(): number { @@ -177,20 +176,41 @@ export class ArtistsView private createGridLayout() { const w = this.cardSize ?? CARD_SIZE_DEFAULT; const h = w + this.cardTextHeight; - const gap = ArtistsView.GRID_GAP; - const pad = ArtistsView.GRID_PADDING; + + // One number for the gap, the row gap and the padding: whatever + // a row could not spend on another card, shared out equally, so + // the outside is never wider than the inside. See + // `utils/grid-spacing.ts`. + const spacing = this.spacingFor(this.containerWidth); + + this.lastLayoutSpacing = spacing; return grid({ itemSize: { width: `${w}px`, height: `${h}px`, }, - gap: `${gap}px`, - padding: `${pad}px`, - justify: 'center', + gap: `${spacing}px`, + padding: `${spacing}px`, + justify: 'start', }); } + /** The width the grid lays itself out in. */ + private get containerWidth(): number { + return ( + this.renderRoot?.querySelector( + '.grid-scroll-container', + )?.clientWidth || + this.clientWidth || + 0 + ); + } + + private spacingFor(width: number): number { + return gridSpacingFor(width, this.cardSize); + } + /** Sort direction for the artist grid. * * There is only one key to sort by: `library.Artist` carries a @@ -478,6 +498,8 @@ export class ArtistsView override disconnectedCallback() { super.disconnectedCallback(); this.detachWheelListener(); + this.gridResizeObserver?.disconnect(); + this.gridResizeObserver = null; } /** The wheel listener and the scroll debounce belong to the grid @@ -730,10 +752,34 @@ export class ArtistsView * ================================================================ */ private lastLayoutWidth = 0; + private lastLayoutSpacing = 0; + + /** Watches the scroller so a window resize rebuilds the layout: + * the spacing is derived from its width, and nothing else asks + * this view to update when only that changes. */ + private gridResizeObserver: ResizeObserver | null = null; + + private observeGridWidth() { + const container = + this.renderRoot?.querySelector( + '.grid-scroll-container', + ); + + if (!container || this.gridResizeObserver) return; + + this.gridResizeObserver = new ResizeObserver(() => + this.requestUpdate(), + ); + this.gridResizeObserver.observe(container); + } private updateGridLayout() { + this.observeGridWidth(); + if ( - this.cardSize === this.lastLayoutWidth + this.cardSize === this.lastLayoutWidth && + this.lastLayoutSpacing === + this.spacingFor(this.containerWidth) ) { return; } diff --git a/frontend/src/components/cover-grid/cover-grid.ts b/frontend/src/components/cover-grid/cover-grid.ts index 6b3056d..540f4ad 100644 --- a/frontend/src/components/cover-grid/cover-grid.ts +++ b/frontend/src/components/cover-grid/cover-grid.ts @@ -19,6 +19,7 @@ import { LibraryController } from '@store/controllers/library-controller'; import { SearchController } from '@store/controllers/search-controller'; import { ViewLifecycleMixin } from '@utils/view-lifecycle'; import { RovingGridController } from '@utils/roving-grid'; +import { gridColumnsFor, gridSpacingFor } from '@utils/grid-spacing'; import { queueStore } from '@store/queue-store'; import type { QueueSource } from '@store/queue-store'; import '@awesome.me/webawesome/dist/components/popup/popup.js'; @@ -97,19 +98,36 @@ export class CoverGrid private lastAlbumsRef: library.Album[] | null = null; - // Fixed grid spacing constants. - private static readonly GRID_GAP = 8; - private static readonly GRID_PADDING = 8; private static readonly CARD_PADDING = 5; private ctxMenu = new ContextMenuController(this); private favCtrl = new FavoritesController(this); private selMgr = new AlbumSelectionManager(); private scrollMgr = new ScrollManager(this, { - GRID_GAP: CoverGrid.GRID_GAP, - GRID_PADDING: CoverGrid.GRID_PADDING, + columnsFor: (width: number) => this.columnsFor(width), + spacingFor: (width: number) => this.spacingFor(width), }); + /** + * How many cards fit across `width`, by the same arithmetic the + * virtualizer's `space-evenly` grid uses — no gap and no padding + * are reserved, because both come out of what is left over. + * + * The scroll manager restores a position by rebuilding the grid's + * geometry, so this and `spacingFor` must agree with the layout + * rather than approximate it; they were two constants that no + * longer describe anything once the spacing became elastic. + */ + columnsFor(width: number): number { + return gridColumnsFor(width, this.cardWidth); + } + + /** The spacing that width produces: between columns, between rows, + * and around the outside, all the same number. */ + spacingFor(width: number): number { + return gridSpacingFor(width, this.cardWidth); + } + private lastSelectedAlbumIndex: number | null = null; private lastSelectedTrackIndex: number | null = null; @@ -148,10 +166,30 @@ export class CoverGrid } // Virtualizer grid layout instance — recreated when - // the card size changes. + // the card size or the container width changes. private gridLayout = this.createGridLayout(); private gridLayoutWidth = 0; + /** The spacing the current layouts were built with. */ + private gridLayoutSpacing = 0; + + /** Watches the scroll container so a window resize rebuilds the + * layout: the spacing is derived from its width, and nothing else + * asks this component to update when only that changes. */ + private gridResizeObserver: ResizeObserver | null = + null; + + private observeGridWidth(): void { + const container = this.scrollContainer; + + if (!container || this.gridResizeObserver) return; + + this.gridResizeObserver = new ResizeObserver( + () => this.requestUpdate(), + ); + this.gridResizeObserver.observe(container); + } + /** * Secondary layout for the "after" virtualizer in * split mode. Uses zero top padding so there is no @@ -169,22 +207,49 @@ export class CoverGrid } const h = w + this.cardTextHeight; - const gap = CoverGrid.GRID_GAP; - const pad = CoverGrid.GRID_PADDING; + + // The spacing is whatever the row could not spend on another + // card, shared out equally — so it is the same number between + // two cards, between two rows, and down each outside edge. + // See `utils/grid-spacing.ts` for why it is computed rather + // than handed to the virtualizer as `space-evenly`. + const spacing = this.spacingFor( + this.containerWidth, + ); + + if (!noTopPad) { + this.gridLayoutSpacing = spacing; + } return grid({ itemSize: { width: `${w}px`, height: `${h}px`, }, - gap: `${gap}px`, + gap: `${spacing}px`, padding: noTopPad - ? `0 ${pad}px ${pad}px` - : `${pad}px`, - justify: 'center', + ? `0 ${spacing}px ${spacing}px` + : `${spacing}px`, + justify: 'start', }); } + /** + * The width the grid lays itself out in. + * + * Read from the scroll container when there is one; before the + * first render there is not, and the fallback only has to be + * plausible — the layout is rebuilt from the real width as soon as + * one exists. + */ + private get containerWidth(): number { + return ( + this.scrollContainer?.clientWidth || + this.clientWidth || + 0 + ); + } + private dragImageEl: HTMLElement | null = null; // -- Memoisation caches for filtered albums -- @@ -466,6 +531,9 @@ export class CoverGrid ); this.wheelListenerAttached = false; + this.gridResizeObserver?.disconnect(); + this.gridResizeObserver = null; + this.scrollMgr.teardown(); this.scrollMgr.revealContainer( this.scrollContainer, @@ -603,10 +671,18 @@ export class CoverGrid this.wheelListenerAttached = true; } - // Recreate the virtualizer grid layout when - // the card size changes. + this.observeGridWidth(); + + // Recreate the virtualizer grid layout when the card size + // changes — or when the spacing the container width produces + // does, since that is now a derived number rather than a + // constant. Keyed on the spacing rather than on the width, or + // every pixel of a drag rebuilds a layout that would come out + // the same. const cardSizeChanged = - this.gridLayoutWidth !== this.cardWidth; + this.gridLayoutWidth !== this.cardWidth || + this.gridLayoutSpacing !== + this.spacingFor(this.containerWidth); if (cardSizeChanged) { this.gridLayout = this.createGridLayout(); diff --git a/frontend/src/components/cover-grid/scroll-manager.ts b/frontend/src/components/cover-grid/scroll-manager.ts index 03bc000..7f4ce93 100644 --- a/frontend/src/components/cover-grid/scroll-manager.ts +++ b/frontend/src/components/cover-grid/scroll-manager.ts @@ -6,12 +6,21 @@ import type { LibraryController } from '@store/controllers/library-controller'; import type { GridEntry } from './cover-grid-types.js'; /** - * Grid spacing constants shared between the scroll - * manager and the host component. + * Grid geometry, asked of the host rather than written down. + * + * These were two constants, `GRID_GAP` and `GRID_PADDING`, which stopped + * describing anything the moment the grid's spacing became elastic: the + * gap, the padding and the column count are all derived from the + * container width now, and a scroll position rebuilt from a stale 8px + * lands in the wrong row. */ export interface GridConstants { - readonly GRID_GAP: number; - readonly GRID_PADDING: number; + /** Columns that fit across `width`. */ + columnsFor(width: number): number; + + /** The spacing `width` produces — between columns, between rows, + * and around the outside, all the same number. */ + spacingFor(width: number): number; } /** @@ -275,8 +284,8 @@ export class ScrollManager { return; } - const gap = this.gc.GRID_GAP; - const pad = this.gc.GRID_PADDING; + const gap = this.spacing(container); + const pad = gap; const rowStep = this.host.cardHeight + gap; @@ -293,7 +302,7 @@ export class ScrollManager { () => { const rowStep = this.host.cardHeight + - this.gc.GRID_GAP; + this.spacing(container); if (this.pendingFocus === null) { this.isResizing = true; @@ -351,7 +360,7 @@ export class ScrollManager { container: HTMLElement, rowStep: number, ): void { - const pad = this.gc.GRID_PADDING; + const pad = this.spacing(container); const cols = this.currentColumnCount; const filtered = this.host.cachedFilteredAlbums; @@ -410,17 +419,15 @@ export class ScrollManager { ): number { if (!container) return 1; - const gap = this.gc.GRID_GAP; - const pad = this.gc.GRID_PADDING; - const availableWidth = - container.clientWidth - pad * 2; + return this.gc.columnsFor( + container.clientWidth, + ); + } - return Math.max( - 1, - Math.floor( - (availableWidth + gap) / - (this.host.cardWidth + gap), - ), + /** The grid's current spacing, which is also its padding. */ + private spacing(container?: HTMLElement): number { + return this.gc.spacingFor( + container?.clientWidth ?? 800, ); } @@ -439,7 +446,7 @@ export class ScrollManager { container?: HTMLElement, ): number { const cols = this.getColumnCount(container); - const gap = this.gc.GRID_GAP; + const gap = this.spacing(container); return ( cols * this.host.cardWidth + @@ -460,7 +467,7 @@ export class ScrollManager { const cols = this.getColumnCount(container); const colIndex = idx % cols; - const gap = this.gc.GRID_GAP; + const gap = this.spacing(container); return ( colIndex * @@ -597,8 +604,8 @@ export class ScrollManager { if (!this.host.splitMode) return raw; - const gap = this.gc.GRID_GAP; - const pad = this.gc.GRID_PADDING; + const gap = this.spacing(container); + const pad = gap; const columns = this.getColumnCount(container); const rowStep = this.host.cardHeight + gap; @@ -678,8 +685,8 @@ export class ScrollManager { if (expandedIndex < 0) return; - const gap = this.gc.GRID_GAP; - const pad = this.gc.GRID_PADDING; + const gap = this.spacing(container); + const pad = gap; const columns = this.getColumnCount(container); const rowStep = this.host.cardHeight + gap; @@ -772,8 +779,8 @@ export class ScrollManager { if (idx < 0) return; - const gap = this.gc.GRID_GAP; - const pad = this.gc.GRID_PADDING; + const gap = this.spacing(container); + const pad = gap; const cols = this.getColumnCount(container); const rowStep = this.host.cardHeight + gap; @@ -854,9 +861,8 @@ export class ScrollManager { this.getExpandedAlbumIndex(); if (idx >= 0) { - const gap = this.gc.GRID_GAP; - const pad = - this.gc.GRID_PADDING; + const gap = this.spacing(container); + const pad = gap; const cols = this.getColumnCount( container, diff --git a/frontend/src/components/genres-view/genres-view.ts b/frontend/src/components/genres-view/genres-view.ts index fb599b2..92e45f7 100644 --- a/frontend/src/components/genres-view/genres-view.ts +++ b/frontend/src/components/genres-view/genres-view.ts @@ -10,6 +10,7 @@ import type { VisibilityChangedEvent, } from '@lit-labs/virtualizer'; import { grid } from '@lit-labs/virtualizer/layouts/grid.js'; +import { gridSpacingFor } from '@utils/grid-spacing'; import { GetFilePathsByGenres, } from '@go/library/library.js'; @@ -155,8 +156,6 @@ export class GenresView // ----- Grid spacing constants ----- - private static readonly GRID_GAP = 8; - private static readonly GRID_PADDING = 8; private static readonly CARD_PADDING = 5; private get imageSize(): number { @@ -185,20 +184,41 @@ export class GenresView private createGridLayout() { const w = this.cardSize ?? CARD_SIZE_DEFAULT; const h = w + this.cardTextHeight; - const gap = GenresView.GRID_GAP; - const pad = GenresView.GRID_PADDING; + + // One number for the gap, the row gap and the padding: whatever + // a row could not spend on another card, shared out equally, so + // the outside is never wider than the inside. See + // `utils/grid-spacing.ts`. + const spacing = this.spacingFor(this.containerWidth); + + this.lastLayoutSpacing = spacing; return grid({ itemSize: { width: `${w}px`, height: `${h}px`, }, - gap: `${gap}px`, - padding: `${pad}px`, - justify: 'center', + gap: `${spacing}px`, + padding: `${spacing}px`, + justify: 'start', }); } + /** The width the grid lays itself out in. */ + private get containerWidth(): number { + return ( + this.renderRoot?.querySelector( + '.grid-scroll-container', + )?.clientWidth || + this.clientWidth || + 0 + ); + } + + private spacingFor(width: number): number { + return gridSpacingFor(width, this.cardSize); + } + /** Sort key and direction for the genre grid (H-19: it had none). */ @state() private sortField: 'name' | 'tracks' = 'name'; @@ -483,6 +503,8 @@ export class GenresView override disconnectedCallback() { super.disconnectedCallback(); this.detachWheelListener(); + this.gridResizeObserver?.disconnect(); + this.gridResizeObserver = null; } /** See artists-view: off-screen the grid cannot be scrolled, and @@ -737,10 +759,34 @@ export class GenresView * ================================================================ */ private lastLayoutWidth = 0; + private lastLayoutSpacing = 0; + + /** Watches the scroller so a window resize rebuilds the layout: + * the spacing is derived from its width, and nothing else asks + * this view to update when only that changes. */ + private gridResizeObserver: ResizeObserver | null = null; + + private observeGridWidth() { + const container = + this.renderRoot?.querySelector( + '.grid-scroll-container', + ); + + if (!container || this.gridResizeObserver) return; + + this.gridResizeObserver = new ResizeObserver(() => + this.requestUpdate(), + ); + this.gridResizeObserver.observe(container); + } private updateGridLayout() { + this.observeGridWidth(); + if ( - this.cardSize === this.lastLayoutWidth + this.cardSize === this.lastLayoutWidth && + this.lastLayoutSpacing === + this.spacingFor(this.containerWidth) ) { return; } diff --git a/frontend/src/utils/grid-spacing.ts b/frontend/src/utils/grid-spacing.ts new file mode 100644 index 0000000..74dfbbe --- /dev/null +++ b/frontend/src/utils/grid-spacing.ts @@ -0,0 +1,58 @@ +/** + * Even spacing for the three card grids — albums, artists, genres. + * + * All three used `justify: 'center'` with a fixed 8px gap and 8px + * padding, which gives the row a fixed width and pushes everything left + * over to the two margins: on a 1440px window the albums grid drew its + * cards 16px apart inside 78px of nothing down each side. The outside + * was five times the inside. + * + * The fix is to spend the leftover on the spacing instead, so there is + * one number: between two cards, between two rows, and down each edge. + * The virtualizer has a word for that — `justify: 'space-evenly'` with + * `gap: 'auto'` — and it cannot be used, because it fits + * `floor(width / cardWidth)` columns without reserving the gap it is + * about to need: a width one card short of exact fits seven cards a + * pixel apart. Deciding the column count here is what puts a floor + * under the spacing, and the grid is then given plain numbers. + */ + +/** The narrowest the spacing is allowed to get. */ +export const MIN_GRID_SPACING = 8; + +/** + * How many cards of `cardWidth` fit across `width`. + * + * A row of c cards spends c×cardWidth on cards and (c+1)×spacing on the + * spaces between and beside them, so c is bounded by + * (width − spacing) / (cardWidth + spacing) at the minimum spacing. + */ +export function gridColumnsFor( + width: number, + cardWidth: number, +): number { + if (cardWidth <= 0) return 1; + + const fit = Math.floor( + (width - MIN_GRID_SPACING) / (cardWidth + MIN_GRID_SPACING), + ); + + return Math.max(1, fit); +} + +/** + * The spacing `width` produces — the gap, the row gap and the padding, + * which are all the same number. + */ +export function gridSpacingFor( + width: number, + cardWidth: number, +): number { + const columns = gridColumnsFor(width, cardWidth); + const leftover = width - columns * cardWidth; + + return Math.max( + MIN_GRID_SPACING, + Math.floor(leftover / (columns + 1)), + ); +}