From 5ada5274e09d120ba299021bb11e20ab70c71563 Mon Sep 17 00:00:00 2001 From: fabiodalez-dev Date: Tue, 28 Jul 2026 16:17:11 +0200 Subject: [PATCH 1/5] 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/5] 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/5] 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/5] =?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/5] 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