New/Import forms: replace the always-on name-charset helper with live inline validation (error only on invalid chars, no grid shift) #486

Closed
opened 2026-09-13 19:28:55 +00:00 by crueber · 3 comments
Owner

What's requested

The Name field on both repo-creation surfaces (/new and /import) carries an always-visible charset hint ("Letters, digits, and . _ - — the URL path after the owner."). Replace that static helper with live inline validation that renders an error only when the entered name actually contains invalid characters, and do it without layout shift: the reserved space for the message must not appear/disappear as the user types (no grid reflow of the fields below).

Evidence (current tree, verified)

  • web/src/pages/New.jsx:215 — <span id="new-name-help" class="muted text-xs">Letters, digits, and . _ - — the URL path after the owner.</span> — always rendered.
  • web/src/pages/Import.jsx:291 — identical always-on span, id import-name-help.
  • The validator already exists and is correct: validateRepoName in web/sdk/src/create.js:19-33 (/^[A-Za-z0-9._-]+$/, 1–100 chars, no leading dot, not .., .git suffix stripped). New.jsx wires it live (fieldError(), line 75) but:
    • its error paragraph (New.jsx:243-245) is conditionally rendered inside the card's grid gap-3 flow — appearing/disappearing shifts the rows below it while typing;
    • it also fires for the required-error case, not just invalid characters;
    • Import.jsx never validates the name client-side at all — only mirror-mode errors surface (Import.jsx:364-368); a one-shot import with an invalid name fails only at submit time via the server 400.
  • The server re-validates on both endpoints, so this is UX-only; no wire or SDK change.

Architecture notes

  • Add a shared headless helper (repo convention: web/src/lib/, unit-testable in Node — see lib/orgs.js validateOrgName precedent) such as validateRepoChars(name) that returns an error only for the invalid-character case, distinct from "required"/empty (empty is already enforced by the disabled submit buttons on both pages).
  • Both forms consume it in an onInput handler; the error region renders reserved-height (e.g. a fixed-height slot, min-h-[1.25rem] wrapper, or aria-live container that keeps its space) so sibling grid rows never shift.
  • On invalid input, both pages should set aria-invalid on the input and keep aria-describedby pointing at the message element.
  • The static help span is removed from both files; charset guidance lives entirely in the error message (shown only on error), mirroring how lib/orgs.js messages state the rule ("organization names are lowercase letters, digits, and hyphens, 1–39 characters").

Acceptance criteria

  • The always-on "Letters, digits, and . _ -" helper span no longer renders on /new or /import.
  • Typing a valid name (foo.bar-2_baz) shows no message anywhere; typing an invalid one (foo bar, a/b, .., .hidden, hi git) shows a red inline message naming the charset rule, live as the user types.
  • An empty name shows NO validation message (the disabled submit button already covers required), on both pages.
  • Grid shift: with the error appearing and disappearing, no sibling row of either card form moves — verified at narrow width (grid-cols-1 stacked) as well as sm:grid-cols-2.
  • Both inputs carry aria-invalid + aria-describedby wired to the live message; the message container is aria-live="polite".
  • Validation logic is one shared pure function in web/src/lib/ (not copy-pasted per page), consumed by both New.jsx and Import.jsx (and by mirror mode on both pages, which shares the same name field).
  • Submit-time behavior unchanged: server 400s still render inline in their existing error blocks; client gate still blocks submit on an invalid name (New.jsx already does via fieldError; Import.jsx gains the gate).
  • No changes to web/sdk/src/create.js semantics or any wire payload.
