refactor(web): one shared head row per kind — extract SectionHead, fold the rest into SectionCard #286

Merged
dominik.polakovics merged 4 commits from afk/280 into main 2026-08-04 22:00:16 +02:00

Closes #280

Unifies the two competing head-row patterns onto shared components, so the row's markup and spacing live in one place per kind.

What changed

before after
page-level head <div class="section-head"> hand-rolled in 12 route files <SectionHead title=… action=… />
card-level head SectionCard (#275) + 4 card sections with a bare <h2> SectionCard everywhere
h2 reset 3 identical rules (shell.css, cards.css, settings-ia.css), cross-referenced by a "Mirrors .section-head h2" comment 1 rule in cards.css
  • SectionHead (+ colocated test in SectionCard.test.tsx's style) — heading, always-present .spacer, optional action/controls slot. title is a JSX.Element rather than SectionCard's string, because the detail routes put an inline .issue-number span inside the heading. The #275 spacing contract extends here: the component owns the gap below the row, call sites add nothing.
  • .section-head drops justify-content: space-between — the spacer right-aligns the action now, so the row's markup, and therefore its height, is identical with and without one. The test pins that: ['H2','SPAN'] vs ['H2','SPAN','BUTTON'].
  • Four card sections migrated to SectionCard — IssueDetail's Comments, CRDetail's Diff, RepoIssues' ready queue, RepoLabels' new-label card. .ready-list / .comment-list drop the top margins the component makes redundant.

Out of scope and untouched, as the issue specifies: .card-title list rows (#282) and the .card.form-card shells (#281).

Visual verification

Measured in headless chromium against HEAD's stylesheet and HEAD's markup — before.css / after.css are script-built by concatenating every file in base.css's import order from each revision — at 1280×900 and 390×844, with the real Source Serif 4 face loaded.

Every page head: 0.00px delta on row height, row edges, heading position, action position, and the gap below the row. All eight specimens, both viewports.

Two deltas are real and intended:

  1. History's filters cluster right (first filter +131px on desktop; row height, second filter and gap below unchanged). space-between only ever looked right with two children, and History is the one head with three, so its filters were spread across the row. #280 names History's controls as the action slot's use case, and every other multi-control head in the app already right-clusters via .head-actions.
  2. The four migrated cards take a 12px head gap where it previously fell out of h2's 0.35rem margin collapsing with the body's top margin — +6.41px (ready queue, diff), +2.41px (comments). That is the #275 contract working as designed (the gap is stated, not inherited from font metrics) and it is the value the other 21 section cards already use.

Follow-up filed

#285 — the measurement showed .section-head has no flex-wrap (unlike .card-head), so History's two min-width: 180px selects already overflowed a phone viewport by 28.63px; the spacer's extra 0.65rem gap widens that to 39.02px. Filed rather than fixed here: wrapping visibly changes History's phone layout, which #280 puts out of scope.

Checks

npm run lint, npm run format:check, npm test (70 files / 1001 tests), npm run build (tsc --noEmit + vite) — all green locally.

Closes #280 Unifies the two competing head-row patterns onto shared components, so the row's markup and spacing live in one place per kind. ## What changed | | before | after | | --- | --- | --- | | page-level head | `<div class="section-head">` hand-rolled in 12 route files | `<SectionHead title=… action=… />` | | card-level head | `SectionCard` (#275) + 4 card sections with a bare `<h2>` | `SectionCard` everywhere | | `h2` reset | 3 identical rules (shell.css, cards.css, settings-ia.css), cross-referenced by a "Mirrors `.section-head h2`" comment | 1 rule in cards.css | - **`SectionHead`** (+ colocated test in `SectionCard.test.tsx`'s style) — heading, always-present `.spacer`, optional action/controls slot. `title` is a `JSX.Element` rather than `SectionCard`'s `string`, because the detail routes put an inline `.issue-number` span inside the heading. The #275 spacing contract extends here: the component owns the gap below the row, call sites add nothing. - **`.section-head` drops `justify-content: space-between`** — the spacer right-aligns the action now, so the row's markup, and therefore its height, is identical with and without one. The test pins that: `['H2','SPAN']` vs `['H2','SPAN','BUTTON']`. - **Four card sections migrated to `SectionCard`** — IssueDetail's Comments, CRDetail's Diff, RepoIssues' ready queue, RepoLabels' new-label card. `.ready-list` / `.comment-list` drop the top margins the component makes redundant. Out of scope and untouched, as the issue specifies: `.card-title` list rows (#282) and the `.card.form-card` shells (#281). ## Visual verification Measured in headless chromium against HEAD's stylesheet *and* HEAD's markup — `before.css` / `after.css` are script-built by concatenating every file in `base.css`'s import order from each revision — at 1280×900 and 390×844, with the real Source Serif 4 face loaded. **Every page head: 0.00px delta** on row height, row edges, heading position, action position, and the gap below the row. All eight specimens, both viewports. Two deltas are real and intended: 1. **History's filters cluster right** (first filter +131px on desktop; row height, second filter and gap below unchanged). `space-between` only ever looked right with two children, and History is the one head with three, so its filters were spread across the row. #280 names History's controls as the action slot's use case, and every other multi-control head in the app already right-clusters via `.head-actions`. 2. **The four migrated cards take a 12px head gap** where it previously fell out of `h2`'s 0.35rem margin collapsing with the body's top margin — **+6.41px** (ready queue, diff), **+2.41px** (comments). That is the #275 contract working as designed (the gap is stated, not inherited from font metrics) and it is the value the other 21 section cards already use. ## Follow-up filed **#285** — the measurement showed `.section-head` has no `flex-wrap` (unlike `.card-head`), so History's two `min-width: 180px` selects already overflowed a phone viewport by 28.63px; the spacer's extra 0.65rem gap widens that to 39.02px. Filed rather than fixed here: wrapping visibly changes History's phone layout, which #280 puts out of scope. ## Checks `npm run lint`, `npm run format:check`, `npm test` (70 files / 1001 tests), `npm run build` (`tsc --noEmit` + vite) — all green locally.
refactor(web): one shared head row per kind — extract SectionHead, fold the rest into SectionCard
All checks were successful
ci / native (pull_request) Successful in 8m25s
9c4c98587d
Two competing patterns rendered the same visual row: page-level `.section-head`
(shell.css), hand-rolled in 12 route call sites, and card-level `.card-head`
(cards.css), which #275 already wrapped in `SectionCard`. The duplication was
explicit enough that the CSS commented on itself — `.card-head h2` carried a
"Mirrors `.section-head h2`" note keeping two identical resets in sync by hand.

- New `SectionHead` (title, action) owns the page-level row, extending #275's
  contract: the component owns the gap below the row, call sites add nothing.
  Its `title` is a `JSX.Element` where `SectionCard`'s is a `string`, because
  the detail routes put an inline `.issue-number` span inside the heading.
- All 12 `.section-head` call sites render through it — the list routes, the
  issue/CR detail heads, and both settings shells' index titles.
- `.section-head` drops `justify-content: space-between`: the always-present
  `.spacer` does the right-aligning now, exactly as it does in `SectionCard`,
  so the row's markup — and therefore its height — is identical with and
  without an action.
- The three head-row `h2` resets (`.section-head`, `.card-head`,
  `.settings-back-head`) collapse into one rule in cards.css. The reason is
  unchanged from #275: a row's height must never depend on heading font
  metrics, since Source Serif 4 loads with `font-display: swap`.
- The four remaining card sections that hand-rolled a bare `<h2>` head —
  IssueDetail's Comments, CRDetail's Diff, RepoIssues' ready queue and
  RepoLabels' new-label card — render through `SectionCard`, and
  `.ready-list` / `.comment-list` drop the top margins it makes redundant.

Verified in headless chromium against HEAD's stylesheet and markup, at
1280x900 and 390x844. Every page head measures a 0.00px delta on row height,
row edges, heading position, action position and the gap below the row. Two
deltas are real and intended:

- History's two filter selects now cluster at the right edge instead of being
  spread across the row: `space-between` only ever looked right with two
  children, and History is the one head with three. Its first filter moves
  131px right on desktop; row height, the second filter and the gap below are
  unchanged. Every other multi-control head in the app already right-clusters.
- The four migrated cards take `.section-card-head`'s 12px gap under the
  heading in place of a gap that fell out of `h2`'s own 0.35rem margin
  collapsing with the body's top margin: +6.41px (ready queue, diff), +2.41px
  (comments). That is the point of the #275 contract — the gap is stated, not
  inherited from font metrics — and it is the value the other 21 section cards
  already use.

The same measurement turned up #285: `.section-head` has no `flex-wrap`, so
History's two 180px-min-width selects already overflowed a phone viewport, and
the spacer's extra 0.65rem gap widens that overflow from 28.63px to 39.02px.
Filed rather than fixed here — wrapping visibly changes History's phone layout,
which #280 puts out of scope.

Closes #280

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
Owner

[autoland] verdict: pass

PASS — validated against the validation core.

Signal relied on: ci / native (pull_request) — success in 8m25s. Its "SPA — lint, format, test, build" step (npm run lint, format:check, test, build) runs unconditionally on every PR and covers exactly this web-only diff; the path-gated ci-nix gate is not implicated (no nix/dependency changes). No local re-run needed.

Conventions: Conventional Commits title; Closes #280 present and well-formed on branch afk/280.

Diff scope: matches #280's brief. All five acceptance criteria met — SectionHead exists with a colocated test (5 cases), no hand-rolled .section-head remains in routes/components, four card sections migrated to SectionCard, the always-present .spacer keeps row height independent of the action (pinned by the ['H2','SPAN'] vs ['H2','SPAN','BUTTON'] assertion). #281/#282 families correctly left untouched, and the flex-wrap finding was filed as #285 rather than scope-crept in.

CSS relocation verified: moving the h2 reset into cards.css is safe — .section-head h2 moves later in base.css import order (shell 11 → cards 12) with no competing rule; .settings-back-head h2 moves earlier (settings-ia 34 → cards 12) and nothing between 13 and 34 matches it at equal-or-higher specificity. .spacer is global (layout.css:9). .card.ready-queue still matches SectionCard's class=\"card ready-queue\" output.

Conflict: none. Base advanced to 83ad232 (#284, EmptyState); git merge-tree is clean, and the two sides are disjoint region-by-region across the six shared route files — #284 touched only <p class=\"empty\"> bodies inside <Match>, this PR only head rows. Merged import blocks are correctly ordered.

The two visual deltas the body documents (History's filters right-clustering, +12px head gap on the four migrated cards) are the issue's stated intent, measured and disclosed — not defects.

[autoland] verdict: pass PASS — validated against the validation core. **Signal relied on:** `ci / native (pull_request)` — success in 8m25s. Its "SPA — lint, format, test, build" step (`npm run lint`, `format:check`, `test`, `build`) runs unconditionally on every PR and covers exactly this web-only diff; the path-gated `ci-nix` gate is not implicated (no nix/dependency changes). No local re-run needed. **Conventions:** Conventional Commits title; `Closes #280` present and well-formed on branch `afk/280`. **Diff scope:** matches #280's brief. All five acceptance criteria met — SectionHead exists with a colocated test (5 cases), no hand-rolled `.section-head` remains in routes/components, four card sections migrated to SectionCard, the always-present `.spacer` keeps row height independent of the action (pinned by the `['H2','SPAN']` vs `['H2','SPAN','BUTTON']` assertion). #281/#282 families correctly left untouched, and the flex-wrap finding was filed as #285 rather than scope-crept in. **CSS relocation verified:** moving the `h2` reset into cards.css is safe — `.section-head h2` moves later in `base.css` import order (shell 11 → cards 12) with no competing rule; `.settings-back-head h2` moves earlier (settings-ia 34 → cards 12) and nothing between 13 and 34 matches it at equal-or-higher specificity. `.spacer` is global (layout.css:9). `.card.ready-queue` still matches SectionCard's `class=\"card ready-queue\"` output. **Conflict:** none. Base advanced to 83ad232 (#284, EmptyState); `git merge-tree` is clean, and the two sides are disjoint region-by-region across the six shared route files — #284 touched only `<p class=\"empty\">` bodies inside `<Match>`, this PR only head rows. Merged import blocks are correctly ordered. The two visual deltas the body documents (History's filters right-clustering, +12px head gap on the four migrated cards) are the issue's stated intent, measured and disclosed — not defects.
Merge branch 'main' into afk/280
All checks were successful
ci / native (pull_request) Successful in 8m4s
3f7bb6a96e
Base advanced past this branch's fork point with #284 (EmptyState). The two
sides are disjoint: #284 replaced the routes' `<p class="empty">` bodies
inside `<Match>`, this branch reworked their head rows. No conflicts.
Merge branch 'main' into afk/280
All checks were successful
ci / native (pull_request) Successful in 8m33s
40548f55da
Merge branch 'main' into afk/280
All checks were successful
ci / native (pull_request) Successful in 8m8s
7ac62f3915
Author
Owner

This was generated by AI while landing a PR.

Merge conflict with main resolved — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit 7ac62f3 (Merge branch 'main' into afk/280), pushed to the head branch; no rebase, no force-push.

Three files conflicted: web/src/routes/Repos.tsx, web/src/routes/RepoLabels.tsx, web/src/routes/Tokens.tsx — one hunk each, all the same shape:

  • this PR replaced the hand-rolled <div class="section-head"> with <SectionHead …/>
  • #289 renamed <ErrorBanner …/> to <Banner …/> on the line directly below

The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim:

<SectionHead title="…" />
<Banner message={error()} onDismiss={() => setError(null)} />

Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (Banner in ErrorBanner's slot, SectionHead added). Verified after resolution: zero remaining ErrorBanner references, zero hand-rolled class="section-head" outside SectionHead.tsx itself, and SectionHead imported in all three files.

Re-verified: the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on 7ac62f3ci / native (pull_request) success in 8m8s (run 395).

> *This was generated by AI while landing a PR.* **Merge conflict with `main` resolved** — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit `7ac62f3` (`Merge branch 'main' into afk/280`), pushed to the head branch; no rebase, no force-push. Three files conflicted: `web/src/routes/Repos.tsx`, `web/src/routes/RepoLabels.tsx`, `web/src/routes/Tokens.tsx` — one hunk each, all the same shape: - **this PR** replaced the hand-rolled `<div class="section-head">` with `<SectionHead …/>` - **#289** renamed `<ErrorBanner …/>` to `<Banner …/>` on the line directly below The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim: ```jsx <SectionHead title="…" /> <Banner message={error()} onDismiss={() => setError(null)} /> ``` Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (`Banner` in `ErrorBanner`'s slot, `SectionHead` added). Verified after resolution: zero remaining `ErrorBanner` references, zero hand-rolled `class="section-head"` outside `SectionHead.tsx` itself, and `SectionHead` imported in all three files. **Re-verified:** the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on `7ac62f3` — `ci / native (pull_request)` **success in 8m8s** (run 395).
Author
Owner

This was generated by AI while landing a PR.

Merge conflict with main resolved — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit 7ac62f3 (Merge branch 'main' into afk/280), pushed to the head branch; no rebase, no force-push.

Three files conflicted: web/src/routes/Repos.tsx, web/src/routes/RepoLabels.tsx, web/src/routes/Tokens.tsx — one hunk each, all the same shape:

  • this PR replaced the hand-rolled <div class="section-head"> with <SectionHead …/>
  • #289 renamed <ErrorBanner …/> to <Banner …/> on the line directly below

The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim:

<SectionHead title="…" />
<Banner message={error()} onDismiss={() => setError(null)} />

Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (Banner in ErrorBanner's slot, SectionHead added). Verified after resolution: zero remaining ErrorBanner references, zero hand-rolled class="section-head" outside SectionHead.tsx itself, and SectionHead imported in all three files.

Re-verified: the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on 7ac62f3ci / native (pull_request) success in 8m8s (run 395).

> *This was generated by AI while landing a PR.* **Merge conflict with `main` resolved** — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit `7ac62f3` (`Merge branch 'main' into afk/280`), pushed to the head branch; no rebase, no force-push. Three files conflicted: `web/src/routes/Repos.tsx`, `web/src/routes/RepoLabels.tsx`, `web/src/routes/Tokens.tsx` — one hunk each, all the same shape: - **this PR** replaced the hand-rolled `<div class="section-head">` with `<SectionHead …/>` - **#289** renamed `<ErrorBanner …/>` to `<Banner …/>` on the line directly below The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim: ```jsx <SectionHead title="…" /> <Banner message={error()} onDismiss={() => setError(null)} /> ``` Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (`Banner` in `ErrorBanner`'s slot, `SectionHead` added). Verified after resolution: zero remaining `ErrorBanner` references, zero hand-rolled `class="section-head"` outside `SectionHead.tsx` itself, and `SectionHead` imported in all three files. **Re-verified:** the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on `7ac62f3` — `ci / native (pull_request)` **success in 8m8s** (run 395).
Author
Owner

This was generated by AI while landing a PR.

Merge conflict with main resolved — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit 7ac62f3 (Merge branch 'main' into afk/280), pushed to the head branch; no rebase, no force-push.

Three files conflicted: web/src/routes/Repos.tsx, web/src/routes/RepoLabels.tsx, web/src/routes/Tokens.tsx — one hunk each, all the same shape:

  • this PR replaced the hand-rolled <div class="section-head"> with <SectionHead …/>
  • #289 renamed <ErrorBanner …/> to <Banner …/> on the line directly below

The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim:

<SectionHead title="…" />
<Banner message={error()} onDismiss={() => setError(null)} />

Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (Banner in ErrorBanner's slot, SectionHead added). Verified after resolution: zero remaining ErrorBanner references, zero hand-rolled class="section-head" outside SectionHead.tsx itself, and SectionHead imported in all three files.

Re-verified: the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on 7ac62f3ci / native (pull_request) success in 8m8s (run 395).

> *This was generated by AI while landing a PR.* **Merge conflict with `main` resolved** — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit `7ac62f3` (`Merge branch 'main' into afk/280`), pushed to the head branch; no rebase, no force-push. Three files conflicted: `web/src/routes/Repos.tsx`, `web/src/routes/RepoLabels.tsx`, `web/src/routes/Tokens.tsx` — one hunk each, all the same shape: - **this PR** replaced the hand-rolled `<div class="section-head">` with `<SectionHead …/>` - **#289** renamed `<ErrorBanner …/>` to `<Banner …/>` on the line directly below The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim: ```jsx <SectionHead title="…" /> <Banner message={error()} onDismiss={() => setError(null)} /> ``` Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (`Banner` in `ErrorBanner`'s slot, `SectionHead` added). Verified after resolution: zero remaining `ErrorBanner` references, zero hand-rolled `class="section-head"` outside `SectionHead.tsx` itself, and `SectionHead` imported in all three files. **Re-verified:** the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on `7ac62f3` — `ci / native (pull_request)` **success in 8m8s** (run 395).
Author
Owner

This was generated by AI while landing a PR.

Merge conflict with main resolved — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit 7ac62f3 (Merge branch 'main' into afk/280), pushed to the head branch; no rebase, no force-push.

Three files conflicted: web/src/routes/Repos.tsx, web/src/routes/RepoLabels.tsx, web/src/routes/Tokens.tsx — one hunk each, all the same shape:

  • this PR replaced the hand-rolled <div class="section-head"> with <SectionHead …/>
  • #289 renamed <ErrorBanner …/> to <Banner …/> on the line directly below

The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim:

<SectionHead title="…" />
<Banner message={error()} onDismiss={() => setError(null)} />

Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (Banner in ErrorBanner's slot, SectionHead added). Verified after resolution: zero remaining ErrorBanner references, zero hand-rolled class="section-head" outside SectionHead.tsx itself, and SectionHead imported in all three files.

Re-verified: the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on 7ac62f3ci / native (pull_request) success in 8m8s (run 395).

> *This was generated by AI while landing a PR.* **Merge conflict with `main` resolved** — base had advanced to dc62f8c (#289, the Banner refactor). Merge commit `7ac62f3` (`Merge branch 'main' into afk/280`), pushed to the head branch; no rebase, no force-push. Three files conflicted: `web/src/routes/Repos.tsx`, `web/src/routes/RepoLabels.tsx`, `web/src/routes/Tokens.tsx` — one hunk each, all the same shape: - **this PR** replaced the hand-rolled `<div class="section-head">` with `<SectionHead …/>` - **#289** renamed `<ErrorBanner …/>` to `<Banner …/>` on the line directly below The two edits are orthogonal — different elements, adjacent lines, neither touching the other's semantics — so the resolution is the union of both sides, taken verbatim: ```jsx <SectionHead title="…" /> <Banner message={error()} onDismiss={() => setError(null)} /> ``` Deterministic and behaviour-preserving; no semantic choice was made, nothing was dropped from either side. Import blocks auto-merged correctly (`Banner` in `ErrorBanner`'s slot, `SectionHead` added). Verified after resolution: zero remaining `ErrorBanner` references, zero hand-rolled `class="section-head"` outside `SectionHead.tsx` itself, and `SectionHead` imported in all three files. **Re-verified:** the resolution commit is one no gate had seen, so the earlier green no longer covered it. CI re-ran on `7ac62f3` — `ci / native (pull_request)` **success in 8m8s** (run 395).
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Cloonar/coding-lab!286
No description provided.