Repository navigation
fix(bulma-ui): associate Autocomplete and Taginput labels with their inner inputs - #496
Conversation
…inner inputs The label prop on both components rendered an unassociated Field label, and a user-supplied id landed on the wrapper div instead of the labellable input. Both now run through useAutoLabelId: the inner input gets your id or a generated one, the label gets a matching htmlFor, and an explicit labelProps.htmlFor still wins. Behavior notes: a user id now applies to the inner text input rather than the wrapper div (nothing in the repo or docs relied on the old placement); Taginput's hardcoded aria-label="Add tag" becomes a fallback so a wired visible label names the input, and at maxTags (no input rendered) the label stays unwired rather than pointing at nothing.
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Walkthrough
ChangesForm label association
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant FormConsumer
participant AutocompleteOrTaginput
participant useAutoLabelId
participant Field
participant InnerInput
FormConsumer->>AutocompleteOrTaginput: provide label, labelProps, and optional id
AutocompleteOrTaginput->>useAutoLabelId: derive control ID and label props
AutocompleteOrTaginput->>Field: render computed label props
AutocompleteOrTaginput->>InnerInput: render computed control ID
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
Extends #492’s automatic label association to Autocomplete and Taginput.
Changes:
- Moves supplied/generated IDs to inner text inputs and wires labels.
- Refines
Taginputaccessible naming andmaxTagsbehavior. - Adds tests, documentation, and skill guidance.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
bulma-ui/src/form/Autocomplete.tsx |
Wires labels to the inner input. |
bulma-ui/src/form/Taginput.tsx |
Adds label wiring and accessible-name fallback logic. |
bulma-ui/src/form/fieldProps.ts |
Updates association guidance. |
bulma-ui/src/form/__tests__/Autocomplete.test.tsx |
Tests label and ID behavior. |
bulma-ui/src/form/__tests__/Taginput.test.tsx |
Tests label, fallback, and limit behavior. |
docs/docs/api/form/autocomplete.md |
Documents association and ID relocation. |
docs/docs/api/form/taginput.md |
Documents accessibility behavior. |
skills/bestax-form/SKILL.md |
Adds both components to auto-wired guidance. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Preview DeploymentPreview URL: https://7b4e26a6.bestax.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@bulma-ui/src/form/Taginput.tsx`:
- Around line 652-654: Update the aria-label condition in Taginput so the
fallback “Add tag” is removed only when fieldLabelProps.htmlFor matches a
defined controlId; otherwise preserve it, including explicit non-matching
htmlFor values. Add a regression test covering labelProps={{ htmlFor: 'other' }}
without a matching id and verify the input retains its accessible name.
In `@docs/docs/api/form/autocomplete.md`:
- Line 474: Update the ref type documentation for the Autocomplete API at
docs/docs/api/form/autocomplete.md:474-474 and the TagInput API at
docs/docs/api/form/taginput.md:540-540 from React.Ref<HTMLElement> to
React.Ref<HTMLInputElement>, preserving the existing ref descriptions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 51ce0b1b-2d39-44ab-be9b-6075cc823a0f
📒 Files selected for processing (8)
bulma-ui/src/form/Autocomplete.tsxbulma-ui/src/form/Taginput.tsxbulma-ui/src/form/__tests__/Autocomplete.test.tsxbulma-ui/src/form/__tests__/Taginput.test.tsxbulma-ui/src/form/fieldProps.tsdocs/docs/api/form/autocomplete.mddocs/docs/api/form/taginput.mdskills/bestax-form/SKILL.md
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Deep review — 0 blocking · 1 advisory
| # | Severity | Area | Finding | Location |
|---|---|---|---|---|
| 1 | 🔵 Advisory | Accessibility | Taginput's aria-label="Add tag" fallback is suppressed whenever a label is rendered outside a Field, even when labelProps.htmlFor is overridden to a target other than this input — leaving the input with no id, no wired label, and no fallback name (a nameless control). |
bulma-ui/src/form/Taginput.tsx:654 |
Overall: The change is sound and does exactly what #493 asks: id is destructured out of the wrapper spread and threaded onto the inner labellable <input> in both components, the label is wired through the shared useAutoLabelId hook (generated id, user id, or labelProps.htmlFor override), and Taginput's hardcoded aria-label correctly becomes a fallback in the common cases. Tests (201 passing), docs, the fieldProps.ts neutrality comment, and the bestax-form skill were all updated consistently. The riskiest spot is Taginput's fallback-name heuristic (label && !insideField), which keys off "is a label rendered" rather than "does the wired label actually name this input" — a divergence that only surfaces under an unusual labelProps.htmlFor override; the human can decide whether to tighten it now or leave it documented.
Residual risk:
- Nameless-input variants of the addressed class — the
aria-labelfallback misfires only whenlabelis set ANDlabelProps.htmlForis overridden to a non-matching id (noidprop given). In every normal path the label is wired (getByLabelTexttests confirm) or theAdd tagfallback is kept (unlabeled / inside-Field tests confirm). Theid-without-labelcase correctly keepsAdd tagbecause the heuristic keys offlabel, notcontrolId. Recorded as the advisory above rather than a blocker because it requires a deliberate, self-defeating override. idrelocation breaking internal wiring — refuted: neither component derivesaria-controls/popover/aria-activedescendantfromid; the relocatedidfeeds only the<input>, so no listbox/dropdown association is disturbed.- maxTags dangling
for— refuted: atmaxTagsthe input isn't rendered andrendersLabelis false, souseAutoLabelIdinjects nohtmlFor; the label renders unwired (test-covered) rather than pointing at a missing control. Hooks stay unconditional (useIdcalled every render), so the threshold toggle is hydration-safe.
🏄 Clean little set, dude — the id finally rides the real input instead of wiping out on the wrapper div, and the labels all link up nice. Only ripple is Taginput dropping its "Add tag" name if you paddle the htmlFor somewhere weird on purpose. Good to send it. 🌊
… its input Review round on #496: the aria-label suppression keyed off "a label is rendered" rather than "the wired label names this input", so label plus a labelProps.htmlFor pointing elsewhere (with no id) produced a nameless control. The condition now requires the rendered label's htmlFor to equal the input's id. Also corrects the documented ref types to React.Ref<HTMLInputElement>.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
bulma-ui/src/form/Taginput.tsx:660
- This can remove the input's only accessible name even though no label is rendered. For example,
<Taginput id="tags" labelProps={{ htmlFor: 'tags' }} />produces matching values here, butFieldomits the absentlabel, soaria-labelbecomes undefined. Gate suppression on the convenience label actually being rendered and not being inside an outerField.
controlId && fieldLabelProps?.htmlFor === controlId
Preview DeploymentPreview URL: https://b2daa02b.bestax.pages.dev |
|
🎉 This PR is included in version 5.8.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.9.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 4.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 1.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #493. Second of three stacked label-association PRs following #492 (merge order: this one → #494's PR → #495's PR).
What
The convenience
labelprop on Autocomplete and Taginput rendered an unassociated Field label, and a user-suppliedidlanded on the wrapper<div>instead of the labellable inner<input>. Both now run through theuseAutoLabelIdhook from #492: the inner input gets yourid(or a generated one), the label gets a matchingfor, and an explicitlabelProps.htmlForwins outright.Behavior changes to review
idnow applies to the inner text input (the labellable control), not the wrapper div. Verified nothing in the repo, docs, stories, or skills relied on the old placement; anyone targeting the wrapper by#idin CSS/tests would need to adjust. Documented in the props tables.aria-label="Add tag"becomes a fallback — suppressed when the wired visible label names the input (aria-label would otherwise shadow it), kept when unlabeled or nested in an outerField.maxTags: the text input isn't rendered, so the label deliberately renders withoutforinstead of dangling.Tests
New
describe('label association (#493)')blocks in both suites: association viagetByLabelText, id relocation locked (input has it, wrapper doesn't),labelProps.htmlForoverride, no-label/no-id, inside-Field no injection, both aria-label fallback directions, and the maxTags unwired case. All React 18/19-safe (no id text asserted).pnpm allgreen.Docs / skill
Accessibility sections updated on
autocomplete.mdandtaginput.md; TSDoc shadow-redeclares drive the regenerated props tables;skills/bestax-formmoves both components into the auto-wired list (remaining manual:Field-composed usage and the group inputs — #494 and #495 are next in the stack).Summary by CodeRabbit
Accessibility Improvements
Documentation