## What's requested The Name field on both repo-creation surfaces (`/new` and `/import`) carries an **always-visible charset hint** ("Letters, digits, and . _ - — the URL path after the owner."). Replace that static helper with **live inline validation that renders an error only when the entered name actually contains invalid characters**, and do it without layout shift: the reserved space for the message must not appear/disappear as the user types (no grid reflow of the fields below). ## Evidence (current tree, verified) - `web/src/pages/New.jsx:215` — `<span id="new-name-help" class="muted text-xs">Letters, digits, and . _ - — the URL path after the owner.</span>` — always rendered. - `web/src/pages/Import.jsx:291` — identical always-on span, id `import-name-help`. - The validator already exists and is correct: `validateRepoName` in `web/sdk/src/create.js:19-33` (`/^[A-Za-z0-9._-]+$/`, 1–100 chars, no leading dot, not `..`, `.git` suffix stripped). New.jsx wires it live (`fieldError()`, line 75) but: - its error paragraph (New.jsx:243-245) is conditionally rendered inside the card's `grid gap-3` flow — appearing/disappearing shifts the rows below it while typing; - it also fires for the required-error case, not just invalid characters; - Import.jsx never validates the name client-side at all — only mirror-mode errors surface (Import.jsx:364-368); a one-shot import with an invalid name fails only at submit time via the server 400. - The server re-validates on both endpoints, so this is UX-only; no wire or SDK change. ## Architecture notes - Add a shared headless helper (repo convention: `web/src/lib/`, unit-testable in Node — see `lib/orgs.js` `validateOrgName` precedent) such as `validateRepoChars(name)` that returns an error **only** for the invalid-character case, distinct from "required"/empty (empty is already enforced by the disabled submit buttons on both pages). - Both forms consume it in an `onInput` handler; the error region renders **reserved-height** (e.g. a fixed-height slot, `min-h-[1.25rem]` wrapper, or `aria-live` container that keeps its space) so sibling grid rows never shift. - On invalid input, both pages should set `aria-invalid` on the input and keep `aria-describedby` pointing at the message element. - The static help span is removed from both files; charset guidance lives entirely in the error message (shown only on error), mirroring how `lib/orgs.js` messages state the rule ("organization names are lowercase letters, digits, and hyphens, 1–39 characters"). ## Acceptance criteria - [ ] The always-on "Letters, digits, and . _ -" helper span no longer renders on `/new` or `/import`. - [ ] Typing a valid name (`foo.bar-2_baz`) shows no message anywhere; typing an invalid one (`foo bar`, `a/b`, `..`, `.hidden`, `hi git`) shows a red inline message naming the charset rule, live as the user types. - [ ] An empty name shows NO validation message (the disabled submit button already covers required), on both pages. - [ ] Grid shift: with the error appearing and disappearing, no sibling row of either card form moves — verified at narrow width (`grid-cols-1` stacked) as well as `sm:grid-cols-2`. - [ ] Both inputs carry `aria-invalid` + `aria-describedby` wired to the live message; the message container is `aria-live="polite"`. - [ ] Validation logic is one shared pure function in `web/src/lib/` (not copy-pasted per page), consumed by both New.jsx and Import.jsx (and by mirror mode on both pages, which shares the same name field). - [ ] Submit-time behavior unchanged: server 400s still render inline in their existing error blocks; client gate still blocks submit on an invalid name (New.jsx already does via `fieldError`; Import.jsx gains the gate). - [ ] No changes to `web/sdk/src/create.js` semantics or any wire payload.
crueber added this to the v1 milestone 2026-09-13 19:29:19 +00:00
Author
Owner

Fix ready for review: #489 (branch fix/issue-486) — live name validation replaces the always-on helper on /new + /import, reserved-height slot (no grid shift), shared headless rule, Import gains the client gate. No backend change. Tests: full node suite 1055/1053/2 (2 pre-existing live-server smoke fails, identical on main); vite + esbuild green. Browser proof open (shared-daemon loopback guard).

Fix ready for review: #489 (branch fix/issue-486) — live name validation replaces the always-on helper on /new + /import, reserved-height slot (no grid shift), shared headless rule, Import gains the client gate. No backend change. Tests: full node suite 1055/1053/2 (2 pre-existing live-server smoke fails, identical on main); vite + esbuild green. Browser proof open (shared-daemon loopback guard).
Author
Owner

Review of PR #489 (fix/issue-486), verified in scratch worktree /tmp/pr489 @ fe1d272 (+1 review commit fb4fa0b). Full node suite: 1053 pass / 2 fail — the 2 are the pre-existing live-server smoke subtests, byte-identical on pristine main (main smoke run: 1 pass / 2 fail). vite build green. No browser drive (per task rules; no-shift verified by reasoning + structural pins — stated explicitly).

(1) Charset rule mirrors server — EXACT MATCH. Server rule is git.validPart/ParseRepoId (internal/git/contract.go:14-29,32-47): ASCII [A-Za-z0-9._-], 1-100 chars, no leading dot, not '..', '.git' suffix stripped. Client web/src/lib/repo-name.js:15,28-35: same regex (REPO_NAME_RE), trim, single '.git' strip, '..'/leading-dot/>100 checks. Edge parity confirmed by execution: '.git'->invalid both, 'a.git'->valid both, '..'/'.hidden'/101-char->invalid both. .git-strip handled.
(2) Matrix — PASS (executed, all 12): 'foo.bar-2_baz' silent; 'foo bar','a/b','..','.hidden','hi git' all return the rule text; ''/' '/null silent; 100-char ok, 101-char flagged; 'a.git' silent (suffix strip), '.git' flagged.
(3) No-shift — ONE FINDING, FIXED: min-h-[1rem] under-reserved. The 81-char message wraps to 2 lines at sm:2-col cell widths (~300px) and at 390px mobile-stacked, so the error still grew the Owner/Name row at exactly the widths #486 calls out. Fixed in fb4fa0b: New.jsx:225 + Import.jsx:315 min-h-[1rem]->min-h-[2rem], test pin + 12_web_ui.md updated. Reserved slot is always-rendered inside the name label cell (no gate); Owner/Name row 'grid grid-cols-1 gap-3 sm:grid-cols-2' untouched both pages.
(4) Aria — PASS: both inputs carry aria-invalid={!!nameCharsError()} + aria-describedby->{new,import}-name-error; message

