feat(v3): CheckboxGroup, FieldCheckboxGroup #602
Conversation
ntatoud
added
enhancement
WalkthroughNew CheckboxGroup and FieldCheckboxGroup components are introduced, along with Storybook stories demonstrating their usage, including nested and form-integrated scenarios. The form controller is updated to support the new checkbox group type. Accessibility and validation handling are improved for both checkbox and radio group components, with enhanced UI states and error styling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Storybook
participant FieldCheckboxGroup
participant ReactHookForm
participant Zod
participant CheckboxGroup
participant Checkbox
User->>Storybook: Interacts with FieldCheckboxGroup story
Storybook->>FieldCheckboxGroup: Renders form
FieldCheckboxGroup->>ReactHookForm: Registers field with Zod validation
ReactHookForm->>Zod: Validates input on blur/submit
FieldCheckboxGroup->>CheckboxGroup: Passes value, onValueChange, accessibility props
CheckboxGroup->>Checkbox: Renders options as checkboxes
User->>Checkbox: Selects/deselects options
Checkbox->>CheckboxGroup: Triggers onValueChange
CheckboxGroup->>FieldCheckboxGroup: Updates form state
FieldCheckboxGroup->>ReactHookForm: Updates controlled value
ReactHookForm->>Zod: Re-validates if needed
sequenceDiagram
participant User
participant CheckboxGroup
participant Checkbox (Parent)
participant Checkbox (Child)
User->>Checkbox (Parent): Toggles parent checkbox
Checkbox (Parent)->>CheckboxGroup: Selects/deselects all children
CheckboxGroup->>Checkbox (Child): Updates checked state
User->>Checkbox (Child): Toggles child checkbox
Checkbox (Child)->>CheckboxGroup: Updates value array
CheckboxGroup->>Checkbox (Parent): Updates indeterminate/checked state
Suggested reviewers
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
ntatoud
force-pushed
the
v3/feat-field-checkbox-group
branch
from
186d08f to
68f9828
Compare
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
app/components/ui/checkbox.tsx (1)
44-87: Consider adding aria-invalid support to parent state for consistency.The parent state implementation is comprehensive but lacks
aria-invalidstyling that's present in the default state. This might be needed for form validation consistency.Consider adding aria-invalid styling to the parent state className:
className={cn( 'flex size-5 cursor-pointer items-center justify-center rounded-sm outline-none', 'focus-visible:ring-[3px] focus-visible:ring-ring/50', + 'aria-invalid:focus-visible:ring-destructive/50 aria-invalid:data-unchecked:border-destructive', 'data-checked:bg-primary data-unchecked:border data-unchecked:border-primary/50', 'disabled:cursor-not-allowed disabled:bg-muted-foreground disabled:opacity-20', className )}app/components/form/field-checkbox-group/index.tsx (1)
58-61: Consider removing empty classNameThe container div has an empty className string which serves no purpose.
- className={cn('', containerProps?.className)} + className={cn(containerProps?.className)}app/components/ui/checkbox-group.stories.tsx (1)
101-116: Review the parent-child synchronization logicThe logic for synchronizing parent and child checkbox states could be more robust. Currently, when "bear-3" is selected, it automatically selects all little bears, but the reverse logic might not work as expected in all scenarios.
Consider extracting this logic into a custom hook for better testability and reusability:
const useNestedCheckboxSync = (parentValue: string, childValues: string[]) => { // Implementation for parent-child synchronization };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/components/form/field-checkbox-group/docs.stories.tsx(1 hunks)app/components/form/field-checkbox-group/index.tsx(1 hunks)app/components/form/field-radio-group/index.tsx(4 hunks)app/components/form/form-field-controller.tsx(3 hunks)app/components/ui/checkbox-group.stories.tsx(1 hunks)app/components/ui/checkbox-group.tsx(1 hunks)app/components/ui/checkbox.tsx(3 hunks)app/components/ui/radio-group.tsx(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
app/components/form/field-radio-group/index.tsx (1)
app/components/ui/radio-group.tsx (1)
Radio(26-67)
app/components/ui/checkbox-group.tsx (1)
app/lib/tailwind/utils.ts (1)
cn(4-6)
app/components/ui/checkbox.tsx (1)
app/lib/tailwind/utils.ts (1)
cn(4-6)
app/components/form/field-checkbox-group/index.tsx (5)
app/components/ui/checkbox.tsx (2)
CheckboxProps(8-14)Checkbox(16-117)app/components/form/form-field-controller.tsx (1)
FieldProps(40-47)app/components/ui/checkbox-group.tsx (1)
CheckboxGroup(7-17)app/components/form/form-field.tsx (1)
useFormField(50-56)app/lib/tailwind/utils.ts (1)
cn(4-6)
app/components/ui/checkbox-group.stories.tsx (2)
app/components/ui/checkbox-group.tsx (1)
CheckboxGroup(7-17)app/components/ui/checkbox.tsx (1)
Checkbox(16-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (19)
app/components/form/form-field-controller.tsx (3)
13-16: Import statement follows established patterns.The import is correctly placed and follows the same structure as other field component imports.
61-62: Union type extension is correctly implemented.The FieldCheckboxGroupProps is properly added to the union type, maintaining consistency with the existing field types.
105-106: Switch case implementation follows established pattern.The new case for 'checkbox-group' is correctly implemented and positioned appropriately before the generic comment marker.
app/components/form/field-radio-group/index.tsx (2)
56-56: Good refactoring to centralize invalid state logic.Creating a local
isInvalidconstant eliminates repetition and improves maintainability. The boolean conversion is correct for thearia-invalidattribute.
67-67: Consistent application of invalid state across components.The
isInvalidconstant is properly applied to the RadioGroup, renderOption, and Radio components, ensuring consistent accessibility semantics.Also applies to: 86-86, 95-102
app/components/ui/radio-group.tsx (1)
48-48: Enhanced styling for invalid states improves user experience.The added Tailwind classes provide appropriate visual feedback for invalid radio buttons, complementing the
aria-invalidattribute usage in the form components.app/components/ui/checkbox.tsx (2)
36-40: Good use of UI state management for different checkbox variants.The
getUiStateusage effectively handles the different UI states based on theparentprop, providing a clean separation of concerns.
88-112: Default state maintains existing functionality with enhanced validation styling.The default state implementation preserves the original checkbox behavior while adding appropriate
aria-invalidstyling for form validation.app/components/form/field-checkbox-group/docs.stories.tsx (4)
1-18: LGTM: Clean imports and story setupThe imports are well-organized and the Storybook meta configuration is properly set up for the FieldCheckboxGroup component.
20-26: LGTM: Proper form validation schemaThe Zod schema correctly validates for a non-empty array of strings with a clear error message. The use of
zu.array.nonEmptyis appropriate for checkbox group validation.
28-34: LGTM: Appropriate form configurationThe form options are properly configured with
onBlurmode for validation timing and correct default values for the checkbox group.
42-64: LGTM: Well-structured form storyThe story demonstrates proper integration of the FieldCheckboxGroup within a form context, including proper form field structure with label, helper text, and submission handling.
app/components/form/field-checkbox-group/index.tsx (2)
62-85: LGTM: Excellent accessibility implementationThe CheckboxGroup implementation properly handles all accessibility concerns:
- Correct ARIA attributes linking to form field context
- Proper error state handling in aria-describedby
- Individual checkbox accessibility attributes
- Clean props spreading and value management
49-91: LGTM: Solid Controller integrationThe react-hook-form Controller is properly integrated with all necessary props and correct render prop usage. The field state management and error handling are well implemented.
app/components/ui/checkbox-group.stories.tsx (5)
7-16: LGTM: Clean story setupThe Storybook meta configuration and test data structure are well-organized. The astrobears data includes all necessary properties for demonstrating different checkbox states.
18-28: LGTM: Simple default storyThe default story properly demonstrates basic CheckboxGroup usage with mapped options.
30-68: LGTM: Good coverage of checkbox statesThe DefaultValue, Disabled, and DisabledOption stories provide comprehensive coverage of different checkbox group states and behaviors.
137-145: LGTM: Solid child-to-parent synchronizationThe logic for updating the parent when all children are selected/deselected is well-implemented. The use of
Array.from(new Set([...prev, 'bear-3']))ensures no duplicates are added.
103-168: LGTM: Comprehensive nested group demonstrationThe WithNestedGroups story effectively demonstrates the parent checkbox functionality and complex state management for hierarchical checkbox groups. The visual indentation with
pl-4classes makes the hierarchy clear.
| className={cn( | ||
| 'className="flex flex-col items-start gap-1', | ||
| props?.className | ||
| )} |
There was a problem hiding this comment.
Fix syntax error in className string.
There's a syntax error in the className string on line 12 where "className= is included in the string literal.
Apply this diff to fix the syntax error:
- className={cn(
- 'className="flex flex-col items-start gap-1',
- props?.className
- )}
+ className={cn(
+ 'flex flex-col items-start gap-1',
+ props?.className
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| className={cn( | |
| 'className="flex flex-col items-start gap-1', | |
| props?.className | |
| )} | |
| className={cn( | |
| 'flex flex-col items-start gap-1', | |
| props?.className | |
| )} |
🤖 Prompt for AI Agents
In app/components/ui/checkbox-group.tsx around lines 11 to 14, remove the
incorrect inclusion of 'className=' inside the string literal passed to the cn
function. The className string should only contain the CSS classes without the
'className=' prefix. Fix the syntax by deleting 'className=' from the string.
| @@ -17,6 +18,7 @@ export function Checkbox({ | |||
| className, | |||
| noLabel, | |||
| labelProps, | |||
| parent, | |||
There was a problem hiding this comment.
Missing parent prop in CheckboxProps type definition.
The parent prop is used in the component but not defined in the CheckboxProps type interface.
Add the parent prop to the CheckboxProps type:
export type CheckboxProps = Omit<CheckboxPrimitive.Root.Props, 'type'> & {
/**
* By default, the checkbox is wrapped in a `<label>`. Set to `false` if you do not want it.
*/
noLabel?: boolean;
labelProps?: React.ComponentProps<'label'>;
+ /**
+ * Whether this checkbox represents a parent/group checkbox with indeterminate state support.
+ */
+ parent?: boolean;
};🤖 Prompt for AI Agents
In app/components/ui/checkbox.tsx at line 21, the `parent` prop is used in the
Checkbox component but is missing from the `CheckboxProps` type definition. To
fix this, add a `parent` property with the appropriate type to the
`CheckboxProps` interface to ensure type safety and proper prop validation.
| TFIeldValues extends FieldValues = FieldValues, | ||
| TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>, | ||
| >( | ||
| props: FieldCheckboxGroupProps<TFIeldValues, TName> |
There was a problem hiding this comment.
Fix typo in props type
The props type reference also needs to be corrected.
- props: FieldCheckboxGroupProps<TFIeldValues, TName>
+ props: FieldCheckboxGroupProps<TFieldValues, TName>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| props: FieldCheckboxGroupProps<TFIeldValues, TName> | |
| props: FieldCheckboxGroupProps<TFieldValues, TName> |
🤖 Prompt for AI Agents
In app/components/form/field-checkbox-group/index.tsx at line 32, there is a
typo in the props type name "TFIeldValues" which should be corrected to
"TFieldValues". Update the type reference to fix the capitalization error to
ensure proper typing.
| export type FieldCheckboxGroupProps< | ||
| TFIeldValues extends FieldValues = FieldValues, | ||
| TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>, |
There was a problem hiding this comment.
Fix typo in generic type parameter
There's a typo in the generic type parameter name: TFIeldValues should be TFieldValues.
-export type FieldCheckboxGroupProps<
- TFIeldValues extends FieldValues = FieldValues,
- TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>,
+export type FieldCheckboxGroupProps<
+ TFieldValues extends FieldValues = FieldValues,
+ TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type FieldCheckboxGroupProps< | |
| TFIeldValues extends FieldValues = FieldValues, | |
| TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>, | |
| export type FieldCheckboxGroupProps< | |
| TFieldValues extends FieldValues = FieldValues, | |
| TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>, |
🤖 Prompt for AI Agents
In app/components/form/field-checkbox-group/index.tsx around lines 15 to 17, fix
the typo in the generic type parameter by renaming `TFIeldValues` to
`TFieldValues` to ensure consistent and correct naming.
| export const FieldCheckboxGroup = < | ||
| TFIeldValues extends FieldValues = FieldValues, | ||
| TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>, | ||
| >( |
There was a problem hiding this comment.
Fix typo in generic type parameter
The same typo exists in the component function signature.
-export const FieldCheckboxGroup = <
- TFIeldValues extends FieldValues = FieldValues,
- TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>,
+export const FieldCheckboxGroup = <
+ TFieldValues extends FieldValues = FieldValues,
+ TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const FieldCheckboxGroup = < | |
| TFIeldValues extends FieldValues = FieldValues, | |
| TName extends FieldPath<TFIeldValues> = FieldPath<TFIeldValues>, | |
| >( | |
| export const FieldCheckboxGroup = < | |
| TFieldValues extends FieldValues = FieldValues, | |
| TName extends FieldPath<TFieldValues> = FieldPath<TFieldValues>, | |
| >( |
🤖 Prompt for AI Agents
In app/components/form/field-checkbox-group/index.tsx around lines 28 to 31,
there is a typo in the generic type parameter name "TFIeldValues" which should
be corrected to "TFieldValues" consistently in the component function signature
to fix the typo.
ivan-dalmet
force-pushed
the
v3-main
branch
2 times, most recently
from
cf02ee7 to
cbaf95e
Compare
Summary by CodeRabbit
New Features
Documentation
Style
Refactor