fix(form): use native as default validation behavior #4425
Conversation
🦋 Changeset detectedLatest commit: 7b6c02c The changes in this PR will be included in the next version bump. This PR includes changesets to release 11 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces updates to the NextUI library, specifically focusing on form validation behavior. The default validation behavior has been changed to "native," and the explicit Changes
Possibly related PRs
Suggested Labels
Suggested Reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (4)
apps/docs/content/components/form/form-usage.raw.jsx (1)
Add
validationBehavior="aria"to ensure consistent accessibility validationThe form example is missing the
validationBehavior="aria"prop that is present in other form examples in the codebase (demo.raw.jsxandcustom-validation-aria.raw.jsx). While the form has basic validation attributes:
isRequiredtype="email"errorMessageAdding
validationBehavior="aria"to the Form component would ensure consistent accessibility validation behavior across all form examples.🔗 Analysis chain
Line range hint
17-24: Verify accessibility with native validation.The Input component correctly implements native validation with:
- Required field indicator (
isRequired)- Proper error message
- Semantic email type
- Accessible label
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for consistent validation behavior across form examples rg -l 'validationBehavior="aria"' apps/docs/content/components/form/Length of output: 182
Script:
#!/bin/bash # Check the implementation details of the form examples rg -A 5 'validationBehavior="aria"' apps/docs/content/components/form/Length of output: 1164
Script:
#!/bin/bash # Check if there are other validation behaviors used rg 'validationBehavior=' apps/docs/content/components/form/Length of output: 292
Script:
#!/bin/bash # Check for other accessibility attributes in the form rg -A 2 'aria-|role=' apps/docs/content/components/form/form-usage.raw.jsxLength of output: 76
Script:
#!/bin/bash # Check the form-usage.raw.jsx file content to understand the validation context cat apps/docs/content/components/form/form-usage.raw.jsxLength of output: 918
Script:
#!/bin/bash # Check if there are any validation-related props or behaviors in the Form component rg -A 3 '<Form' apps/docs/content/components/form/form-usage.raw.jsxLength of output: 211
apps/docs/content/components/form/controlled.raw.jsx (1)
Add
validationBehaviorprop to Form component for explicit validation controlThe current implementation might lead to inconsistent validation behavior since
validationBehavioris not specified. According to the documentation:
- Form validation uses ARIA validation by default (
validationBehavior="aria")- The current implementation combines controlled input with native validation attributes (
isRequired,errorMessage)- For proper native HTML form validation that prevents form submission on invalid input,
validationBehavior="native"should be explicitly set• Add
validationBehavior="native"to the Form component to ensure proper validation behavior with the controlled input:<Form className="w-full max-w-xs" onSubmit={onSubmit} validationBehavior="native">🔗 Analysis chain
Line range hint
16-27: Verify controlled input behavior with native validation.The implementation correctly combines controlled input behavior with native form validation. However, we should ensure that the native validation doesn't conflict with the controlled state updates.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential validation conflicts in controlled components rg -A 5 'onValueChange.*isRequired.*errorMessage' apps/docs/content/components/form/Length of output: 86
Script:
#!/bin/bash # Let's search for the controlled form component implementation rg -l "Form.*onSubmit.*Input.*isRequired.*errorMessage" apps/docs/content/components/form/ # Check the complete implementation of controlled form cat apps/docs/content/components/form/controlled.raw.jsx # Search for any validation behavior documentation or examples rg -i "validationBehavior.*form" apps/docs/content/Length of output: 2132
apps/docs/content/components/form/demo.raw.jsx (1)
Line range hint
134-143: Remove explicit validationBehavior from CheckboxThe Checkbox component still uses explicit
validationBehavior="aria"which contradicts the PR objective to use "native" as default.<Checkbox isRequired classNames={{ label: "text-small", }} isInvalid={!!errors.terms} name="terms" - validationBehavior="aria" value="true" onValueChange={() => setErrors((prev) => ({...prev, terms: undefined}))} >apps/docs/content/docs/components/form.mdx (1)
Line range hint
80-96: Update example code to reflect new defaultsThe example code explicitly sets
validationBehavior="aria"which might confuse users since it contradicts the new default behavior explanation.-<Form validationBehavior="aria"> +<Form> <Input isRequired name="username"
🧹 Nitpick comments (11)
packages/components/autocomplete/__tests__/autocomplete.test.tsx (1)
781-781: Consider adding test coverage for transition from aria to native validation.While the test case correctly validates the aria behavior, consider adding test cases that verify the transition from aria to native validation to ensure backward compatibility.
it("should support transition from aria to native validation", async () => { const { rerender, getByRole } = render( <Form validationBehavior="aria"> <AutocompleteExample isRequired /> </Form> ); const input = getByRole("combobox"); expect(input).toHaveAttribute("aria-required"); // Rerender with native validation rerender( <Form validationBehavior="native"> <AutocompleteExample isRequired /> </Form> ); expect(input).toHaveAttribute("required"); expect(input).not.toHaveAttribute("aria-required"); });packages/components/date-picker/__tests__/date-picker.test.tsx (2)
786-786: Consider removing the explicit "native" setting if it is now the default behavior.
If “native” is already the default, specifying “validationBehavior='native'” here might be redundant. Keeping it is harmless but removing it can help reduce clutter.Proposed change (only if you wish to rely exclusively on the new default):
- <Form data-testid="form" validationBehavior="native"> + <Form data-testid="form">
833-834: Double-check necessity of explicit "native" validation in this controlled scenario.
With “native” now the default, explicitly setting “validationBehavior='native'” may not be necessary. Consider removing it for brevity unless you want to emphasize that the form uses native validation explicitly.Proposed change:
- <Form validationBehavior="native" validationErrors={serverErrors} onSubmit={onSubmit}> + <Form validationErrors={serverErrors} onSubmit={onSubmit}>apps/docs/content/components/form/custom-validation-aria.ts (1)
3-6: Optional naming clarification.
While using a constant named “react” is valid, consider renaming it to avoid confusion with the React library.apps/docs/content/components/form/native-validation.raw.jsx (1)
9-9: Consider adding a code comment about native validationSince this example specifically demonstrates native validation behavior (which is now the default), consider adding a comment to explicitly highlight this for users.
+ // This form uses the default native validation behavior <Form className="w-full max-w-xs" onSubmit={onSubmit}>apps/docs/content/components/form/custom-error-messages.raw.jsx (1)
9-9: Consider documenting validation behavior interactionSince this example combines native validation with custom error messages, it would be helpful to document how these interact.
+ // Native validation triggers the custom error message callback <Form className="w-full max-w-xs" onSubmit={onSubmit}>apps/docs/content/components/form/custom-validation-aria.raw.jsx (1)
25-27: Consider adding aria-label to the submit buttonFor better accessibility, consider adding an aria-label to the submit button.
- <Button type="submit" variant="bordered"> + <Button type="submit" variant="bordered" aria-label="Submit form"> Submit </Button>apps/docs/content/components/input/custom-validation.raw.jsx (1)
14-14: LGTM! Consider adding a documentation note about validation behavior.The removal of explicit
validationBehavior="native"aligns with the new default behavior. However, since this is a breaking change, it would be helpful to add a comment explaining that native validation is now the default behavior.Add a comment above the Form component:
+// The Form component uses native validation behavior by default <Form className="w-full max-w-xs" onSubmit={onSubmit}>apps/docs/content/components/input/server-validation.raw.jsx (1)
19-19: Consider documenting validation state precedence.The example correctly demonstrates server validation with the new native validation behavior. However, it would be helpful to document how native client-side validation interacts with server validation errors.
Add a comment explaining the validation flow:
+// Native validation runs first, then server validation errors are displayed +// if the form passes client-side validation <Form className="w-full max-w-xs" validationErrors={errors} onSubmit={onSubmit}>apps/docs/content/components/input/real-time-validation.raw.jsx (1)
Line range hint
26-45: Consider enhancing password validation feedbackThe current implementation shows all errors at once. Consider showing errors progressively as the user types, improving the user experience.
- <Input + <Input + description="Password must contain at least 4 characters, 1 uppercase letter, and 1 symbol" errorMessage={() => ( - <ul> - {errors.map((error, i) => ( - <li key={i}>{error}</li> - ))} - </ul> + errors.length > 0 && errors[0] )} isInvalid={errors.length > 0}packages/components/input/__tests__/input.test.tsx (1)
449-449: Consider adding more ARIA validation test cases.While the ARIA validation tests are good, consider adding test cases for:
- Field focus handling
- Screen reader announcements
- Validation state transitions
Also applies to: 477-477
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
.changeset/flat-ducks-attack.md(1 hunks)apps/docs/content/components/form/controlled.raw.jsx(1 hunks)apps/docs/content/components/form/custom-error-messages.raw.jsx(1 hunks)apps/docs/content/components/form/custom-validation-aria.raw.jsx(1 hunks)apps/docs/content/components/form/custom-validation-aria.ts(1 hunks)apps/docs/content/components/form/custom-validation.raw.jsx(1 hunks)apps/docs/content/components/form/demo.raw.jsx(1 hunks)apps/docs/content/components/form/events.raw.jsx(0 hunks)apps/docs/content/components/form/form-usage.raw.jsx(1 hunks)apps/docs/content/components/form/index.ts(2 hunks)apps/docs/content/components/form/native-validation.raw.jsx(1 hunks)apps/docs/content/components/form/usage.raw.jsx(1 hunks)apps/docs/content/components/input-otp/input-otp-required.raw.jsx(0 hunks)apps/docs/content/components/input-otp/required.raw.jsx(0 hunks)apps/docs/content/components/input/built-in-validation.raw.jsx(1 hunks)apps/docs/content/components/input/custom-validation.raw.jsx(1 hunks)apps/docs/content/components/input/real-time-validation.raw.jsx(1 hunks)apps/docs/content/components/input/server-validation.raw.jsx(1 hunks)apps/docs/content/docs/api-references/nextui-provider.mdx(4 hunks)apps/docs/content/docs/components/form.mdx(3 hunks)apps/docs/content/docs/components/input.mdx(1 hunks)apps/docs/content/docs/guide/forms.mdx(6 hunks)packages/components/autocomplete/__tests__/autocomplete.test.tsx(3 hunks)packages/components/checkbox/__tests__/checkbox-group.test.tsx(1 hunks)packages/components/checkbox/stories/checkbox-group.stories.tsx(1 hunks)packages/components/checkbox/stories/checkbox.stories.tsx(1 hunks)packages/components/date-picker/__tests__/date-picker.test.tsx(5 hunks)packages/components/form/src/form.tsx(1 hunks)packages/components/input-otp/stories/input-otp.stories.tsx(0 hunks)packages/components/input/__tests__/input.test.tsx(5 hunks)packages/components/select/__tests__/select.test.tsx(2 hunks)
💤 Files with no reviewable changes (4)
- apps/docs/content/components/input-otp/input-otp-required.raw.jsx
- apps/docs/content/components/input-otp/required.raw.jsx
- apps/docs/content/components/form/events.raw.jsx
- packages/components/input-otp/stories/input-otp.stories.tsx
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/docs/components/input.mdx
[style] ~130-~130: To form a complete sentence, be sure to include a subject.
Context: ...ook-form.com/). ### With Form Input can be used with a Form component to leve...
(MISSING_IT_THERE)
🔇 Additional comments (34)
packages/components/autocomplete/__tests__/autocomplete.test.tsx (2)
656-657: LGTM! Test case aligns with the new default validation behavior.
The test case correctly validates the native validation behavior with the isRequired prop, which is consistent with the PR's objective to use native validation as the default.
698-699: LGTM! Comprehensive server validation test with native behavior.
The test case properly validates server-side errors with native validation behavior, ensuring proper error handling and accessibility attributes.
packages/components/date-picker/__tests__/date-picker.test.tsx (3)
873-873: ARIA-based validation test confirmed.
This scenario accurately tests “aria” validation, demonstrating a clear contrast with “native” validation. Looks good.
906-906: ARIA validation used correctly.
Continuing to test ARIA-based workflows is consistent and ensures coverage alongside the new default.
933-933: Consistent use of “aria” for server validation scenario.
This case ensures that ARIA-based server validation flow remains functional when “aria” is explicitly specified.
packages/components/checkbox/__tests__/checkbox-group.test.tsx (2)
297-298: Switching to native validation looks consistent.
This aligns with the new default validation behavior. Please ensure that integration points using this test have been updated accordingly.
Line range hint 1202-1207: Retaining ARIA-based test coverage is valuable.
Having a test scenario that exercises ARIA validation ensures comprehensive validation coverage, demonstrating correct fallback or alternate usage.
packages/components/select/__tests__/select.test.tsx (2)
1202-1207: Good demonstration of ARIA validation usage in the test.
Continuing to test ARIA-specific behavior ensures we have a robust suite for different validation modes.
1257-1257: Validate that default selection doesn't conflict with custom validation behavior.
The inline validate function flags "penguin" as invalid, which might initially cause confusion if someone is expecting "native" validation to handle it. This is likely intentional for demonstration, but please confirm clarity in documentation or inline comments.
apps/docs/content/components/form/custom-validation-aria.ts (2)
1-2: Import usage checks out.
Successfully importing the raw JSX content will help highlight ARIA-based validation in docs or demos.
7-9: Good practice for modular exports.
Spreading the object keeps the file flexible and easy to maintain moving forward.
.changeset/flat-ducks-attack.md (2)
1-9: Ensure all version increments are correct.
Check that these minor and patch bumps align with each component’s release plan and that dependent packages won’t break on these version increments.
11-11: Accurate reflection of the PR.
Changing the default validation from ARIA to native is consistent with the objectives of this release.
packages/components/form/src/form.tsx (1)
11-11: LGTM! Default validation behavior correctly updated to "native"
The implementation maintains proper precedence (props -> globalContext -> default) while changing the default behavior as intended.
Let's verify this is the only instance setting the default validation behavior:
✅ Verification successful
Default validation behavior is consistently set across components
The verification shows that the default validation behavior is correctly set to "native" across all form-related components:
form/src/form.tsx: Default is "native"radio/src/use-radio-group.ts: Default is "native"select/src/use-select.ts: Default is "native"date-input/src/use-date-input.ts: Default is "native"input/src/use-input.ts: Default is "native"checkbox/src/use-checkbox-group.ts: Default is "native"autocomplete/src/use-autocomplete.ts: Default is "native"
All components follow the same precedence pattern: props -> formValidationBehavior -> globalContext -> "native"
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other instances setting validation behavior defaults
rg -g '!*.test.*' -g '!*.spec.*' "validationBehavior.*=.*[\"'](?:native|aria)[\"']"
Length of output: 4524
apps/docs/content/components/form/index.ts (1)
7-7: LGTM! Addition of ARIA validation example
The addition of customValidationAria example is appropriate as it provides guidance for users who need to explicitly opt into ARIA validation behavior after this breaking change.
Also applies to: 19-19
apps/docs/content/components/form/custom-validation.raw.jsx (1)
9-9: LGTM! Removal of explicit native validation
The removal of validationBehavior="native" is appropriate as it will become the default behavior. The code is now more concise while maintaining the same functionality.
apps/docs/content/components/form/custom-validation-aria.raw.jsx (1)
9-28: LGTM! Well-structured ARIA validation example
This example clearly demonstrates how to opt into ARIA validation behavior after the breaking change, with proper form setup and custom validation logic.
apps/docs/content/components/form/form-usage.raw.jsx (1)
15-15: LGTM! Form implementation aligns with new native validation behavior.
The removal of explicit validationBehavior prop correctly implements the new default native validation behavior.
apps/docs/content/components/form/usage.raw.jsx (1)
15-15: LGTM! Consistent implementation across examples.
The form implementation maintains consistency with other examples in adopting the new native validation behavior.
apps/docs/content/components/input/built-in-validation.raw.jsx (1)
14-14: Verify email validation behavior with native validation.
The removal of explicit validationBehavior prop is correct as it now defaults to native. However, we should verify that the email validation and error messages work as expected with native validation.
✅ Verification successful
Email validation implementation is correctly configured
The implementation shows proper email validation setup:
- Uses HTML5 native validation through
type="email" - Has required field validation with
isRequired - Implements custom error handling for type mismatch
- Correctly omits explicit
validationBehaviorprop to use the new default
The code demonstrates the intended behavior for email validation with appropriate error messaging and required field validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any test files that verify email validation behavior
fd -e test.tsx -e test.jsx -e spec.tsx -e spec.jsx | xargs rg -l "email.*validation|validation.*email"
# Search for related validation behavior tests
ast-grep --pattern 'test($_, $_) {
$$$
email
$$$
}'
Length of output: 164
Script:
#!/bin/bash
# Let's check the actual implementation of the form validation
rg -A 10 "onSubmit.*=.*{.*onSubmit" apps/docs/content/components/input/built-in-validation.raw.jsx
# Search for email input field configuration
rg "type.*=.*[\"']email[\"']" apps/docs/content/components/input/built-in-validation.raw.jsx
# Check for any validation-related props or configurations
rg "validation|required|pattern" apps/docs/content/components/input/built-in-validation.raw.jsx
Length of output: 589
Script:
#!/bin/bash
# Let's check the complete implementation of the form
rg -B 5 -A 15 "<Form" apps/docs/content/components/input/built-in-validation.raw.jsx
# Check if there are any other validation behavior configurations in the file
rg -l "validationBehavior" apps/docs/content/components/input/built-in-validation.raw.jsx
# Look for any comments or documentation about validation
rg -B 2 -A 2 "validation" apps/docs/content/components/input/built-in-validation.raw.jsx
Length of output: 1059
apps/docs/content/components/input/real-time-validation.raw.jsx (1)
26-26: LGTM: Form component changes align with new default behavior
The removal of explicit validationBehavior prop aligns with the PR objective to use "native" as default.
apps/docs/content/components/form/demo.raw.jsx (1)
61-64: LGTM: Form validation error handling is well-implemented
The use of validationErrors prop for server-side validation is correct and follows best practices.
apps/docs/content/docs/components/form.mdx (2)
75-76: LGTM: Documentation clearly explains the new default behavior
The documentation accurately reflects that native validation is now the default behavior and explains how to opt into ARIA validation.
127-127: LGTM: API documentation correctly shows new default value
The API documentation accurately reflects that "native" is now the default value for validationBehavior.
apps/docs/content/docs/api-references/nextui-provider.mdx (3)
70-74: LGTM! Improved locale list formatting
The reformatting of the locale list enhances readability while maintaining all supported locales.
Line range hint 183-189: LGTM! Enhanced reducedMotion prop documentation
The documentation now clearly explains all available options and their implications for motion preferences.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~186-~186: Loose punctuation mark.
Context: ...tings for reduced motion. - "always": Disables all animations. - "never":...
(UNLIKELY_OPENING_PUNCTUATION)
176-179: Breaking Change: Default validation behavior updated
The default validation behavior has been changed from "aria" to "native". This is a breaking change that might affect existing implementations.
Let's verify the impact:
packages/components/checkbox/stories/checkbox.stories.tsx (1)
183-183: LGTM! Removed redundant validationBehavior prop
The removal of explicit validationBehavior="native" aligns with the new default behavior.
packages/components/checkbox/stories/checkbox-group.stories.tsx (1)
Line range hint 156-160: LGTM! Simplified CheckboxGroup validation
The removal of explicit validation behavior prop aligns with the new default behavior while maintaining server-side validation functionality.
apps/docs/content/docs/components/input.mdx (1)
130-130: LGTM! Documentation update aligns with the new validation behavior.
The documentation now correctly describes Form component integration without tying it to a specific validation behavior, making it more maintainable.
🧰 Tools
🪛 LanguageTool
[style] ~130-~130: To form a complete sentence, be sure to include a subject.
Context: ...ook-form.com/). ### With Form Input can be used with a Form component to leve...
(MISSING_IT_THERE)
packages/components/input/__tests__/input.test.tsx (3)
313-314: LGTM! Comprehensive test coverage for native validation.
The test correctly verifies the native validation behavior with the Form component.
347-348: LGTM! Proper test coverage for custom validation function.
The test effectively verifies the validate function behavior with native validation.
393-399: LGTM! Well-structured server validation test.
The test comprehensively covers server validation scenarios with the native validation behavior.
apps/docs/content/docs/guide/forms.mdx (1)
228-229: LGTM! Clear explanation of ARIA validation behavior.
The documentation clearly explains when and why to use ARIA validation, helping developers make informed decisions.
Closes eng-1742
📝 Description
validationBehavior="aria"as default behaviorvalidationBehavior="native"to be consistent with RACvalidationBehavior="native"from docs⛳️ Current behavior (updates)
validationBehavior="aria"as default behavior🚀 New behavior
validationBehavior="native"as default behavior💣 Is this a breaking change (Yes/No):
Yes
validationBehavior="aria"will need to update their usagevalidationBehavior="native"📝 Additional Information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
NextUIProviderandFormcomponent to reflect changes in validation behavior.Tests