refactor(web): extract FormCard — shared add/create form shell (heading + error banner + busy submit) #287

Merged

dominik.polakovics merged 1 commit from afk/281 into main

2026-08-04 20:54:12 +02:00

dominik.polakovics commented

2026-08-04 20:42:51 +02:00

Owner

Splits the add/create form card shell out of the seven sites that hand-rolled it.

Each site duplicated a .card.form-card container, an <h2>, an ErrorBanner fed by a local error signal, a <form>, and a primary submit button disabled while busy whose label swaps ("Creating…" / "Create token") — re-deriving the busy wiring and error plumbing every time, and drifting in the details.

New FormCard (with a colocated test) owns the shell; fields stay with the caller as children. Migrated: create-token, create-credential, AddRepo, NewIssue, and the add-forms in repo settings → Secrets, Imports, Schedules.

Decisions worth reviewing

The <form> is the card, rather than a <div class="card form-card"> wrapping one. Six sites nested a bare <form> inside the card div while NewIssue already put the classes on the form element, and one shape had to win. Form-as-card is the CSS-safe direction, verified against web/src/styles/: .form-card only sets margin-bottom plus .form-card > p { margin-top: 0 }; no rule selects a bare form (every form selector is classed or scoped); nothing selects .card > h2 or any other direct-child structure; and a <form> carries no UA-default margin or padding — so hoisting the heading and banner inside it moves nothing on screen. It also keeps NewIssue's existing form.form-card assertions meaningful, and both still pass unmodified, which is the evidence the DOM was preserved.

Drift became props, not copies: wide (five sites want primary wide; Tokens and Schedules want plain primary), disabled for a second condition OR'd with busy (Imports stays off until a repo is picked), and actions for extra buttons — which re-creates the .card-actions row only Schedules needs, while the other six keep their bare trailing button. Schedules also exercises the reactive path: its title and idle submit label are live prop expressions, its busy label unconditionally "Saving…".

intro is a separate slot from children because the two kinds of prose here sit on opposite sides of the banner. AddRepo's "lab clones a bare mirror it owns…" is standing explanation — equally true before and during an error — so it renders above the banner; Schedules' success note is a result of the last submit and stays in children, below it. Routing AddRepo's paragraph through children would have pushed it under the banner and silently reordered that card the moment an error showed, invisible until then. That the slot belongs to the shell rather than one site's quirk is already written into the CSS: .form-card > p { margin-top: 0 } exists to style exactly that paragraph.

FormCard deliberately does not call preventDefault() — every call site's handler already does, and NewIssue validates before it.

Reviewing the diff

The Schedules diff is large only because every field dedents two spaces. Under git diff -w it is the shell swap plus two <small class="hint"> blocks Prettier re-wrapped at the new indent, text byte-identical. Same for AddRepo.

Verification

Run on the integrated branch:

npx vitest run — 71 files, 1015 tests passed (13 of them the new FormCard.test.tsx)
npm run build (tsc --noEmit && vite build) — clean
npx eslint . — clean
npx prettier --check . — clean

No test at any of the seven sites needed modifying. The component test asserts the full ordered child shape with every slot populated at once; mutating intro to below the banner fails exactly that one test.

No visual change.

Closes #281

Splits the add/create form card shell out of the seven sites that hand-rolled it. Each site duplicated a `.card.form-card` container, an `<h2>`, an `ErrorBanner` fed by a local error signal, a `<form>`, and a primary submit button disabled while busy whose label swaps ("Creating…" / "Create token") — re-deriving the busy wiring and error plumbing every time, and drifting in the details. New `FormCard` (with a colocated test) owns the shell; fields stay with the caller as `children`. Migrated: create-token, create-credential, AddRepo, NewIssue, and the add-forms in repo settings → Secrets, Imports, Schedules. ## Decisions worth reviewing **The `<form>` is the card**, rather than a `<div class="card form-card">` wrapping one. Six sites nested a bare `<form>` inside the card div while NewIssue already put the classes on the form element, and one shape had to win. Form-as-card is the CSS-safe direction, verified against `web/src/styles/`: `.form-card` only sets `margin-bottom` plus `.form-card > p { margin-top: 0 }`; no rule selects a bare `form` (every form selector is classed or scoped); nothing selects `.card > h2` or any other direct-child structure; and a `<form>` carries no UA-default margin or padding — so hoisting the heading and banner inside it moves nothing on screen. It also keeps NewIssue's existing `form.form-card` assertions meaningful, and **both still pass unmodified**, which is the evidence the DOM was preserved. **Drift became props, not copies:** `wide` (five sites want `primary wide`; Tokens and Schedules want plain `primary`), `disabled` for a second condition OR'd with busy (Imports stays off until a repo is picked), and `actions` for extra buttons — which re-creates the `.card-actions` row only Schedules needs, while the other six keep their bare trailing button. Schedules also exercises the reactive path: its title and idle submit label are live prop expressions, its busy label unconditionally "Saving…". **`intro` is a separate slot from `children`** because the two kinds of prose here sit on opposite sides of the banner. AddRepo's "lab clones a bare mirror it owns…" is standing explanation — equally true before and during an error — so it renders above the banner; Schedules' success note is a result of the last submit and stays in `children`, below it. Routing AddRepo's paragraph through `children` would have pushed it under the banner and silently reordered that card the moment an error showed, invisible until then. That the slot belongs to the shell rather than one site's quirk is already written into the CSS: `.form-card > p { margin-top: 0 }` exists to style exactly that paragraph. `FormCard` deliberately does not call `preventDefault()` — every call site's handler already does, and NewIssue validates before it. ## Reviewing the diff The Schedules diff is large only because every field dedents two spaces. Under `git diff -w` it is the shell swap plus two `<small class="hint">` blocks Prettier re-wrapped at the new indent, text byte-identical. Same for AddRepo. ## Verification Run on the integrated branch: - `npx vitest run` — 71 files, **1015 tests passed** (13 of them the new `FormCard.test.tsx`) - `npm run build` (`tsc --noEmit && vite build`) — clean - `npx eslint .` — clean - `npx prettier --check .` — clean No test at any of the seven sites needed modifying. The component test asserts the full ordered child shape with every slot populated at once; mutating `intro` to below the banner fails exactly that one test. No visual change. Closes #281

