Fix: adjust spacing in RadioGroupItemField label for better a11y#136
Fix: adjust spacing in RadioGroupItemField label for better a11y#136
Conversation
|
|
WalkthroughRemoved the wrapper horizontal gap between the radio input and label in RadioGroupItemField and moved the spacing to the label via left padding; bumped package version from 0.19.5 to 0.19.6. No behavioral/API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed Checks (1 warning)
✅ Passed Checks (2 passed)
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
⏰ 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)
Comment |
|
📝 Storybook Preview: View Storybook This preview will be updated automatically when you push new changes to this PR.
|
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/src/ui/radio-group-item-field.tsx (2)
36-39: Enable label’s peer-disabled styles by adding peer to the item.Without peer on the radio item, the label’s peer-disabled variants never activate.
Apply:
- className={cn( - 'h-4 w-4 rounded-full border border-primary text-primary ring-offset-background focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50', + className={cn( + 'peer h-4 w-4 rounded-full border border-primary text-primary ring-offset-background focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50', className, )}
64-65: Export the props type to satisfy package guidelines.Matches “Always export both the component and props type.”
export { RadioGroupItemField }; +export type { RadioGroupItemFieldProps };
🧹 Nitpick comments (2)
packages/components/src/ui/radio-group-item-field.tsx (2)
50-51: Minor UX: make the label clearly interactive (and RTL-friendly if applicable).Add cursor-pointer and select-none; optionally swap pl-2 for directional variants if you support RTL.
- 'text-sm pl-2 font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70', + 'text-sm pl-2 cursor-pointer select-none font-medium leading-none peer-disabled:cursor-not-allowed peer-disabled:opacity-70',If RTL is supported, consider:
- 'text-sm pl-2 ... + 'text-sm ltr:pl-2 rtl:pr-2 ...
2-2: Ensure label association even when id isn’t provided (fallback to useId).Prevents accidental unlabeled items when consumers omit id.
-import type * as React from 'react'; +import type * as React from 'react'; +import { useId } from 'react'; @@ -const RadioGroupItemField = ({ - label, - labelClassName, - wrapperClassName, - components, - className, - children, - ...props -}: RadioGroupItemFieldProps) => { +const RadioGroupItemField = ({ + label, + labelClassName, + wrapperClassName, + components, + className, + children, + ...props +}: RadioGroupItemFieldProps) => { + const generatedId = useId(); + const { id: idProp, ...rest } = props as { id?: string }; + const id = idProp ?? generatedId; @@ - {...props} + id={id} + {...rest} @@ - htmlFor={props.id} + htmlFor={id}Also applies to: 19-27, 40-41, 48-49
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/components/src/ui/radio-group-item-field.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
packages/components/src/ui/**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
packages/components/src/ui/**/*.tsx: Build on @radix-ui components as the foundation for base UI components
Follow the component composition pattern for UI components that accept form integration
Files:
packages/components/src/ui/radio-group-item-field.tsx
packages/components/src/ui/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Base UI components should be named as ComponentName in ui/ directory
Files:
packages/components/src/ui/radio-group-item-field.tsx
**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
**/*.{tsx,ts}: Props interfaces should be named as ComponentNameProps
Form schemas should be named formSchema or componentNameSchema
Files:
packages/components/src/ui/radio-group-item-field.tsx
packages/components/src/{remix-hook-form,ui}/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/form-component-patterns.mdc)
Always export both the component and its props type
Files:
packages/components/src/ui/radio-group-item-field.tsx
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/*.{ts,tsx}: Use package name imports for published packages (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form')
Import from specific entry points (e.g., import { TextField } from '@lambdacurry/forms/remix-hook-form/text-field')
Do not use relative imports across packages (e.g., avoid import { TextField } from '../../packages/components/src/remix-hook-form/text-field')
Order imports: 1) external libraries, 2) internal package imports, 3) cross-package imports, 4) type-only imports (grouped separately)
Files:
packages/components/src/ui/radio-group-item-field.tsx
{apps,packages}/**/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{ts,tsx}: Use relative imports within the same package (e.g., import { FormControl } from './form')
Use relative imports for sibling directories (e.g., import { Button } from '../ui/button')
Files:
packages/components/src/ui/radio-group-item-field.tsx
packages/components/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
packages/components/src/**/*.{ts,tsx}: Always export both component and props type (e.g., export { ComponentName }; export type { ComponentNameProps };)
Use named exports for components for better tree-shaking (e.g., export const ComponentName = ...; avoid default exports)
Avoid default exports for components
Use tree-shaking friendly exports
Files:
packages/components/src/ui/radio-group-item-field.tsx
{apps,packages}/**/src/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
{apps,packages}/**/src/**/*.{tsx,ts}: Use kebab-case for component file names (e.g., text-field.tsx, data-table.tsx)
Match component name in PascalCase to the exported component from the file (e.g., text-field.tsx exports TextField)
Files:
packages/components/src/ui/radio-group-item-field.tsx
packages/components/src/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Don't import from remix-hook-form package in ui components
Files:
packages/components/src/ui/radio-group-item-field.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo-organization.mdc)
Avoid circular dependencies
Files:
packages/components/src/ui/radio-group-item-field.tsx
🧬 Code graph analysis (1)
packages/components/src/ui/radio-group-item-field.tsx (1)
packages/components/src/ui/utils/index.ts (1)
cn(4-6)
⏰ 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: test
🔇 Additional comments (1)
packages/components/src/ui/radio-group-item-field.tsx (1)
34-34: A11y: Good fix—removing wrapper spacing eliminates the click dead zone.Shifting spacing responsibility to the label is the right approach for a larger hit target.
|
@codegen-sh go into the library and use npm to bump the patch version with a summary message, don't publish just bump the version, and push up the change.
|
- Fix: adjust spacing in RadioGroupItemField label for better a11y - Patch version bump for accessibility improvements Co-authored-by: lcmohsen <mohsen@lambdacurry.dev>
|
✅ Version bump completed successfully! I've bumped the What was done:
Next steps:
The version bump is now live on the remote branch! 🎉 💻 View my work • 🛑 Stop • React 👍 or 👎 |
2 participants
As you can see in the image, adding margin to create space between the element and its label causes an issue: the user can’t click the label when the cursor is in that small gap between the two elements.
Summary by CodeRabbit
Style
Chores