From 5ada5274e09d120ba299021bb11e20ab70c71563 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 28 Jul 2026 16:17:11 +0200 Subject: [PATCH 1/6] fix(#298): align catalogue card rows without wasting space on subtitle-less rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The book title had a fixed min-height and the subtitle was rendered only when present, so cards with a subtitle pushed author/publisher/"Details" down out of line with their neighbours, while cards with a short title left a gap. #298 proposed removing the mobile min-height, but that only makes the title inherit the 2.8em base (taller, not shorter) and doesn't address alignment. Instead: keep the subtitle conditional (no reserved space on the majority of rows that have none) and equalise per grid row with a small layout script — it sets every title in a row to the row's tallest title, pads the shorter real subtitles, and injects a hidden .book-subtitle placeholder on the cards of a subtitle row that have none. Result: rows with no subtitle stay compact; rows that contain one line up title → subtitle → author → publisher → Details across all their cards. Pure layout (heights only) — colours stay on the theme variables, so it's identical across themes. tests/catalog-subtitle-align-298.spec.js seeds a catalogue with plain and subtitled books and asserts, per row: subtitle rows keep authors aligned, a placeholder is reserved on their plain cards, and subtitle-less rows get none. --- app/Views/frontend/catalog-grid.php | 91 +++++++++++++++++++++- tests/catalog-subtitle-align-298.spec.js | 98 ++++++++++++++++++++++++ 2 files changed, 185 insertions(+), 4 deletions(-) create mode 100644 tests/catalog-subtitle-align-298.spec.js diff --git a/app/Views/frontend/catalog-grid.php b/app/Views/frontend/catalog-grid.php index c5c9a3d67..5ae49c1a3 100644 --- a/app/Views/frontend/catalog-grid.php +++ b/app/Views/frontend/catalog-grid.php @@ -162,7 +162,13 @@ line-height: 1.4; margin-bottom: 0.5rem; color: var(--text-primary); - min-height: 2.8em; + /* #298: cap at two lines; the exact height is equalised per grid row by the + script below, so a row of short titles stays compact while a row that + needs two lines lines up. */ + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; + overflow: hidden; } .book-title a { @@ -182,6 +188,9 @@ -webkit-line-clamp: 2; -webkit-box-orient: vertical; overflow: hidden; + /* #298: the subtitle is rendered only when the book has one, so it takes + space conditionally — no reserved gap on the (majority) of cards without + a subtitle. */ } .book-title a:hover { @@ -192,7 +201,13 @@ font-size: 0.875rem; color: var(--text-secondary); margin-bottom: 0.5rem; - min-height: 1.2em; + /* #298: fixed single-line box so a long author name can't push the row + below out of alignment with adjacent cards. */ + height: 1.3em; + display: -webkit-box; + -webkit-line-clamp: 1; + -webkit-box-orient: vertical; + overflow: hidden; } .book-meta { @@ -200,7 +215,12 @@ color: var(--text-muted); line-height: 1.5; margin-bottom: auto; - min-height: 1.5em; + /* #298: fixed single-line box (see .book-author). */ + height: 1.5em; + display: -webkit-box; + -webkit-line-clamp: 1; + -webkit-box-orient: vertical; + overflow: hidden; } .book-actions { @@ -275,7 +295,6 @@ .book-title { font-size: 1rem; - min-height: 2.4em; } } @@ -302,3 +321,67 @@ font-size: 0.75rem; } + diff --git a/tests/catalog-subtitle-align-298.spec.js b/tests/catalog-subtitle-align-298.spec.js new file mode 100644 index 000000000..37f585393 --- /dev/null +++ b/tests/catalog-subtitle-align-298.spec.js @@ -0,0 +1,98 @@ +// @ts-check +// Issue #298 — catalogue grid alignment. A subtitle is optional and used to be +// rendered only when present, so a card WITH a subtitle pushed its author / +// publisher / "Details" down relative to neighbouring cards WITHOUT one. The fix +// keeps the subtitle conditional (no wasted space on rows where nobody has one) +// but a per-row script reserves the subtitle's height on the other cards of any +// row that DOES contain one — so everything lines up row by row. +// +// This asserts, on a real catalogue: (a) rows that contain a subtitle keep their +// authors aligned across cards, (b) a spacer is actually injected there, and +// (c) rows with no subtitle get no spacer (stay compact). Pure layout behaviour. +// +// Run: /tmp/run-e2e.sh tests/catalog-subtitle-align-298.spec.js --config=tests/playwright.config.js --workers=1 +const { test, expect } = require('@playwright/test'); +const { execFileSync } = require('child_process'); + +const BASE = process.env.E2E_BASE_URL || 'http://localhost:8081'; +const DB_USER = process.env.E2E_DB_USER || ''; +const DB_PASS = process.env.E2E_DB_PASS || ''; +const DB_SOCKET = process.env.E2E_DB_SOCKET || ''; +const DB_NAME = process.env.E2E_DB_NAME || ''; + +test.skip(!DB_USER || !DB_NAME, 'DB credentials not configured (this spec seeds the catalogue directly)'); + +function db(sql) { + const args = []; + if (DB_SOCKET) args.push('-S', DB_SOCKET); + args.push('-u', DB_USER, DB_NAME, '-N', '-B', '-e', sql); + return execFileSync('mysql', args, { encoding: 'utf-8', timeout: 10000, env: { ...process.env, MYSQL_PWD: DB_PASS } }).trim(); +} +function sqlq(s) { return "'" + String(s).replace(/\\/g, '\\\\').replace(/'/g, "\\'") + "'"; } + +const TAG = 'ZZ298ALIGN'; +// A handful of books so a 2/3-column grid puts a subtitled card next to a +// plain one: several plain titles + two with a subtitle. +const SEED = [ + { t: `${TAG} Alpha`, s: null }, + { t: `${TAG} Bravo`, s: null }, + { t: `${TAG} Charlie has a rather long subtitle`, s: 'An explanatory subtitle that runs across the card width' }, + { t: `${TAG} Delta`, s: null }, + { t: `${TAG} Echo`, s: null }, + { t: `${TAG} Foxtrot`, s: 'Another subtitle, second one' }, +]; + +test.beforeAll(() => { + db(`DELETE FROM libri WHERE titolo LIKE ${sqlq(TAG + '%')}`); + for (const b of SEED) { + const sub = b.s === null ? 'NULL' : sqlq(b.s); + db(`INSERT INTO libri (titolo, sottotitolo, stato) VALUES (${sqlq(b.t)}, ${sub}, 'disponibile')`); + } +}); +test.afterAll(() => { + db(`DELETE FROM libri WHERE titolo LIKE ${sqlq(TAG + '%')}`); +}); + +test('#298: subtitle space is reserved per row so cards stay aligned', async ({ page }) => { + await page.setViewportSize({ width: 1200, height: 1400 }); + await page.goto(`${BASE}/catalogo`); + // The alignment script runs on load and settles; wait for it rather than networkidle. + await page.waitForFunction(() => document.querySelectorAll('.books-grid .book-card').length > 0, { timeout: 15000 }); + await page.waitForTimeout(600); + + const r = await page.evaluate(() => { + const cards = Array.from(document.querySelectorAll('.books-grid .book-card')); + // group by visual row + const rows = []; + cards.forEach((c) => { + const top = Math.round(c.getBoundingClientRect().top); + let row = rows.find((x) => Math.abs(x.top - top) < 8); + if (!row) { row = { top, cards: [] }; rows.push(row); } + row.cards.push(c); + }); + let rowsWithSubtitle = 0, spacers = 0, misalignedRows = 0, badSpacerRows = 0; + rows.forEach((row) => { + const hasSub = row.cards.some((c) => c.querySelector('.book-subtitle:not(.subtitle-ph)')); + if (!hasSub) { + // compact row: no placeholder must have been injected + if (row.cards.some((c) => c.querySelector('.subtitle-ph'))) badSpacerRows++; + return; + } + rowsWithSubtitle++; + spacers += row.cards.filter((c) => c.querySelector('.subtitle-ph')).length; + // authors must line up across the row (allow a couple px for sub-pixel) + const tops = row.cards + .map((c) => c.querySelector('.book-author')) + .filter(Boolean) + .map((a) => Math.round(a.getBoundingClientRect().top)); + if (tops.length > 1 && Math.max(...tops) - Math.min(...tops) > 3) misalignedRows++; + }); + return { total: cards.length, rowsWithSubtitle, spacers, misalignedRows, badSpacerRows }; + }); + + expect(r.total, 'catalogue rendered cards').toBeGreaterThan(0); + expect(r.rowsWithSubtitle, 'at least one row contains a subtitle (seeded)').toBeGreaterThan(0); + expect(r.misalignedRows, 'every row with a subtitle keeps its authors aligned').toBe(0); + expect(r.badSpacerRows, 'rows without a subtitle get no spacer (stay compact)').toBe(0); + expect(r.spacers, 'a spacer was reserved on the plain cards of subtitle rows').toBeGreaterThan(0); +}); From 07adfff4714e3e579f73ba2897d38449785523dd Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 28 Jul 2026 17:09:04 +0200 Subject: [PATCH 2/6] fix(#298): scope per-row alignment to each .books-grid CodeRabbit: align() grouped every .book-card on the page by vertical position, so if a page renders two .books-grid containers, cards from different grids at the same top could be merged into one logical row and get wrong placeholders / heights. Iterate over each .books-grid and group only its own cards. --- app/Views/frontend/catalog-grid.php | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/Views/frontend/catalog-grid.php b/app/Views/frontend/catalog-grid.php index 5ae49c1a3..d7904ea99 100644 --- a/app/Views/frontend/catalog-grid.php +++ b/app/Views/frontend/catalog-grid.php @@ -345,8 +345,8 @@ function equalise(els, prop) { els.forEach(function (e) { e.style[prop] = max + 'px'; }); return max; } - function align() { - var cards = Array.prototype.slice.call(document.querySelectorAll('.books-grid .book-card')); + function alignGrid(grid) { + var cards = Array.prototype.slice.call(grid.querySelectorAll('.book-card')); if (!cards.length) return; // undo previous adjustments so a resize recomputes from scratch cards.forEach(function (c) { @@ -377,6 +377,11 @@ function align() { }); }); } + function align() { + // Scope per grid: two .books-grid on the same page must not have their rows + // merged just because cards happen to share a vertical position. + Array.prototype.slice.call(document.querySelectorAll('.books-grid')).forEach(alignGrid); + } var timer; function schedule() { clearTimeout(timer); timer = setTimeout(align, 120); } if (document.readyState === 'loading') document.addEventListener('DOMContentLoaded', align); From 9b7e97bc688258d5901f7e08d2b8d64d2a597e99 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 28 Jul 2026 17:21:31 +0200 Subject: [PATCH 3/6] fix(#298): explicit line-height on .book-author matching its fixed height CodeRabbit: .book-author has height:1.3em + overflow:hidden but no explicit line-height, so the default (~1.2) could clip the single line top/bottom. Set line-height:1.3 to match the box height (.book-meta already pairs 1.5/1.5em). --- app/Views/frontend/catalog-grid.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/Views/frontend/catalog-grid.php b/app/Views/frontend/catalog-grid.php index d7904ea99..9a6f88da2 100644 --- a/app/Views/frontend/catalog-grid.php +++ b/app/Views/frontend/catalog-grid.php @@ -202,8 +202,10 @@ color: var(--text-secondary); margin-bottom: 0.5rem; /* #298: fixed single-line box so a long author name can't push the row - below out of alignment with adjacent cards. */ + below out of alignment with adjacent cards. line-height matches the + height so the single line isn't clipped by overflow:hidden. */ height: 1.3em; + line-height: 1.3; display: -webkit-box; -webkit-line-clamp: 1; -webkit-box-orient: vertical; From 3e5a771b9ef346a5a2e2b9db92325f7aa5a9ece8 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 28 Jul 2026 17:50:42 +0200 Subject: [PATCH 4/6] =?UTF-8?q?test(#298):=20regression=20=E2=80=94=20two?= =?UTF-8?q?=20.books-grid=20on=20one=20page=20align=20independently?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit: the spec only exercised a single grid, so a return to global row grouping wouldn't be caught. Add a case that builds two side-by-side grids sharing the same row top, and asserts a subtitle in grid A reserves space only within grid A — grid B (no subtitle) is untouched. --- tests/catalog-subtitle-align-298.spec.js | 48 ++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/tests/catalog-subtitle-align-298.spec.js b/tests/catalog-subtitle-align-298.spec.js index 37f585393..ae37f212b 100644 --- a/tests/catalog-subtitle-align-298.spec.js +++ b/tests/catalog-subtitle-align-298.spec.js @@ -96,3 +96,51 @@ test('#298: subtitle space is reserved per row so cards stay aligned', async ({ expect(r.badSpacerRows, 'rows without a subtitle get no spacer (stay compact)').toBe(0); expect(r.spacers, 'a spacer was reserved on the plain cards of subtitle rows').toBeGreaterThan(0); }); + +// Regression for the CodeRabbit finding: alignment must be scoped per .books-grid. +// Two side-by-side grids share the same vertical position on their first row; a +// subtitle in grid A must NOT inject a placeholder into grid B (which would happen +// if align() grouped all .book-card elements on the page globally). +test('#298: two grids on one page are aligned independently', async ({ page }) => { + await page.setViewportSize({ width: 1200, height: 1000 }); + await page.goto(`${BASE}/catalogo`); + // wait until the page's own script (with align()/resize listener) is live + await page.waitForFunction(() => document.querySelectorAll('.books-grid').length > 0, { timeout: 15000 }); + + const res = await page.evaluate(async () => { + function card(title, subtitle) { + var c = document.createElement('div'); c.className = 'book-card'; + var content = document.createElement('div'); content.className = 'book-content'; + var t = document.createElement('h3'); t.className = 'book-title'; t.textContent = title; content.appendChild(t); + if (subtitle) { var s = document.createElement('p'); s.className = 'book-subtitle'; s.textContent = subtitle; content.appendChild(s); } + var a = document.createElement('p'); a.className = 'book-author'; a.textContent = 'Author name'; content.appendChild(a); + c.appendChild(content); return c; + } + var wrap = document.createElement('div'); + wrap.id = 'twogrid-regression'; wrap.style.display = 'flex'; wrap.style.gap = '20px'; + // force two columns so A1|A2 (and B1|B2) sit on the same row + var gA = document.createElement('div'); gA.className = 'books-grid'; + gA.style.width = '340px'; gA.style.gridTemplateColumns = '1fr 1fr'; + gA.appendChild(card('A1 title', 'A subtitle that reserves a line')); gA.appendChild(card('A2 title', null)); + var gB = document.createElement('div'); gB.className = 'books-grid'; + gB.style.width = '340px'; gB.style.gridTemplateColumns = '1fr 1fr'; + gB.appendChild(card('B1 title', null)); gB.appendChild(card('B2 title', null)); + wrap.appendChild(gA); wrap.appendChild(gB); + document.body.appendChild(wrap); + + window.dispatchEvent(new Event('resize')); // align() is debounced ~120ms + await new Promise(function (r) { setTimeout(r, 350); }); + + var out = { + gridA_placeholders: gA.querySelectorAll('.subtitle-ph').length, + gridB_placeholders: gB.querySelectorAll('.subtitle-ph').length, + }; + wrap.remove(); + return out; + }); + + // grid A's plain card gets a placeholder (its row has a subtitle)… + expect(res.gridA_placeholders, 'grid A plain card reserves the subtitle space').toBe(1); + // …but grid B, which has no subtitle, must be untouched despite sharing the row's top. + expect(res.gridB_placeholders, 'grid B is not affected by grid A (per-grid scope)').toBe(0); +}); From a7810c0d966d989ba080e9c46c662161e21b0318 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 28 Jul 2026 21:01:13 +0200 Subject: [PATCH 5/6] fix(#302): realign catalogue cards after AJAX refresh --- app/Views/frontend/catalog-grid.php | 4 ++ app/Views/frontend/catalog.php | 1 + tests/catalog-subtitle-align-298.spec.js | 47 ++++++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/app/Views/frontend/catalog-grid.php b/app/Views/frontend/catalog-grid.php index 9a6f88da2..15c583133 100644 --- a/app/Views/frontend/catalog-grid.php +++ b/app/Views/frontend/catalog-grid.php @@ -390,5 +390,9 @@ function schedule() { clearTimeout(timer); timer = setTimeout(align, 120); } else align(); window.addEventListener('load', align); window.addEventListener('resize', schedule); + // The catalogue replaces this partial through AJAX after filters, sorting and + // pagination. Scripts inserted through innerHTML do not execute, so the page + // explicitly emits this event once the replacement cards are in the DOM. + document.addEventListener('pinakes:catalog-grid-updated', align); })(); diff --git a/app/Views/frontend/catalog.php b/app/Views/frontend/catalog.php index a463b8dc0..f635d6e38 100644 --- a/app/Views/frontend/catalog.php +++ b/app/Views/frontend/catalog.php @@ -1919,6 +1919,7 @@ function loadBooks() { } else { container.style.display = 'grid'; container.innerHTML = data.html; + container.dispatchEvent(new Event('pinakes:catalog-grid-updated', { bubbles: true })); container.classList.add('fade-in'); } diff --git a/tests/catalog-subtitle-align-298.spec.js b/tests/catalog-subtitle-align-298.spec.js index ae37f212b..8f33378d3 100644 --- a/tests/catalog-subtitle-align-298.spec.js +++ b/tests/catalog-subtitle-align-298.spec.js @@ -97,6 +97,53 @@ test('#298: subtitle space is reserved per row so cards stay aligned', async ({ expect(r.spacers, 'a spacer was reserved on the plain cards of subtitle rows').toBeGreaterThan(0); }); +test('#298: AJAX catalogue refresh realigns the replacement cards', async ({ page }) => { + await page.setViewportSize({ width: 1200, height: 1400 }); + await page.goto(`${BASE}/catalogo`); + await page.waitForFunction(() => document.querySelectorAll('#books-grid .book-card').length > 0, { timeout: 15000 }); + + // Remove the initial alignment so a placeholder can only reappear if the + // post-AJAX replacement explicitly triggers a fresh alignment pass. + await page.evaluate(() => { + document.querySelectorAll('#books-grid .subtitle-ph').forEach((el) => el.remove()); + document.querySelectorAll('#books-grid .book-title, #books-grid .book-subtitle').forEach((el) => { el.style.height = ''; }); + }); + + const response = page.waitForResponse((res) => + res.request().resourceType() === 'fetch' && res.url().includes('catalog') && res.ok() + ); + await page.evaluate(() => updateFilter('sort', 'title_desc')); + await response; + await page.waitForFunction(() => document.querySelectorAll('#books-grid .subtitle-ph').length > 0, { timeout: 15000 }); + + const result = await page.evaluate(() => { + const cards = Array.from(document.querySelectorAll('#books-grid .book-card')); + const rows = []; + cards.forEach((card) => { + const top = Math.round(card.getBoundingClientRect().top); + let row = rows.find((candidate) => Math.abs(candidate.top - top) < 8); + if (!row) { row = { top, cards: [] }; rows.push(row); } + row.cards.push(card); + }); + + let subtitleRows = 0; + let misalignedRows = 0; + rows.forEach((row) => { + if (!row.cards.some((card) => card.querySelector('.book-subtitle:not(.subtitle-ph)'))) return; + subtitleRows++; + const authorTops = row.cards + .map((card) => card.querySelector('.book-author')) + .filter(Boolean) + .map((author) => Math.round(author.getBoundingClientRect().top)); + if (authorTops.length > 1 && Math.max(...authorTops) - Math.min(...authorTops) > 3) misalignedRows++; + }); + return { subtitleRows, misalignedRows }; + }); + + expect(result.subtitleRows, 'AJAX result contains a row with a subtitle').toBeGreaterThan(0); + expect(result.misalignedRows, 'replacement cards are realigned after AJAX').toBe(0); +}); + // Regression for the CodeRabbit finding: alignment must be scoped per .books-grid. // Two side-by-side grids share the same vertical position on their first row; a // subtitle in grid A must NOT inject a placeholder into grid B (which would happen From 338387ba8e718184aab1777b691d010c50cc26ba Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Wed, 29 Jul 2026 10:52:36 +0200 Subject: [PATCH 6/6] fix(#302 review): robust card alignment (fonts, thrashing, edge cases) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five review findings on the alignment script/test: 1. The script measured heights on DOMContentLoaded/load only, never on document.fonts.ready — a late web-font swap could reflow a title from one to two lines after the pass, clipping it (line-clamp + fixed height) and leaving the row misaligned until a resize. Now re-aligns on fonts.ready. 2. Layout thrashing: alignGrid() interleaved reads (getBoundingClientRect / offsetHeight) and writes (height) row by row, forcing repeated synchronous reflows, and ran twice on initial load. Split into a single READ phase then a single WRITE phase, and coalesced all triggers (load, resize, AJAX, fonts) through one requestAnimationFrame. 3. equalise wrote height:0px when every card measured 0 (grid inside a display:none ancestor), collapsing titles. Now skips a row whose titles measure 0 (not laid out). 4. Added a mobile single-column test (375px): asserts one column, zero spurious placeholders, and no zero-height titles — a path the suite never exercised. 5. Test 1 now waits for document.fonts.ready before measuring (was a fixed 600ms that could sample a pre-font-swap layout) and pins its assertions to the seeded fixture so ambient data can't silently substitute unrelated books. Verified: all 4 E2E tests green (alignment unchanged for the asserted cases). --- app/Views/frontend/catalog-grid.php | 50 ++++++++++++++++------- tests/catalog-subtitle-align-298.spec.js | 51 +++++++++++++++++++++++- 2 files changed, 86 insertions(+), 15 deletions(-) diff --git a/app/Views/frontend/catalog-grid.php b/app/Views/frontend/catalog-grid.php index 15c583133..e8a760bfd 100644 --- a/app/Views/frontend/catalog-grid.php +++ b/app/Views/frontend/catalog-grid.php @@ -341,30 +341,39 @@ function rowsOf(cards) { }); return rows; } - function equalise(els, prop) { + function maxHeight(els) { var max = 0; els.forEach(function (e) { if (e.offsetHeight > max) max = e.offsetHeight; }); - els.forEach(function (e) { e.style[prop] = max + 'px'; }); return max; } function alignGrid(grid) { var cards = Array.prototype.slice.call(grid.querySelectorAll('.book-card')); if (!cards.length) return; - // undo previous adjustments so a resize recomputes from scratch + // 1) RESET (writes only): undo previous adjustments so the READ phase below + // measures natural heights. cards.forEach(function (c) { var t = c.querySelector('.book-title'); if (t) t.style.height = ''; var s = c.querySelector('.book-subtitle:not(.subtitle-ph)'); if (s) s.style.height = ''; var ph = c.querySelector('.subtitle-ph'); if (ph) ph.parentNode.removeChild(ph); }); - rowsOf(cards).forEach(function (row) { - // titles: same height as the tallest title in the row + // 2) READ (measurements only): batch every getBoundingClientRect/offsetHeight + // read here so the WRITE phase can't interleave reads and force repeated + // synchronous reflows (#302 review, layout-thrashing fix). + var plan = rowsOf(cards).map(function (row) { var titles = row.cards.map(function (c) { return c.querySelector('.book-title'); }).filter(Boolean); - if (titles.length) equalise(titles, 'height'); - // subtitles: only if the row has at least one var realSubs = row.cards.map(function (c) { return c.querySelector('.book-subtitle'); }).filter(Boolean); - if (!realSubs.length) return; - var maxS = equalise(realSubs, 'height'); // pad the shorter real subtitles too - row.cards.forEach(function (c) { + return { row: row, titles: titles, realSubs: realSubs, maxT: maxHeight(titles), maxS: maxHeight(realSubs) }; + }); + // 3) WRITE (mutations only): apply heights and inject placeholders. No reads + // here, so the browser reflows at most once after this batch. + plan.forEach(function (p) { + // maxT === 0 means the grid is not laid out (e.g. an ancestor is + // display:none) — skip rather than collapse every title to 0px. + if (!p.maxT) return; + p.titles.forEach(function (e) { e.style.height = p.maxT + 'px'; }); + if (!p.realSubs.length) return; // row has no subtitle → stays compact + p.realSubs.forEach(function (e) { e.style.height = p.maxS + 'px'; }); + p.row.cards.forEach(function (c) { if (c.querySelector('.book-subtitle')) return; // already has one var t = c.querySelector('.book-title'); if (!t) return; // a hidden .book-subtitle placeholder inherits the same margins, so the @@ -373,16 +382,23 @@ function alignGrid(grid) { ph.className = 'book-subtitle subtitle-ph'; ph.setAttribute('aria-hidden', 'true'); ph.style.visibility = 'hidden'; - ph.style.height = maxS + 'px'; + ph.style.height = p.maxS + 'px'; ph.innerHTML = ' '; t.insertAdjacentElement('afterend', ph); }); }); } + var frame = null; function align() { - // Scope per grid: two .books-grid on the same page must not have their rows - // merged just because cards happen to share a vertical position. - Array.prototype.slice.call(document.querySelectorAll('.books-grid')).forEach(alignGrid); + // Coalesce bursts of triggers (DOMContentLoaded+load, rapid resize/AJAX) + // into a single alignment per animation frame (#302 review, thrashing fix). + if (frame) return; + frame = (window.requestAnimationFrame || window.setTimeout)(function () { + frame = null; + // Scope per grid: two .books-grid on the same page must not have their rows + // merged just because cards happen to share a vertical position. + Array.prototype.slice.call(document.querySelectorAll('.books-grid')).forEach(alignGrid); + }, 16); } var timer; function schedule() { clearTimeout(timer); timer = setTimeout(align, 120); } @@ -390,6 +406,12 @@ function schedule() { clearTimeout(timer); timer = setTimeout(align, 120); } else align(); window.addEventListener('load', align); window.addEventListener('resize', schedule); + // Re-align once web fonts settle: a late font swap can reflow a title from one + // line to two after the initial pass, leaving the row misaligned until a resize + // (#302 review). Harmless where fonts are already loaded. + if (document.fonts && document.fonts.ready && typeof document.fonts.ready.then === 'function') { + document.fonts.ready.then(align); + } // The catalogue replaces this partial through AJAX after filters, sorting and // pagination. Scripts inserted through innerHTML do not execute, so the page // explicitly emits this event once the replacement cards are in the DOM. diff --git a/tests/catalog-subtitle-align-298.spec.js b/tests/catalog-subtitle-align-298.spec.js index 8f33378d3..47876be85 100644 --- a/tests/catalog-subtitle-align-298.spec.js +++ b/tests/catalog-subtitle-align-298.spec.js @@ -30,6 +30,21 @@ function db(sql) { } function sqlq(s) { return "'" + String(s).replace(/\\/g, '\\\\').replace(/'/g, "\\'") + "'"; } +// Wait until the alignment pass has run AFTER web fonts settle: resolve +// document.fonts.ready, then two animation frames guarantee the coalesced +// align() frame has painted. Avoids measuring a pre-font-swap layout, which +// would be flaky or hide a real reflow-after-swap bug (#302 review). +async function waitForAligned(page) { + await page.evaluate(() => new Promise((resolve) => { + const settle = () => requestAnimationFrame(() => requestAnimationFrame(resolve)); + if (document.fonts && document.fonts.ready && typeof document.fonts.ready.then === 'function') { + document.fonts.ready.then(settle); + } else { + settle(); + } + })); +} + const TAG = 'ZZ298ALIGN'; // A handful of books so a 2/3-column grid puts a subtitled card next to a // plain one: several plain titles + two with a subtitle. @@ -58,7 +73,15 @@ test('#298: subtitle space is reserved per row so cards stay aligned', async ({ await page.goto(`${BASE}/catalogo`); // The alignment script runs on load and settles; wait for it rather than networkidle. await page.waitForFunction(() => document.querySelectorAll('.books-grid .book-card').length > 0, { timeout: 15000 }); - await page.waitForTimeout(600); + await waitForAligned(page); + + // Pin the assertion to the seeded fixture: if ambient data pushed the seeds off + // page 1 (LIMIT 12, created_at DESC), fail clearly rather than silently + // measuring unrelated books (#302 review). + const seedsVisible = await page.evaluate((tag) => + Array.from(document.querySelectorAll('.books-grid .book-title')) + .filter((t) => (t.textContent || '').includes(tag)).length, TAG); + expect(seedsVisible, 'seeded fixture cards are on the first page').toBeGreaterThan(0); const r = await page.evaluate(() => { const cards = Array.from(document.querySelectorAll('.books-grid .book-card')); @@ -97,6 +120,32 @@ test('#298: subtitle space is reserved per row so cards stay aligned', async ({ expect(r.spacers, 'a spacer was reserved on the plain cards of subtitle rows').toBeGreaterThan(0); }); +test('#298: single-column (mobile) layout injects no spurious placeholder', async ({ page }) => { + // At a narrow viewport the auto-fill grid collapses to one column, so each + // visual row holds exactly one card. A subtitled card alone must keep its own + // subtitle (no placeholder), a plain card alone must get nothing, and no title + // may collapse to zero height (#302 review — this path was previously untested). + await page.setViewportSize({ width: 375, height: 1600 }); + await page.goto(`${BASE}/catalogo`); + await page.waitForFunction(() => document.querySelectorAll('.books-grid .book-card').length > 0, { timeout: 15000 }); + await waitForAligned(page); + + const r = await page.evaluate(() => { + const grid = document.querySelector('.books-grid'); + const cards = Array.from(grid.querySelectorAll('.book-card')); + const cols = new Set(cards.map((c) => Math.round(c.getBoundingClientRect().left))).size; + const placeholders = grid.querySelectorAll('.subtitle-ph').length; + const zeroHeightTitles = cards.map((c) => c.querySelector('.book-title')) + .filter(Boolean).filter((t) => t.offsetHeight === 0).length; + return { cards: cards.length, cols, placeholders, zeroHeightTitles }; + }); + + expect(r.cards, 'catalogue rendered cards').toBeGreaterThan(0); + expect(r.cols, 'layout collapses to a single column at 375px').toBe(1); + expect(r.placeholders, 'no spurious subtitle placeholder in single-column layout').toBe(0); + expect(r.zeroHeightTitles, 'no title collapsed to zero height').toBe(0); +}); + test('#298: AJAX catalogue refresh realigns the replacement cards', async ({ page }) => { await page.setViewportSize({ width: 1200, height: 1400 }); await page.goto(`${BASE}/catalogo`);