dominik.polakovics added 1 commit

2026-08-04 20:42:51 +02:00

refactor(web): extract FormCard, migrate all 7 add/create forms

ci / native (pull_request) Successful in 10m39s

Details

b9f079ac7f

Seven add/create forms hand-rolled the same shell — a `.card.form-card`
container, an `<h2>`, an `ErrorBanner` fed by a local error signal, a
`<form>`, and a primary submit button disabled while busy whose label swaps
("Creating…" / "Create token"). Each re-derived the busy wiring and the error
plumbing, and they had drifted apart in the details.

New `FormCard` owns the whole shell so no call site can hand-roll it again.
Fields stay with the caller as `children`. Migrated: the create-token and
create-credential cards, AddRepo, NewIssue, and the add-forms in repo settings
→ Secrets, Imports and Schedules.

The `<form>` IS the card, rather than a `<div class="card form-card">` wrapping
one. Six sites nested a bare `<form>` inside the card div while NewIssue
already put the classes on the form element, and one shape had to win.
Form-as-card is the CSS-safe direction: `.form-card` only sets `margin-bottom`
plus `.form-card > p { margin-top: 0 }`, no rule selects a bare `form` (every
form selector is classed or scoped), nothing selects `.card > h2` or any other
direct-child structure, and a `<form>` has no UA-default margin or padding — so
hoisting the heading and the banner inside it moves nothing on screen. It also
keeps NewIssue's existing `form.form-card` assertions meaningful; both still
pass unmodified, which is the evidence the DOM was preserved.

The drift became props rather than copies: `wide` (five sites want
`primary wide`, Tokens and Schedules want plain `primary`), `disabled` for a
second condition OR'd with busy (Imports stays off until a repo is picked), and
`actions` for extra buttons, which re-creates the `.card-actions` row that only
Schedules needs — the other six keep their bare trailing button. Schedules also
exercises the reactive path: its title and idle submit label are both live
prop expressions, and its busy label is unconditionally "Saving…".

`intro` is a separate slot from `children` because the two kinds of prose in
this card sit on opposite sides of the banner. AddRepo's "lab clones a bare
mirror it owns…" is standing explanation, equally true before and during an
error, so it renders above the banner; Schedules' success note is a result of
the last submit and stays in `children`, below it. Routing AddRepo's paragraph
through `children` would have pushed it under the banner and silently reordered
that card the moment an error showed — invisible until then. That the slot
belongs to the shell and not to one site's quirk is already written into the
CSS: `.form-card > p { margin-top: 0 }` exists to style exactly that paragraph.

FormCard deliberately does not call `preventDefault()`: every call site's
handler already does, and NewIssue validates before it.

The Schedules diff is large only because every field dedents two spaces; under
`git diff -w` it is the shell swap plus two `<small class="hint">` blocks that
Prettier re-wrapped at the new indent, text byte-identical.

No visual change. No test needed modifying at any of the seven sites.

Closes #281

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

dominik.polakovics commented

2026-08-04 20:54:07 +02:00

Author

Owner

[autoland] verdict: pass

dominik.polakovics merged commit d4ea730736 into main

2026-08-04 20:54:12 +02:00

dominik.polakovics referenced this pull request from a commit

2026-08-04 20:54:13 +02:00

Merge pull request 'refactor(web): extract FormCard — shared add/create form shell (heading + error banner + busy submit)' (#287) from afk/281 into main