refactor(web): extract ListRowCard — one component for the entity list-row card #288

Merged

dominik.polakovics merged 3 commits from afk/282 into main 2026-08-05 00:08:25 +02:00

Conversation 1 Commits 3 Files changed 7 +366 -65

dominik.polakovics commented 2026-08-04 21:09:52 +02:00

Owner

Copy link

Split out of umbrella #276, follow-up to #275 (SectionCard) and #279 (EmptyState).

What

The entity list-row card — <article class="card"> wrapping a .card-head of a .card-title span, whatever chips belong beside it, a .spacer, and the row's action buttons, over an optional <p class="muted card-sub"> metadata line — was hand-rolled at five production sites. web/src/components/ListRowCard.tsx now owns that markup, with a colocated test in SectionCard's style.

<ListRowCard title={…} badges={…} actions={…} sub={…} class="…">{body}</ListRowCard>

Migrated (the row markup for these entities is now produced only by the component):

Site	Component
routes/Tokens.tsx	TokenCard
routes/repo-settings/sections/Imports.tsx	ImportRow
routes/Credentials.tsx	CredentialCard
routes/settings/sections/Notifications.tsx	the device row
routes/repo-settings/sections/Schedules.tsx	ScheduleRow

Design notes

• Two head slots, not one. The halves of the head row live on opposite sides of the spacer. badges renders before it, so Credentials' kind and "in use" chips keep hugging the title; actions renders after it, so buttons stay at the trailing edge. A single combined slot would have to pick a side, and picking "after" would slide Credentials' chips to the right edge — a visual change.
• title is a JSX slot, because Notifications decorates its device label with a Show-gated <span class="muted"> · this device</span> inside the .card-title span.
• sub is gated on !== undefined, not truthiness. Every caller that passes one rendered the line unconditionally before; Schedules' cadenceSummary falls back to expr.trim(), so a truthiness gate would silently drop that row's metadata line for a blank cadence.
• The confirm-then-delete wiring stays with each caller. The issue left this open as an implementation decision; the busy models genuinely differ — Tokens and Imports hold a boolean signal, Schedules a 'delete' | 'reenable' | null so it can say which of its two buttons is working, Credentials a shared run() helper that also drives an inline ErrorBanner and a view/rename/replace mode. Folding them together would either change behaviour or need one knob per call site. The shared thing here is the markup, not the wiring.

No visual change

Same elements, class strings, text, confirm prompts, busy labels and error propagation at all five sites. The one node added anywhere is a trailing <span class="spacer"> in Credentials' head, which is inert there: .spacer is flex: 1 — flex: 1 1 0% — so it claims only space that was already empty in that left-packed row and cannot force a wrap at flex-basis: 0. .card-title / .card-sub / .card-head in styles/cards.css are untouched, and per-entity modifiers still attach (class="token-card" on Tokens).

Rows deliberately left hand-rolled

Every remaining <div class="card-head"> in production code, with why it is not this shape:

• components/ProviderAuthCard.tsx — a <section class="card auth-status-card">, not an <article> list row; its head is a Switch over login state plus a non-destructive Refresh, and its body is a multi-branch Switch of forms and confirms. Provider-auth family, not an entity row.
• routes/Repos.tsx — the head's trailing items are three <A class="card-link"> navigations (Issues / CRs / Settings) alongside a conditional "Stop all". That is the card-link row family under #276, not title + action.
• routes/History.tsx — the head ends in an outcome chip and has no action button; the body is a multi-block run summary (chip row, failure note, escalation block, trailing card-link).
• routes/RepoIssues.tsx — the card is the <A> (built with classList, not a literal class), so there is no <article> to hand over.
• routes/RepoCRs.tsx — the .card-head is nested inside an <A> rather than being a direct child of the card.
• routes/repo-settings/sections/Secrets.tsx — the closest miss. It stacks two card-sub lines (a Show-gated description above a fixed "Updated …"), which the single metadata slot cannot order without reintroducing them as children, and its title needs card-title mono — a title-class knob no other row wants. Worth revisiting if a second row ever wants either.

Verification

web/ has no node_modules and this worktree has no Node toolchain, so vitest / tsc / eslint / prettier could not be run locally — the CI native job is the gate. Review was done by reading: every migrated row was checked element-by-element against its pre-migration JSX for order, class strings and text, and the existing suites for these routes (Tokens.test.tsx, Imports.test.tsx, Notifications.test.tsx, Schedules.test.tsx, settings/index.test.tsx, repo-settings/index.test.tsx, e2e/smoke.spec.ts) assert on rendered text and button labels rather than on .card/.card-head structure, with the one exception of Imports.test.tsx's .card-title probe, which ListRowCard renders unconditionally.

