feat(ui): standardize optional-field labels and required semantics (#242)

[daniilperkin]

What this does

Closes the remaining scope of #242 — Improve optional-field labels and keyboard navigation: forms get one shared, accessible convention for marking optional and required fields, wired through the Field primitive so no caller can get it wrong per-form.

• Field gains optional?: boolean → renders a styled (optional) inside the label (visible and in the accessible name).
• fieldContext carries required → Input / Textarea / Select set aria-required="true". The asterisk alone was aria-hidden decoration — invisible to screen readers.
• 12 forms migrated or swept; tests that pinned the old exact label strings are relaxed to the leading part (/^Name/, …).
• New axe coverage: wizard sources step, inline GitHub token form, desktop token companion — plus end-to-end semantics assertions.

Base-branch decision (dev, not #267)

The plan left this open; evidence says base on dev:

• Zero file overlap with Make the buddy the tutor along the onboarding path #267 (feature/311-buddy-onboarding-tutor, 72 files — board/buddy/onboarding/starter-work areas). None of this PR's files appear in its diff.
• Make the buddy the tutor along the onboarding path #267 is currently CONFLICTING with dev; stacking this on it would couple this PR's mergeability to a conflicted branch for no shared-file benefit.
• PRs cut from the same base already show clean incremental diffs; per the repo's stacking discipline, hard-stacking is reserved for 3+ PRs racing on the same core files — not the case here.

Relation to #268

Issue #242's keyboard-navigation half (focus restore after the inline token form closes) is @DavidLeuter's #268 (hotfix/token-companion-keyboard-focus) — green, awaiting review. This branch deliberately does not absorb it; it closes the label/semantics scope only. Whichever merges first, dev ends up with both halves of #242.

1. Shared primitive (feat(ui))

Change	Why
Field.optional	One marker, one place. Renders " (optional)" inside the <label>, so it joins the control's accessible name — the convention screen readers announce with the field.
fieldContext.required	The asterisk is aria-hidden; the semantics now travel via context to the control as aria-required.
Input / Textarea / Select	Precedence: explicit aria-required → HTML required → enclosing Field. Emitted only when true — existing plain inputs do not grow a stray aria-required="false".

Gotcha worth knowing: the space before (optional) is its own text node, not the span's first character — dom-accessibility-api trims each element child's contribution, so a leading space inside the span vanished from the accessible name while surviving in textContent (Nickname (optional) vs Nickname(optional)). The separate node keeps both computations identical, and a unit test pins it.

2. Form migrations (refactor(forms))

Removed ad-hoc copies (visible text deliberately unchanged):

• AddCardForm, NoteCard — "Title (optional)", "What to call it (optional)" label strings.
• WizardDetailsStep — Industry's "Optional. " hint prefix (the hint keeps the real guidance); Description now marks itself optional.
• AddArrivalStepModal — same hint-prefix cleanup; "Optional link." → "Link to tool or docs.". "What needs to be done" now declares the required-ness its canSubmitCustom guard already enforced.

Migrated outright:

• NewStarterTaskModal — bypassed Field entirely (raw <label>/<input> pairs, a local inputClasses string, a span-based "(optional)"). Now four Fields around Input/Textarea; the competency-keys guidance moved into Field's hint slot, which gets aria-describedby for free.

Marked where the code already enforces it:

• TokenAddForm, AtlassianCredentialAddForm, TokenRotateForm — credential fields already gated by required inputs / submit guards.
• ProjectDetailsDrawer — Name required, Description optional, matching the wizard's semantics for the same fields.
• Sweep for the same rule: AccountForm (first/last/email carry required), NewAreaForm (submit disabled while empty), ConfluenceConnectStep (base URL + Space ID carry required).

Deliberately not changed:

• ArrivalStepAuthoring — its editor tolerates a blank title in the save path (no guard to mirror), so it gets no required mark.
• Placeholder-level "(optional)" strings (e.g. https://… (optional)) — that is guidance text, not a field mark.

3. Tests (test(a11y) + unit)

• Field.test.tsx: +7 tests — optional label in visible text and accessible name; required wins over optional; optional controls are not marked required; propagation to all three controls; HTML-required mirror; explicit aria-required={false} override.
• CreateProjectWizard.a11y.test.tsx: +4 tests — sources step axe; inline GitHub token form axe (asserts aria-required on both token fields); desktop companion axe via mockViewport(true) — the portalled second dialog is included because baseElement is the whole body; a semantics test asserting Name → aria-required while Description (optional) / Industry (optional) are labelled.
• Query relaxations caused by the new marks (5 test files): /^Name/, /^Description/, /^Title/, /^Industry/, /^Space ID/, /^Confluence base URL/, /^Token name/.

Verification

npm run try on Node 22 — full chain, all green:

format:check   All matched files use Prettier code style!
build          tsc -b && vite build — ok
lint           eslint . — clean
unit           331 files / 3226 tests passed
a11y            55 files /   73 tests passed

Reviewer notes

• 24 files, +319/−114. Three commits: primitive → migrations → a11y coverage; every commit is independently green.
• The (optional) text joining the accessible name is deliberate (Improve optional-field labels and keyboard navigation #242 asks for it); the asterisk stays out of it — aria-required carries that meaning instead.
• If any surface still queries a newly marked label by its full old string, CI will surface it — the fix is the leading-part regex.

[kiranfin]

Review

I did not find anything blocking.

🟡 Worth a look

1. Field required only produces aria-required, never the native HTML required attribute — src/components/ui/Input.tsx, Select.tsx, Textarea.tsx (the isRequired = ariaRequired ?? rest.required ?? field?.required line in each).
   Most of the newly-marked-required fields in this PR (ProjectDetailsDrawer "Name", NewAreaForm, ConfluenceConnectStep, TokenAddForm, TokenRotateForm, the three AtlassianCredentialAddForm fields, both AtlassianCredentialRow fields, NewStarterTaskModal "Title", AddArrivalStepModal) set required only on Field, not on the wrapped control. That's enough for aria-required, but the underlying <input>/<textarea> never gets the native required attribute unless the caller also passes it directly (as AccountForm and the Atlassian forms happen to do). Practically this is a no-op here since every one of these forms gates submission through a disabled button rather than native constraint validation, and it's not a regression — none of these controls had required before either. But it's a bit of a trap for the next person: <Field required> visually and semantically (for AT) reads as "this is required," yet it silently does not turn on the browser's own :required/native-validation behavior, which could be surprising for a control used outside a JS-gated form. Worth either a short note in Field's doc comment, or having Field's required flow through to the control's native required too, so the two can't drift.

Overall: solid, self-contained accessibility fix with strong test coverage (including two full a11y suites exercising axe against the new required/optional states). I'd merge as-is; item 1 is a documentation/consistency thought for later, not something that should hold this up.

[daniilperkin]

Thanks @kiranfin — took the documentation option (7ac15cc0), plus pinned it with a test so the note can't rot. One correction to the list while I'm here, because I re-ran the sweep before writing this.

The actual split (measured, repo-wide)

<Field required> sites that ALSO carry the native attribute on the control: 29 — including every site you listed as Field-only:

• TokenAddForm (both inputs), TokenRotateForm (New GitHub PAT), ConfluenceConnectStep (both), all three AtlassianCredentialAddForm fields, both AtlassianCredentialRow fields — all dual, they pass required to the <Input> as well (that's also why AccountForm's and the Atlassian forms' behaviour you called out as the exception is in fact the majority).

Field-only sites from this PR: exactly four — NewStarterTaskModal "Title", NewAreaForm "Name this area", AddArrivalStepModal "What needs to be done", ProjectDetailsDrawer "Name". (Plus two pre-existing ones outside the PR: the wizard's "Name" and StepQuickEdit's "Title".) All four are guarded, matching your read: canSave / canSubmitCustom / disabled submit / saveChanges's early return with "Project name is required.".

What changed

• Field's required doc now says what it is — presentation and ARIA — and what it deliberately is not: no native required on the wrapped control, so no :required styling and no browser popup. It points at the escape hatch: pass required to the control when native validation is wanted.
• fieldContext's required carries the same boundary note, since that's where a control author reads it.
• A unit test pins both directions: Field-only mark ⇒ input.required === false (attribute off), control-level required ⇒ lands on the element. If the two ever drift, the suite says so instead of a reviewer.

Why not flow-through (your option 2)

Making required on Field imply the native attribute would change runtime behaviour in forms that validate themselves — the guarded buttons and inline errors would silently become a second, competing validator, with browser chrome (locale-dependent popups) instead of the app's own copy. That's a real design change for ten-plus forms, and it would be odd to smuggle it inside a label/semantics PR. If the team prefers drift-impossible over explicit, I'm happy to file it as its own issue with a per-form audit of the popups — say the word.

Gate re-run after the change: npm run try (Node 22) green — unit 332 files / 3229 tests, a11y 55 / 73. CI re-runs on the push.