fix(search): the mobile hero search was indented by a card's padding #130

Merged
tudor merged 2 commits from fix/mobile-hero-search into main 2026-08-26 20:51:21 +00:00
Owner

Measured on staging at a 390px viewport:

Element Left Width
headline / lede 34 323
search box, hint, location link 48 295

Everything in the search block sat 14px right of the copy above it, and the field was 28px narrower for no reason.

Where 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 immediately after .filterBar. A bare .filterBar rule inside a media query comes later and silently wins instead.

The two rules directly below it in the same block were already written as .filterBar:not(.heroMode) — so the scoping was deliberate and this one was simply missed.

What changes

  • The search box, hint and location link align to the same left edge as the headline (34px).
  • The field gains back its 28px of width.
  • The location link 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 intact.

The guard

A stylesheet test, because the failure is a plausible-looking layout rather than a broken one — nothing short of measuring or looking would catch it. It asserts no bare .filterBar rule inside a media query sets a property .heroMode resets (background, border, border-radius, box-shadow, padding), and it explains the source-order trap in the file.

Verified by reverting — it reports .filterBar sets padding, then passes again once restored.

Frontend 286 passed, tsc clean, next build green.

Not done

The hint copy still wraps to two lines at 390px, and the input keeps its current height. Both are judgement calls rather than defects, so I left them — say the word if you want the copy trimmed or the field taller.

Measured on staging at a 390px viewport: | Element | Left | Width | |---|---|---| | headline / lede | 34 | 323 | | search box, hint, location link | **48** | **295** | Everything in the search block sat 14px right of the copy above it, and the field was 28px narrower for no reason. ## Where the 14px came from ```css @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 immediately after `.filterBar`. A bare `.filterBar` rule inside a media query comes later and silently wins instead. The two rules directly below it in the same block were already written as `.filterBar:not(.heroMode)` — so the scoping was deliberate and this one was simply missed. ## What changes - The search box, hint and location link align to the same left edge as the headline (34px). - The field gains back its 28px of width. - The location link 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 intact. ## The guard A stylesheet test, because the failure is a *plausible-looking* layout rather than a broken one — nothing short of measuring or looking would catch it. It asserts no bare `.filterBar` rule inside a media query sets a property `.heroMode` resets (background, border, border-radius, box-shadow, padding), and it explains the source-order trap in the file. **Verified by reverting** — it reports `.filterBar sets padding`, then passes again once restored. Frontend 286 passed, `tsc` clean, `next build` green. ## Not done The hint copy still wraps to two lines at 390px, and the input keeps its current height. Both are judgement calls rather than defects, so I left them — say the word if you want the copy trimmed or the field taller.
tudor added 1 commit 2026-08-26 20:33:11 +00:00
fix(search): the mobile hero search was indented by a card's padding
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 1m3s
0b15497c09
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj

🤖 AI Code Review (Claude Code)

This PR fixes a mobile CSS regression where an unscoped .filterBar rule inside a @media (max-width: 768px) block was re-adding card padding to the hero search (which .heroMode otherwise zeroes out), by scoping it to .filterBar:not(.heroMode) to match sibling rules, plus a small optical-alignment tweak for .nearMeBtn. The CSS fix itself looks correct and consistent with the existing pattern; the accompanying tests are weaker than they appear and don't fully guard against regression.

🟡 Minor

  • e2e/tests/__m.spec.ts: The test performs no assertions at all — it only console.logs computed geometry. It cannot fail even if the hero padding regression it's named after reoccurs, so it provides no actual regression coverage despite looking like a dedicated test for this bug.
  • e2e/tests/__m.spec.ts: Leftover placeholder code: filterBar: pick('form').constructor === Object ? null : null always evaluates to null regardless of the ternary, and if document.querySelector('form') doesn't match anything, pick('form') returns null and .constructor throws a TypeError inside page.evaluate, causing the whole test to fail for an unrelated reason. This looks like debug/scratch code (filename __m.spec.ts and the // placeholder comment suggest the same) that shouldn't have been committed as-is.
  • nextjs-app/tests/components/filterBarScoping.test.ts: The regex-based selector check only compares the last line of a (possibly multi-line, comma-separated) selector against the literal string '.filterBar'. A future regression written as a comma list, e.g. .filterBar,\n.otherThing { padding: ... } or .filterBar, .other { padding: ... }, would not be caught because the extracted 'selector' wouldn't exactly equal '.filterBar', defeating the purpose of this guard test.
## 🤖 AI Code Review (Claude Code) This PR fixes a mobile CSS regression where an unscoped `.filterBar` rule inside a `@media (max-width: 768px)` block was re-adding card padding to the hero search (which `.heroMode` otherwise zeroes out), by scoping it to `.filterBar:not(.heroMode)` to match sibling rules, plus a small optical-alignment tweak for `.nearMeBtn`. The CSS fix itself looks correct and consistent with the existing pattern; the accompanying tests are weaker than they appear and don't fully guard against regression. ### 🟡 Minor - **e2e/tests/__m.spec.ts**: The test performs no assertions at all — it only console.logs computed geometry. It cannot fail even if the hero padding regression it's named after reoccurs, so it provides no actual regression coverage despite looking like a dedicated test for this bug. - **e2e/tests/__m.spec.ts**: Leftover placeholder code: `filterBar: pick('form').constructor === Object ? null : null` always evaluates to null regardless of the ternary, and if `document.querySelector('form')` doesn't match anything, `pick('form')` returns null and `.constructor` throws a TypeError inside `page.evaluate`, causing the whole test to fail for an unrelated reason. This looks like debug/scratch code (filename `__m.spec.ts` and the `// placeholder` comment suggest the same) that shouldn't have been committed as-is. - **nextjs-app/__tests__/components/filterBarScoping.test.ts**: The regex-based selector check only compares the last line of a (possibly multi-line, comma-separated) selector against the literal string '.filterBar'. A future regression written as a comma list, e.g. `.filterBar,\n.otherThing { padding: ... }` or `.filterBar, .other { padding: ... }`, would not be caught because the extracted 'selector' wouldn't exactly equal '.filterBar', defeating the purpose of this guard test.
tudor added 1 commit 2026-08-26 20:49:50 +00:00
fix(test): drop a committed scratch probe, and close a hole in the guard
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 11s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 9s
0804566736
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
tudor merged commit a7f4c86464 into main 2026-08-26 20:51:21 +00:00
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: tudor/school_compare#130