Closes #282

Split out of umbrella #276, follow-up to #275 (SectionCard) and #279 (EmptyState). ## What The entity list-row card — `<article class="card">` wrapping a `.card-head` of a `.card-title` span, whatever chips belong beside it, a `.spacer`, and the row's action buttons, over an optional `<p class="muted card-sub">` metadata line — was hand-rolled at five production sites. `web/src/components/ListRowCard.tsx` now owns that markup, with a colocated test in `SectionCard`'s style. ```tsx <ListRowCard title={…} badges={…} actions={…} sub={…} class="…">{body}</ListRowCard> ``` Migrated (the row markup for these entities is now produced only by the component): | Site | Component | | --- | --- | | `routes/Tokens.tsx` | `TokenCard` | | `routes/repo-settings/sections/Imports.tsx` | `ImportRow` | | `routes/Credentials.tsx` | `CredentialCard` | | `routes/settings/sections/Notifications.tsx` | the device row | | `routes/repo-settings/sections/Schedules.tsx` | `ScheduleRow` | ## Design notes - **Two head slots, not one.** The halves of the head row live on opposite sides of the spacer. `badges` renders *before* it, so Credentials' kind and "in use" chips keep hugging the title; `actions` renders *after* it, so buttons stay at the trailing edge. A single combined slot would have to pick a side, and picking "after" would slide Credentials' chips to the right edge — a visual change. - **`title` is a JSX slot**, because Notifications decorates its device label with a `Show`-gated `<span class="muted"> · this device</span>` *inside* the `.card-title` span. - **`sub` is gated on `!== undefined`, not truthiness.** Every caller that passes one rendered the line unconditionally before; Schedules' `cadenceSummary` falls back to `expr.trim()`, so a truthiness gate would silently drop that row's metadata line for a blank cadence. - **The confirm-then-delete wiring stays with each caller.** The issue left this open as an implementation decision; the busy models genuinely differ — Tokens and Imports hold a boolean signal, Schedules a `'delete' | 'reenable' | null` so it can say *which* of its two buttons is working, Credentials a shared `run()` helper that also drives an inline `ErrorBanner` and a view/rename/replace mode. Folding them together would either change behaviour or need one knob per call site. The shared thing here is the markup, not the wiring. ## No visual change Same elements, class strings, text, confirm prompts, busy labels and error propagation at all five sites. The one node added anywhere is a trailing `<span class="spacer">` in Credentials' head, which is inert there: `.spacer` is `flex: 1` — `flex: 1 1 0%` — so it claims only space that was already empty in that left-packed row and cannot force a wrap at `flex-basis: 0`. `.card-title` / `.card-sub` / `.card-head` in `styles/cards.css` are untouched, and per-entity modifiers still attach (`class="token-card"` on Tokens). ## Rows deliberately left hand-rolled Every remaining `<div class="card-head">` in production code, with why it is not this shape: - **`components/ProviderAuthCard.tsx`** — a `<section class="card auth-status-card">`, not an `<article>` list row; its head is a `Switch` over login state plus a non-destructive Refresh, and its body is a multi-branch `Switch` of forms and confirms. Provider-auth family, not an entity row. - **`routes/Repos.tsx`** — the head's trailing items are three `<A class="card-link">` navigations (Issues / CRs / Settings) alongside a conditional "Stop all". That is the card-link row family under #276, not title + action. - **`routes/History.tsx`** — the head ends in an outcome chip and has no action button; the body is a multi-block run summary (chip row, failure note, escalation block, trailing `card-link`). - **`routes/RepoIssues.tsx`** — the card *is* the `<A>` (built with `classList`, not a literal `class`), so there is no `<article>` to hand over. - **`routes/RepoCRs.tsx`** — the `.card-head` is nested inside an `<A>` rather than being a direct child of the card. - **`routes/repo-settings/sections/Secrets.tsx`** — the closest miss. It stacks two `card-sub` lines (a `Show`-gated description above a fixed "Updated …"), which the single metadata slot cannot order without reintroducing them as `children`, and its title needs `card-title mono` — a title-class knob no other row wants. Worth revisiting if a second row ever wants either. ## Verification `web/` has no `node_modules` and this worktree has no Node toolchain, so `vitest` / `tsc` / `eslint` / `prettier` could not be run locally — the CI `native` job is the gate. Review was done by reading: every migrated row was checked element-by-element against its pre-migration JSX for order, class strings and text, and the existing suites for these routes (`Tokens.test.tsx`, `Imports.test.tsx`, `Notifications.test.tsx`, `Schedules.test.tsx`, `settings/index.test.tsx`, `repo-settings/index.test.tsx`, `e2e/smoke.spec.ts`) assert on rendered text and button labels rather than on `.card`/`.card-head` structure, with the one exception of `Imports.test.tsx`'s `.card-title` probe, which `ListRowCard` renders unconditionally. Closes #282

