-
-
Notifications
You must be signed in to change notification settings - Fork 88
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[field controls] Respect validationMode
#1053
base: master
Are you sure you want to change the base?
Conversation
Netlify deploy preview |
Signed-off-by: atomiks <[email protected]>
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: atomiks <[email protected]>
@@ -7,6 +7,7 @@ import { CheckboxGroupContext } from './CheckboxGroupContext'; | |||
import type { FieldRoot } from '../field/root/FieldRoot'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't think it's directly related to this PR, but I couldn't make a required checkbox group because required
is a prop on individual checkboxes but not on CheckboxGroup:
// a combination of the CheckboxGroup and Field demos
export default function FieldCheckbox() {
return (
<Field.Root>
<Field.Label>Pick one or more</Field.Label>
<CheckboxGroup defaultValue={[]}>
<label>
<Checkbox.Root name="fuji-apple">
<Checkbox.Indicator>
<CheckIcon />
</Checkbox.Indicator>
</Checkbox.Root>
Fuji
</label>
<label>
<Checkbox.Root name="gala-apple">
<Checkbox.Indicator>
<CheckIcon />
</Checkbox.Indicator>
</Checkbox.Root>
Gala
</label>
<label>
<Checkbox.Root
name="granny-smith-apple"
>
<Checkbox.Indicator>
<CheckIcon />
</Checkbox.Indicator>
</Checkbox.Root>
Granny Smith
</label>
</CheckboxGroup>
<Field.Error match="valueMissing">
You must pick at least one
</Field.Error>
</Field.Root>
);
}
@@ -9,6 +9,8 @@ import { useDirection } from '../direction-provider/DirectionContext'; | |||
import { useRadioGroup } from './useRadioGroup'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://codesandbox.io/p/sandbox/sweet-brattain-q9ptqr
When I make a required RadioGroup that has no initially checked radio and tab through it without checking any of the radios, it doesn't trigger an error state (I logged some stuff and it seems the validation functions are being called)
Also when the required RadioGroup is wrapped in a Form (with a submit button), if I click the submit button without touching the RadioGroup the error doesn't appear which could be the same issue?
However when a radio is checked, the console.log
in the Form's submit handler doesn't log the value 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these bugs (with CheckboxGroup as well) related to this PR or are they on master as well? If on master, we can create a new issue for them to separate the work
Closes #1050