refactor(forms): adopt FieldLabel and FieldError for hand-rolled labels - #8117
Draft
talissoncosta wants to merge 7 commits into
Draft
refactor(forms): adopt FieldLabel and FieldError for hand-rolled labels#8117talissoncosta wants to merge 7 commits into
talissoncosta wants to merge 7 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
front-end
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Jul 27, 2026
talissoncosta
changed the title
Wrapper + FieldLabel + control + FieldError as one layout-only component: unlike the InputGroup component slot it never pretends to wire a control it cannot reach, wiring is the explicit htmlFor. With Field, PasswordField/DateField/SwitchField wrappers never need to exist. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The component slot renders custom controls with no label or error wiring. The 15 non-Select sites now render FieldLabel (plus a form-group wrapper) directly; title-less sites just unwrap. The remaining component= sites are Selects, which move to SelectField in a follow-up, after which the prop is deleted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The label must live inside CopyField: htmlFor needs the inner read-only input's id, which callers cannot reach. The SCIM modal's hand-rolled label collapses into the title prop. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
DateSelect never exposed an id, so neither of its two call sites could associate its label with the input. react-datepicker forwards id to the input, so both labels now resolve. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Chip close icons standardise on Icon name='close' width 18 (the chip-icon box size); the two ad-hoc 13px inline styles go with it. CreateSAML's download icon stays on IonIcon until the icon set gains a download SVG. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each hand-rolled wrapper + FieldLabel pair becomes a single Field element (net -61 lines at the sites). Also annotates two CreateGroup mutation callbacks whose inference flips when anything in the service import cycle changes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
talissoncosta
force-pushed
the
refactor/fieldlabel-component-sites
branch
from
July 29, 2026 13:56
7983e5f to
99dc7a2
Compare
Field provides {controlId, errorId, hasError} when htmlFor is set;
Input adopts them as defaults (explicit props win), so the id is
declared once on Field and aria-invalid/aria-describedby wire
themselves. Context only exists for an explicit htmlFor, keeping
wiring a visible call-site decision. Pattern follows Primer
FormControl / React Aria field context.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Member
|
@talissoncosta is this blocked on anything? If not let's resolve anything outstanding and get a PR up |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Contributes to #8120 (epic #8119)
First adoption pass of the field components over hand-rolled form labels and unwired errors, and it introduces
Field: the shared field skeleton (wrapper +FieldLabel+ control +FieldError, wiring via an explicithtmlFor). Each migrated site is a single element; withFieldthere is no need to ever build PasswordField/DateField/SwitchField wrappers.InputGroup'scomponentslot, which wires nothing) now renderField: metadata editors, date picker, colour picker, value editor, switch, copy field, checkbox group, search rows, user/group chips. Title-less sites just unwrap. Follow-up:InputGroup/SelectFieldcomposeFieldinternally so the app has one field skeleton.CopyFieldowns its label: optionaltitle, wired internally (callers cannot reach the inner input's id forhtmlFor).DateSelectforwards anid, so its two call sites' labels (change request schedule, API key expiry) actually resolve, plus the raw<label>Expiry</label>in AdminAPIKeys is now wired.IonIconfor the customIcon(18px, matching the chip-icon box); ad-hoc inline font sizes go with it.Not in scope here: the wider raw-
<label>population (~87 occurrences across 42 files) continues in batches under #8120, and nothing aboutInputGroup'scomponentprop changes in this PR.How did you test this code?
tsc: 0 new errors vsmain.eslint: clean on all changed files.WithTitlestory added.Smoke test (light + dark):