dominik.polakovics added 1 commit 2026-08-04 21:09:52 +02:00

refactor(web): extract ListRowCard, migrate the five entity list rows ... 0cb944bcc7

The entity list-row card — `<article class="card">` wrapping a `.card-head` of
a `.card-title` span, whatever chips belong beside it, a spacer and the row's
action buttons, over an optional `<p class="muted card-sub">` metadata line —
was hand-rolled at five production sites: Tokens, repo settings -> Imports,
Credentials, user settings -> Notifications, and repo settings -> Schedules.

ListRowCard now owns that markup, shaped like SectionCard (#275) and
EmptyState (#279): a small props surface, plain markup out, a colocated test
in the same style. The head is two slots, not one, because its halves live on
opposite sides of the spacer: `badges` renders before it so Credentials' kind
and in-use chips keep hugging the title, `actions` after it so the delete and
edit buttons stay at the trailing edge. `title` is a JSX slot because
Notifications decorates its device label with a `Show`-gated suffix inside the
`.card-title` span, and `sub` is gated on `!== undefined` rather than
truthiness because every caller rendered its metadata line unconditionally —
Schedules' `cadenceSummary` falls back to `expr.trim()`, so a truthiness gate
would silently drop that line for a blank cadence.

The confirm-then-delete flow stays with each caller on purpose. The busy
models genuinely differ — Tokens and Imports hold a boolean signal, Schedules
a `'delete' | 'reenable' | null` so it can say which of its two buttons is
working, Credentials a shared `run()` helper that also drives an inline
ErrorBanner and a view/rename/replace mode — so folding them together would
either change behaviour or need one knob per call site. The shared thing here
is the markup, not the wiring.

No visual change: same elements, classes, text, confirm prompts and busy
labels at all five sites. The one added node anywhere is a trailing
`<span class="spacer">` in Credentials' head, which is inert — `.spacer` is
`flex: 1 1 0%`, so it claims only space that was already empty in that
left-packed row and cannot force a wrap at `flex-basis: 0`.

Left hand-rolled, deliberately: ProviderAuthCard (a `<section>` status card
whose head is a `Switch` over login state), Repos (head is three `card-link`
navigations), History (head ends in an outcome chip, no action), RepoIssues
and RepoCRs (the card is an `<A>`, or the head sits inside one), and Secrets
(two consecutive `card-sub` lines plus a `card-title mono`, which the single
metadata slot cannot order without a knob no other row wants).

Closes #282

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

dominik.polakovics added 1 commit 2026-08-04 21:16:13 +02:00

style(web): prettier-format ListRowCard.test.tsx ... 7067dc03ee

`prettier --check` failed CI on the colocated test: JSX text followed by
an element on the next line is joined onto one line by prettier. Applied
`prettier --write` verbatim; no semantic change (JSX collapses the
newline-bearing whitespace either way, so the rendered text is identical
and the `Pixel 8 · this device` assertion still holds).

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

dominik.polakovics commented 2026-08-04 21:26:48 +02:00

Author

Owner

Copy link

[autoland] verdict: pass

Validated against the validation core.

Signal: ci / native (pull_request) — success, run 392 (10m11s), the required check. It is the run that includes the formatting fix and the first run to get past prettier --check into npm test, tsc --noEmit/vite build, go build -tags ui, go test -tags ui ./... and golangci-lint. (ci-nix is path-gated to nix/dependency changes and correctly did not run — this diff is SPA-only.)

Fixed inline (trivial): run 391 failed solely on prettier --check flagging web/src/components/ListRowCard.test.tsx. Reproduced locally with the pinned toolchain (Node 24.18.0, repo npm ci) and applied prettier --write verbatim in 7067dc0 — prettier joins JSX text onto the line of the element that follows it (Pixel 8<span class="muted"> · this device</span>). No semantic change: JSX discards newline-bearing whitespace either way, so title.textContent === 'Pixel 8 · this device' still holds.

Conventions: Conventional Commits title, Closes #282 present, diff scope confined to the component, its colocated test and the five migrated routes — no drive-by. No conflict with main.

Correctness: all five migrations read element-for-element identical to their pre-migration JSX — order, class strings, text, confirm prompts, busy labels, error propagation. token-card still attaches via class. sub gated on !== undefined is the right call: Schedules' cadenceSummary can return '' and a truthiness gate would drop that row's metadata line. Every acceptance criterion on #282 is met, including the audit of rows left hand-rolled.

[autoland] verdict: pass Validated against the validation core. **Signal:** `ci / native (pull_request)` — success, run 392 (10m11s), the required check. It is the run that includes the formatting fix and the first run to get past `prettier --check` into `npm test`, `tsc --noEmit`/`vite build`, `go build -tags ui`, `go test -tags ui ./...` and golangci-lint. (`ci-nix` is path-gated to nix/dependency changes and correctly did not run — this diff is SPA-only.) **Fixed inline (trivial):** run 391 failed solely on `prettier --check` flagging `web/src/components/ListRowCard.test.tsx`. Reproduced locally with the pinned toolchain (Node 24.18.0, repo `npm ci`) and applied `prettier --write` verbatim in 7067dc0 — prettier joins JSX text onto the line of the element that follows it (`Pixel 8<span class="muted"> · this device</span>`). No semantic change: JSX discards newline-bearing whitespace either way, so `title.textContent === 'Pixel 8 · this device'` still holds. **Conventions:** Conventional Commits title, `Closes #282` present, diff scope confined to the component, its colocated test and the five migrated routes — no drive-by. No conflict with `main`. **Correctness:** all five migrations read element-for-element identical to their pre-migration JSX — order, class strings, text, confirm prompts, busy labels, error propagation. `token-card` still attaches via `class`. `sub` gated on `!== undefined` is the right call: Schedules' `cadenceSummary` can return `''` and a truthiness gate would drop that row's metadata line. Every acceptance criterion on #282 is met, including the audit of rows left hand-rolled.

dominik.polakovics added 1 commit 2026-08-04 22:05:06 +02:00

Merge branch 'main' into afk/282 ... 10de0597d4

Resolves the conflicts #289 (ErrorBanner -> Banner) and #286 (FormCard /
SectionHead) created against the ListRowCard extraction. The two sides are
orthogonal: main renamed the banner component and restructured the form
cards, this branch extracted the list-row card, and both touched the same
import blocks plus Credentials' CredentialCard.

- Import blocks (all five routes): union. `ErrorBanner` is gone on main --
  renamed to `Banner`, same file and same props at every former call site --
  so the branch's `ErrorBanner` import is dropped rather than kept, and
  `ListRowCard` joins main's `Banner` / `FormCard` in the same slot.
- Credentials.tsx CredentialCard: keep this branch's ListRowCard wrapper
  (title / badges / sub) around main's `<Banner>` -- the two sides differed
  only by that rename and by the wrapper.
- ListRowCard.tsx: the design note naming Credentials' "inline ErrorBanner"
  now names Banner, to match the component that exists.

No behaviour change from the resolution: same elements, class strings, text
and props on both sides of every hunk.

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

dominik.polakovics merged commit 210cad4dfe into main 2026-08-05 00:08:25 +02:00

dominik.polakovics deleted branch afk/282 2026-08-05 00:08:25 +02:00

dominik.polakovics referenced this pull request from a commit 2026-08-05 00:08:26 +02:00

Merge pull request 'refactor(web): extract ListRowCard — one component for the entity list-row card' (#288) from afk/282 into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

Something is broken

  

chore

  

enhancement

New feature or improvement

  

needs-info

Waiting on reporter for more information

  

needs-triage

Maintainer needs to evaluate this issue

  

ready-for-agent

Fully specified, ready for an AFK agent

  

ready-for-human

Requires human implementation

  

wontfix

Will not be actioned

No labels

bug

chore

enhancement

needs-info

needs-triage

ready-for-agent

ready-for-human

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

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!288

No description provided.