fix: remove fieldset/legend wrapping single-control form components - #40
Merged
ViewFromTheBox merged 2 commits intoMay 25, 2026
Merged
Conversation
Single labeled inputs (text, email, textarea, select, etc.) were wrapping each control in <fieldset>/<legend>, which is wrong semantics — fieldset is for groups of related controls (radio sets, checkbox groups). Screen readers announced lone inputs as "group" and hosts without a daisyUI reset got the default browser fieldset border. Switched Input, Textarea, Password, Pin, DatePicker, Select, RichTextEditor, Editor, File, ColorPicker, and Range to render a <div class="fieldset"> + <label class="fieldset-legend" htmlFor> + <span class="input"> pattern (daisyUI v5 utility classes on semantic elements). Inline mode on Input, Password, and DatePicker now uses daisyUI v5's floating-label pattern. RichTextEditor uses aria-labelledby since htmlFor doesn't bind to contenteditable divs. Radio, Checkbox group, and Toggle keep <fieldset> since they're genuine control groups. Added regression tests asserting no <fieldset>/<legend> wraps a single control plus label↔input association for each affected component. Closes #39
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (2)
📝 WalkthroughWalkthroughThis PR replaces single-control ChangesForm Component Semantic Markup Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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 `@packages/react/src/components/form/Input/Input.tsx`:
- Around line 128-135: The clear button in the Input component is rendered only
when clearable is true and currently sets tabIndex={-1}, removing it from
keyboard navigation; update the clear button (the element using onClear inside
the Input component) to be keyboard-focusable (remove tabIndex or set
tabIndex={0}) so keyboard users can reach and activate the clear action,
ensuring aria-label="Clear input" remains present for screen readers.
In `@packages/react/src/components/form/Password/Password.tsx`:
- Around line 130-148: The clear and visibility-toggle buttons (rendered when
clearable and !hideToggle) are removed from keyboard tab order by tabIndex={-1};
update the JSX for those buttons (the clear button that calls onClear and the
toggle button that calls setVisible and reads visible) to remove tabIndex or set
tabIndex={0} so they are keyboard-focusable, and ensure aria-labels remain
unchanged for screen reader clarity.
🪄 Autofix (Beta)
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: ASSERTIVE
Plan: Pro
Run ID: b85a3bff-a56a-4065-88b2-f89042b6d845
📒 Files selected for processing (22)
packages/react/src/__tests__/form/ColorPicker.test.tsxpackages/react/src/__tests__/form/DatePicker.test.tsxpackages/react/src/__tests__/form/Editor.test.tsxpackages/react/src/__tests__/form/File.test.tsxpackages/react/src/__tests__/form/Input.test.tsxpackages/react/src/__tests__/form/Password.test.tsxpackages/react/src/__tests__/form/Pin.test.tsxpackages/react/src/__tests__/form/Range.test.tsxpackages/react/src/__tests__/form/RichTextEditor.test.tsxpackages/react/src/__tests__/form/Select.test.tsxpackages/react/src/__tests__/form/Textarea.test.tsxpackages/react/src/components/form/ColorPicker/ColorPicker.tsxpackages/react/src/components/form/DatePicker/DatePicker.tsxpackages/react/src/components/form/Editor/Editor.tsxpackages/react/src/components/form/File/File.tsxpackages/react/src/components/form/Input/Input.tsxpackages/react/src/components/form/Password/Password.tsxpackages/react/src/components/form/Pin/Pin.tsxpackages/react/src/components/form/Range/Range.tsxpackages/react/src/components/form/RichTextEditor/RichTextEditor.tsxpackages/react/src/components/form/Select/Select.tsxpackages/react/src/components/form/Textarea/Textarea.tsx
Remove tabIndex={-1} from the clear button in Input and from the clear /
visibility-toggle buttons in Password. The buttons rely on aria-label for
screen reader naming, so removing them from the tab order made the actions
unreachable for keyboard users.
Surfaced by CodeRabbit during review of #39.
github-project-automation
Bot
moved this from In Review
to Done
in ArtisanPack UI Overview
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
1 participant
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.
Description
The form components were wrapping each single labeled control in
<fieldset>/<legend>, which is wrong semantics —<fieldset>is for groups of related controls (radio sets, checkbox groups, related questions), not lone inputs. This caused two real bugs downstream:artisanpack-ui/formsTextField→Input).Fixed by switching every single-control form component to daisyUI v5's
fieldsetutility class on a plain<div>(not a<fieldset>element), paired with a<label class="fieldset-legend" htmlFor>and a presentational<span class="input">wrapper. Inline mode onInput,Password, andDatePickernow uses daisyUI v5'sfloating-labelpattern.RichTextEditorusesaria-labelledbysincehtmlFordoesn't bind tocontenteditabledivs.Radio,Checkbox(group), andTogglekeep<fieldset>/<legend>since they're genuine control groups.Closes: #39
Type of Change
Related Issue
Issue: #39
Motivation and Context
Reported via downstream Keystone CMS consumer — every contact-form field rendered as a fieldset-wrapped input, both ugly and inaccessible. Root cause was the v4-style markup left over from earlier work; daisyUI v5's
.fieldsetand.fieldset-legendare utility classes that work on any element, so we can keep the visual style while using semantically correct HTML.Changes Made
Input,Textarea,Password,Pin,DatePicker,Select,RichTextEditor,Editor,File(both modes),ColorPicker,Range: replaced<fieldset class="fieldset">+<legend class="fieldset-legend">with<div class="fieldset">+<label class="fieldset-legend" htmlFor>.Input,Password,DatePicker: inline mode now wraps via<label class="floating-label">(daisyUI v5 floating-label pattern); single-space placeholder injected when no caller placeholder is supplied so:placeholder-showntriggers correctly.RichTextEditor: switched fromhtmlFor/aria-labeltoaria-labelledbypointing at the legend<span>, since<label htmlFor>does not associate with acontenteditablediv.Pin: dropped<fieldset>wrapper; the innerrole="group" aria-label="PIN input"keeps the digits grouped accessibly.DatePicker: extracted sharedcommonInputPropsobject to remove duplication between the adorned and bare input branches.<fieldset>/<legend>wraps a single control and label↔input association is correct.How Has This Been Tested?
Testing Environment:
@artisanpack-ui/react1.0.1-rcTests Performed:
npx vitest run— 702/702 passnpx tsc --noEmit -p packages/react/tsconfig.json— cleannpx eslint packages/react/src— cleanAccessibility Tests Run
Details: Verified that lone inputs no longer get announced with the "group" role;
aria-describedbystill wires hint/error nodes correctly;RichTextEditor'scontenteditableis now properly labeled viaaria-labelledby.Tests Added
Test details: Added regression tests in
Input,Textarea,Password,Pin,DatePicker,Select,RichTextEditor,Editor,File,ColorPicker,Rangetest files asserting no<fieldset>/<legend>wraps a single control. Added label↔input association tests (viagetByLabelText/htmlFor/aria-labelledby) for the same set. StrengthenedInputinline-mode test to assert the input is a descendant of thefloating-labelwrapper and carries theplaceholder=" "trigger.Documentation
Documentation details: Updated the JSDoc on
Inputto document the v5 pattern (and to note that<fieldset>/<legend>are reserved for control groups). Added inline comments explaining the deliberate single-space placeholder used to trigger:placeholder-shownfor floating-label CSS.Pre-Submission Checklist
Summary by CodeRabbit
Refactor
Tests