From 0b15497c09ea53c25b8d7e9aed304b7c18417f5b Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 26 Aug 2026 21:32:41 +0100 Subject: [PATCH 1/2] fix(search): the mobile hero search was indented by a card's padding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured at 390px: the headline and lede sit at x=34, while the search box, the hint and the location link all sat at x=48 and the field was 28px narrower than the copy above it. The 14px came from `@media (max-width: 768px) { .filterBar { padding: 0.875rem } }`. That rule is for the results filter bar, which is a card — background, border, shadow — and needs inner padding. The hero search is not a card: .heroMode strips all of it, padding included. Both selectors are specificity (0,1,0), so source order decides, and .heroMode only wins because it is declared right after .filterBar. A bare .filterBar rule inside a media query comes later and silently wins instead. The two rules directly below this one in the same block were already written as `.filterBar:not(.heroMode)`; this one was missed. Scoping it aligns the search box, hint and location link to the same left edge as the headline and gives the field back its 28px. The location link also carried its own 6px of button padding, so its label started further right than the hint even once the boxes agreed. Pulled back with a negative margin, which keeps the tap target. The guard is a stylesheet test: the failure is a plausible-looking layout rather than a broken one, so nothing short of measuring or looking would catch it. Verified by reverting: it names ".filterBar sets padding". Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- e2e/tests/__m.spec.ts | 36 ++++++++ .../components/filterBarScoping.test.ts | 82 +++++++++++++++++++ nextjs-app/components/FilterBar.module.css | 20 ++++- 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 e2e/tests/__m.spec.ts create mode 100644 nextjs-app/__tests__/components/filterBarScoping.test.ts diff --git a/e2e/tests/__m.spec.ts b/e2e/tests/__m.spec.ts new file mode 100644 index 0000000..83240c0 --- /dev/null +++ b/e2e/tests/__m.spec.ts @@ -0,0 +1,36 @@ +import { test } from '@playwright/test'; +test('mobile hero geometry', async ({ page }) => { + await page.setViewportSize({ width: 390, height: 844 }); + await page.goto('/'); + const out = await page.evaluate(() => { + const pick = (sel: string) => { + const el = document.querySelector(sel) as HTMLElement | null; + if (!el) return null; + const r = el.getBoundingClientRect(); + const cs = getComputedStyle(el); + return { left: Math.round(r.left), right: Math.round(r.right), + width: Math.round(r.width), height: Math.round(r.height), + pad: cs.padding, cls: el.className.toString().slice(0, 40) }; + }; + const input = document.querySelector('input[type="search"]') as HTMLElement; + const box = input?.closest('div'); + const form = input?.closest('form'); + return { + h1: pick('h1'), + lede: pick('h1 + p') ?? pick('p'), + omniBox: box ? { left: Math.round(box.getBoundingClientRect().left), + width: Math.round(box.getBoundingClientRect().width), + pad: getComputedStyle(box).padding, + dir: getComputedStyle(box).flexDirection, + cls: box.className.toString().slice(0,40) } : null, + input: pick('input[type="search"]'), + form: form ? { left: Math.round(form.getBoundingClientRect().left), + width: Math.round(form.getBoundingClientRect().width), + pad: getComputedStyle(form).padding, + cls: form.className.toString().slice(0,40) } : null, + filterBar: pick('form'). // placeholder + constructor === Object ? null : null, + }; + }); + console.log(JSON.stringify(out, null, 2)); +}); diff --git a/nextjs-app/__tests__/components/filterBarScoping.test.ts b/nextjs-app/__tests__/components/filterBarScoping.test.ts new file mode 100644 index 0000000..38ddf3b --- /dev/null +++ b/nextjs-app/__tests__/components/filterBarScoping.test.ts @@ -0,0 +1,82 @@ +import fs from 'fs'; +import path from 'path'; + +/** + * The hero search and the results filter bar are the same component in two + * costumes. `.filterBar` is the card — background, border, shadow, padding — + * and `.heroMode` strips all of it so the search sits directly on the hero + * panel. + * + * Both selectors have specificity (0,1,0), so **source order decides**, and + * `.heroMode` only wins because it is declared immediately after. Any later + * bare `.filterBar` rule — which in practice means one inside a media query — + * silently wins instead, and the hero grows a card's padding back. + * + * That is exactly what happened: `@media (max-width: 768px) { .filterBar { + * padding: 0.875rem } }` re-added 14px in hero mode, indenting the search box, + * the hint and the location link 14px past the headline above them and costing + * the search field 28px of width on a 390px screen. The two rules directly + * below it in the same block were correctly written as + * `.filterBar:not(.heroMode)`; this one was missed, and nothing caught it + * because the result is a plausible-looking layout rather than a broken one. + */ + +const CSS = path.join(__dirname, '..', '..', 'components', 'FilterBar.module.css'); + +/** Properties `.heroMode` resets. A later bare `.filterBar` rule setting any + * of these puts the card back on the hero. */ +const RESET_BY_HERO_MODE = [ + 'background', 'border', 'border-radius', 'box-shadow', 'padding', +]; + +function mediaQueryBodies(css: string): string[] { + const bodies: string[] = []; + const re = /@media[^{]*\{/g; + let m: RegExpExecArray | null; + while ((m = re.exec(css)) !== null) { + // Walk braces from the opening one to find this at-rule's whole body. + let depth = 1; + let i = m.index + m[0].length; + const start = i; + while (i < css.length && depth > 0) { + if (css[i] === '{') depth++; + else if (css[i] === '}') depth--; + i++; + } + bodies.push(css.slice(start, i - 1)); + } + return bodies; +} + +describe('FilterBar hero-mode scoping', () => { + const css = fs.readFileSync(CSS, 'utf8'); + + it('confirms heroMode still resets the card, which is what makes this matter', () => { + const hero = css.match(/\.heroMode\s*\{([^}]*)\}/); + expect(hero).not.toBeNull(); + expect(hero![1]).toMatch(/padding:\s*0/); + }); + + it('never re-applies card styling to the hero from inside a media query', () => { + const offenders: string[] = []; + + for (const body of mediaQueryBodies(css)) { + for (const rule of body.matchAll(/([^{}]+)\{([^{}]*)\}/g)) { + const selector = rule[1].trim().split('\n').pop()!.trim(); + // Only a *bare* .filterBar is dangerous. Scoped variants + // (`.filterBar:not(.heroMode)`) and descendants are fine. + if (selector !== '.filterBar') continue; + + for (const prop of RESET_BY_HERO_MODE) { + if (new RegExp(`(^|[;\\s])${prop}\\s*:`).test(rule[2])) { + offenders.push(`${selector} sets ${prop}`); + } + } + } + } + + // Fix by scoping the rule as `.filterBar:not(.heroMode)`, the way the + // neighbouring rules in the same block already are. + expect(offenders).toEqual([]); + }); +}); diff --git a/nextjs-app/components/FilterBar.module.css b/nextjs-app/components/FilterBar.module.css index 7384f60..287c6d3 100644 --- a/nextjs-app/components/FilterBar.module.css +++ b/nextjs-app/components/FilterBar.module.css @@ -413,7 +413,17 @@ /* ── Narrow ───────────────────────────────────────────────────────── */ @media (max-width: 768px) { - .filterBar { + /* + * Scoped, like the two rules below it. + * + * The results filter bar is a card — background, border, shadow — and needs + * inner padding. The hero's search is not a card: .heroMode zeroes the + * padding, border and background so the search sits directly on the panel. + * Unscoped, this rule put 14px back, which indented the search box, the hint + * and the location link 14px past the headline they sit under, and cost the + * search field 28px of width on a 390px screen. + */ + .filterBar:not(.heroMode) { padding: 0.875rem; } @@ -457,6 +467,14 @@ align-items: flex-start; } + /* Optical alignment: the button's own 6px of padding is what makes its + label start further right than the hint above it, even once both boxes + share a left edge. Pulling the padding back off lines the text up while + keeping the tap target. */ + .heroMode .nearMeBtn { + margin-left: -0.375rem; + } + .geoError { text-align: left; } From 080456673632810806c9bfcbae58ff50f3307fab Mon Sep 17 00:00:00 2001 From: Tudor Date: Wed, 26 Aug 2026 21:49:48 +0100 Subject: [PATCH 2/2] fix(test): drop a committed scratch probe, and close a hole in the guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code review, all three findings valid. e2e/tests/__m.spec.ts was a throwaway probe used to measure the mobile hero geometry. It asserts nothing, so it could never fail; it carried a leftover `pick('form').constructor === Object ? null : null` that is null either way and throws if no form matches; and it should never have been committed. Deleted. It survived because `rm -f e2e/tests/__m.spec.ts` ran with the shell already inside e2e/, so the path resolved to e2e/e2e/tests/... — which does not exist, and rm -f is silent about that. `git add -A` then swept it in. I checked `git diff --stat` before committing, which lists only tracked modifications and never shows an untracked file; `git status --short` would have. The scoping guard compared the last line of a rule's prelude against the literal '.filterBar', so a regression written as a selector list — `.filterBar, .other { padding }`, or the same split across two lines — would have walked straight past the test meant to catch it. Selectors are now split on commas and matched individually, and comments are stripped first so a brace inside one cannot desynchronise the parse. Verified against all three shapes: bare, inline comma list, and multi-line comma list. Each is caught; each passes again once reverted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj --- e2e/tests/__m.spec.ts | 36 ------------------- .../components/filterBarScoping.test.ts | 36 ++++++++++++++++--- 2 files changed, 31 insertions(+), 41 deletions(-) delete mode 100644 e2e/tests/__m.spec.ts diff --git a/e2e/tests/__m.spec.ts b/e2e/tests/__m.spec.ts deleted file mode 100644 index 83240c0..0000000 --- a/e2e/tests/__m.spec.ts +++ /dev/null @@ -1,36 +0,0 @@ -import { test } from '@playwright/test'; -test('mobile hero geometry', async ({ page }) => { - await page.setViewportSize({ width: 390, height: 844 }); - await page.goto('/'); - const out = await page.evaluate(() => { - const pick = (sel: string) => { - const el = document.querySelector(sel) as HTMLElement | null; - if (!el) return null; - const r = el.getBoundingClientRect(); - const cs = getComputedStyle(el); - return { left: Math.round(r.left), right: Math.round(r.right), - width: Math.round(r.width), height: Math.round(r.height), - pad: cs.padding, cls: el.className.toString().slice(0, 40) }; - }; - const input = document.querySelector('input[type="search"]') as HTMLElement; - const box = input?.closest('div'); - const form = input?.closest('form'); - return { - h1: pick('h1'), - lede: pick('h1 + p') ?? pick('p'), - omniBox: box ? { left: Math.round(box.getBoundingClientRect().left), - width: Math.round(box.getBoundingClientRect().width), - pad: getComputedStyle(box).padding, - dir: getComputedStyle(box).flexDirection, - cls: box.className.toString().slice(0,40) } : null, - input: pick('input[type="search"]'), - form: form ? { left: Math.round(form.getBoundingClientRect().left), - width: Math.round(form.getBoundingClientRect().width), - pad: getComputedStyle(form).padding, - cls: form.className.toString().slice(0,40) } : null, - filterBar: pick('form'). // placeholder - constructor === Object ? null : null, - }; - }); - console.log(JSON.stringify(out, null, 2)); -}); diff --git a/nextjs-app/__tests__/components/filterBarScoping.test.ts b/nextjs-app/__tests__/components/filterBarScoping.test.ts index 38ddf3b..5fcaebf 100644 --- a/nextjs-app/__tests__/components/filterBarScoping.test.ts +++ b/nextjs-app/__tests__/components/filterBarScoping.test.ts @@ -29,6 +29,31 @@ const RESET_BY_HERO_MODE = [ 'background', 'border', 'border-radius', 'box-shadow', 'padding', ]; +/** + * Comments are stripped before anything is parsed. + * + * A `{` or `}` inside a comment would otherwise desynchronise the brace walk + * below and the rule regex alike, and the selector text captured for each rule + * would carry the preceding comment along with it. + */ +function withoutComments(css: string): string { + return css.replace(/\/\*[\s\S]*?\*\//g, ''); +} + +/** + * The individual selectors in a rule's prelude. + * + * Split on commas, because a selector list is a list: `.filterBar, .other { }` + * applies to `.filterBar` just as surely as `.filterBar { }` does, and an + * earlier version of this guard compared the whole prelude against the literal + * string '.filterBar' — so writing the regression as a comma list, or across + * two lines, would have walked straight past it. + */ +function selectorsOf(prelude: string): string[] { + return prelude.split(',').map((sel) => sel.trim().replace(/\s+/g, ' ')) + .filter(Boolean); +} + function mediaQueryBodies(css: string): string[] { const bodies: string[] = []; const re = /@media[^{]*\{/g; @@ -49,7 +74,7 @@ function mediaQueryBodies(css: string): string[] { } describe('FilterBar hero-mode scoping', () => { - const css = fs.readFileSync(CSS, 'utf8'); + const css = withoutComments(fs.readFileSync(CSS, 'utf8')); it('confirms heroMode still resets the card, which is what makes this matter', () => { const hero = css.match(/\.heroMode\s*\{([^}]*)\}/); @@ -62,14 +87,15 @@ describe('FilterBar hero-mode scoping', () => { for (const body of mediaQueryBodies(css)) { for (const rule of body.matchAll(/([^{}]+)\{([^{}]*)\}/g)) { - const selector = rule[1].trim().split('\n').pop()!.trim(); - // Only a *bare* .filterBar is dangerous. Scoped variants + // Only a *bare* .filterBar is dangerous, and it is dangerous wherever + // it appears in a selector list. Scoped variants // (`.filterBar:not(.heroMode)`) and descendants are fine. - if (selector !== '.filterBar') continue; + const selectors = selectorsOf(rule[1]); + if (!selectors.includes('.filterBar')) continue; for (const prop of RESET_BY_HERO_MODE) { if (new RegExp(`(^|[;\\s])${prop}\\s*:`).test(rule[2])) { - offenders.push(`${selector} sets ${prop}`); + offenders.push(`${rule[1].trim()} sets ${prop}`); } } }