has aria-live='polite' (New.jsx:222-225, Import.jsx:312-315).
(5) Shared helper — PASS: single validateRepoChars in web/src/lib/repo-name.js consumed by both pages; mirror modes share the same name field so both covered. No copy-paste.
(6) Submit gates — PASS: Import button gains '|| !!nameCharsError()' (Import.jsx:401) + start() guard before the mirror branch covering both modes, landing in existing error block (setErr/setPhase('error')/reportError/setBusy(false), Import.jsx:133-143). New's fieldError gate intact (New.jsx:80,301).
(7) Helper spans removed — PASS both (no 'new-name-help'/'import-name-help', no 'URL path after the owner'); old shifting <Show when={fieldError() && getName()}> block deleted (was also firing on required).
(8) No backend/SDK/wire change, no new deps (dependency-free lib, no package.json diff), docs accurate (12_web_ui.md entry updated for the 2rem fix in the same commit; law 12 satisfied; laws 1/7/8 hold).

Note (non-blocking): New.jsx owner-charset errors are now silent (button disabled, no message) since the removed block was the only inline display for e.g. 'invalid owner'. Acceptable per #486 scope (name-only), flagging for awareness.

No browser proof (shared-daemon loopback guard, same as PR description). MERGE RECOMMENDATION: ready to merge.

Review of PR #489 (fix/issue-486), verified in scratch worktree /tmp/pr489 @ fe1d272 (+1 review commit fb4fa0b). Full node suite: 1053 pass / 2 fail — the 2 are the pre-existing live-server smoke subtests, byte-identical on pristine main (main smoke run: 1 pass / 2 fail). vite build green. No browser drive (per task rules; no-shift verified by reasoning + structural pins — stated explicitly). (1) Charset rule mirrors server — EXACT MATCH. Server rule is git.validPart/ParseRepoId (internal/git/contract.go:14-29,32-47): ASCII [A-Za-z0-9._-], 1-100 chars, no leading dot, not '..', '.git' suffix stripped. Client web/src/lib/repo-name.js:15,28-35: same regex (REPO_NAME_RE), trim, single '.git' strip, '..'/leading-dot/>100 checks. Edge parity confirmed by execution: '.git'->invalid both, 'a.git'->valid both, '..'/'.hidden'/101-char->invalid both. .git-strip handled. (2) Matrix — PASS (executed, all 12): 'foo.bar-2_baz' silent; 'foo bar','a/b','..','.hidden','hi git' all return the rule text; ''/' '/null silent; 100-char ok, 101-char flagged; 'a.git' silent (suffix strip), '.git' flagged. (3) No-shift — ONE FINDING, FIXED: min-h-[1rem] under-reserved. The 81-char message wraps to 2 lines at sm:2-col cell widths (~300px) and at 390px mobile-stacked, so the error still grew the Owner/Name row at exactly the widths #486 calls out. Fixed in fb4fa0b: New.jsx:225 + Import.jsx:315 min-h-[1rem]->min-h-[2rem], test pin + 12_web_ui.md updated. Reserved slot is always-rendered inside the name label cell (no <Show> gate); Owner/Name row 'grid grid-cols-1 gap-3 sm:grid-cols-2' untouched both pages. (4) Aria — PASS: both inputs carry aria-invalid={!!nameCharsError()} + aria-describedby->{new,import}-name-error; message <p> has aria-live='polite' (New.jsx:222-225, Import.jsx:312-315). (5) Shared helper — PASS: single validateRepoChars in web/src/lib/repo-name.js consumed by both pages; mirror modes share the same name field so both covered. No copy-paste. (6) Submit gates — PASS: Import button gains '|| !!nameCharsError()' (Import.jsx:401) + start() guard before the mirror branch covering both modes, landing in existing error block (setErr/setPhase('error')/reportError/setBusy(false), Import.jsx:133-143). New's fieldError gate intact (New.jsx:80,301). (7) Helper spans removed — PASS both (no 'new-name-help'/'import-name-help', no 'URL path after the owner'); old shifting <Show when={fieldError() && getName()}> block deleted (was also firing on required). (8) No backend/SDK/wire change, no new deps (dependency-free lib, no package.json diff), docs accurate (12_web_ui.md entry updated for the 2rem fix in the same commit; law 12 satisfied; laws 1/7/8 hold). Note (non-blocking): New.jsx owner-charset errors are now silent (button disabled, no message) since the removed <Show> block was the only inline display for e.g. 'invalid owner'. Acceptable per #486 scope (name-only), flagging for awareness. No browser proof (shared-daemon loopback guard, same as PR description). MERGE RECOMMENDATION: ready to merge.
Author
Owner

Fixed by PR #489 (review clean + one min-height shift fix by reviewer; server-rule parity executed, matrix + aria verified), merged. Closing.

Fixed by PR #489 (review clean + one min-height shift fix by reviewer; server-rule parity executed, matrix + aria verified), merged. Closing.
Sign in to join this conversation.
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
crueber/walhub#486
No description provided.