refactor(component: switch): make Switch component self-dependent by removing @radix-ui/react-switch dependency #82
Conversation
…removing @radix-ui/react-switch dependency Fixes #63 Refactor the `Switch` component to be self-contained and remove dependency on `@radix-ui/react-switch`. * **Switch Component**: - Remove import statement for `@radix-ui/react-switch`. - Update the `Switch` component to be self-contained with its own state management and event handling. - Update the `Switch` component's props to include `checked` and `onCheckedChange`. * **Documentation**: - Update the documentation in `apps/www/content/docs/components/switch.mdx` to reference the self-contained `Switch` component. - Add a new props table to the documentation to reflect the updated `Switch` component's props. * **Dependencies**: - Remove `@radix-ui/react-switch` from `packages/ui/package.json`. - Update `pnpm-lock.yaml` to remove references to `@radix-ui/react-switch`. * **Registry**: - Update `apps/www/public/registry/index.json` and `apps/www/public/registry/components/switch.json` to remove the dependency on `@radix-ui/react-switch`. - Update `apps/www/registry/ui.ts` to remove the dependency on `@radix-ui/react-switch`. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/ruru-m07/ruru-ui/issues/63?shareId=XXXX-XXXX-XXXX-XXXX).
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe changes involve a comprehensive refactor of the Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🔇 Additional comments (2)
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
apps/www/public/registry/components/switch.json (1)
6-6: Props and state management look good, but consider a minor improvementThe implementation of the Switch component as a custom button with its own state management is well done. The use of
React.forwardRefand the customSwitchPropsinterface provides good type safety and flexibility.However, there's a small improvement we can make to better sync the internal state with the external
checkedprop:Consider updating the
isCheckedstate when thecheckedprop changes. You can do this by adding auseEffecthook:React.useEffect(() => { if (checked !== undefined) { setIsChecked(checked); } }, [checked]);This ensures that the component's state always reflects the
checkedprop, even if it changes externally.apps/www/content/docs/components/switch.mdx (1)
172-180: LGTM: Comprehensive Props section addedThe new Props section is a valuable addition to the documentation. It provides clear and detailed information about each prop, including types, default values, and descriptions. This addition greatly enhances the usability of the component for developers.
One minor suggestion for consistency:
Consider updating the default value for
classNamefrom""(with a space) to""(without a space) to match the format of other default values in the table.packages/ui/src/components/switch.tsx (2)
4-7: UseButtonHTMLAttributesfor more comprehensive propsExtending
React.ButtonHTMLAttributes<HTMLButtonElement>ensures all standard button attributes are included, providing better type safety and flexibility.Apply this diff to update the interface:
-interface SwitchProps extends React.ComponentPropsWithoutRef<"button"> { +interface SwitchProps extends React.ButtonHTMLAttributes<HTMLButtonElement> { checked?: boolean; onCheckedChange?: (checked: boolean) => void; }
13-15: Revise JSDoc comments for accuracy and completenessThe
@paramtags should accurately reflect the component's props. Also, sincerefis handled byforwardRef, it doesn't need to be documented as a prop.Consider updating the JSDoc comments:
/** * Switch component * * @param {string} className - Additional class names for the switch. * @param {boolean} checked - Whether the switch is checked. * @param {(checked: boolean) => void} onCheckedChange - Callback when the switch is checked or unchecked. - * @param {React.Ref<HTMLButtonElement>} ref - Forwarded ref. * * @example * * ```tsx * <Switch /> * ``` */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
- apps/www/content/docs/components/switch.mdx (5 hunks)
- apps/www/public/registry/components/switch.json (1 hunks)
- apps/www/public/registry/index.json (0 hunks)
- apps/www/registry/ui.ts (0 hunks)
- packages/ui/package.json (0 hunks)
- packages/ui/src/components/switch.tsx (1 hunks)
💤 Files with no reviewable changes (3)
- apps/www/public/registry/index.json
- apps/www/registry/ui.ts
- packages/ui/package.json
🧰 Additional context used
🔇 Additional comments (10)
apps/www/public/registry/components/switch.json (4)
6-6: Click handler implementation is correctThe
handleClickfunction correctly toggles the internal state and calls theonCheckedChangecallback when provided. This implementation allows the component to be used in both controlled and uncontrolled modes.
6-6: Rendering and accessibility attributes are well implementedThe use of appropriate ARIA attributes (
role="switch"andaria-checked) enhances the accessibility of the component. The button type is correctly set to "button" to prevent form submission.
6-6: Styling implementation is flexible and maintainableThe use of the
cnutility for class name composition and the conditional application of classes based on theisCheckedstate is a good approach. It allows for easy customization and maintains a clear separation between logic and styling.
Line range hint
1-53: Overall implementation aligns well with PR objectivesThe refactoring of the Switch component to remove the dependency on @radix-ui/react-switch has been successfully implemented. The component is now self-dependent, using native HTML elements and React hooks for its functionality. This aligns perfectly with the objectives outlined in the PR and the linked issue #63.
Key achievements:
- Removal of external dependencies
- Custom implementation using a button element
- Proper state management
- Accessibility considerations
- Flexible styling
The JSON structure of the file is correct, and the absence of listed dependencies in the file accurately reflects the component's new self-dependent nature.
apps/www/content/docs/components/switch.mdx (5)
58-58: LGTM: Import statement updated correctlyThe import statement has been successfully updated to use the new path
"ruru-ui/components/switch". This change is consistent with the PR objective of making the Switch component self-dependent and removing the external dependency.
78-78: LGTM: Disabled switch example updated correctlyThe import statement in the disabled switch example has been correctly updated to use the new path. The usage of the
disabledprop remains unchanged, which is appropriate.
102-102: LGTM: Switch with labels example updated and improvedThe import statement has been correctly updated. Additionally, the inclusion of the
idprop for the Switch component is a great improvement for accessibility. This change ensures that the label is properly associated with the switch, enhancing usability for all users, including those using assistive technologies.
127-127: LGTM: Default checked switch example updated correctlyThe import statement in the default checked switch example has been properly updated to use the new path. The usage of the
defaultCheckedprop remains unchanged, which is correct and consistent with the component's functionality.
Line range hint
1-180: Overall documentation update is comprehensive and aligned with PR objectivesThe changes made to this documentation file are thorough and consistent with the PR's objective of making the Switch component self-dependent. Key improvements include:
- Updated import statements across all examples.
- Addition of the
idprop in examples, enhancing accessibility.- A new, detailed Props section that accurately reflects the component's current implementation.
These updates ensure that the documentation is in sync with the refactored Switch component, providing developers with accurate and comprehensive information for using the component.
packages/ui/src/components/switch.tsx (1)
37-55: Component rendering follows best practicesThe rendering logic is clear, and accessibility attributes like
role="switch"andaria-checkedare properly used. Conditional class names effectively manage the visual state.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (8)
apps/www/components/SwitchEvent.tsx (1)
11-12: Approved: Improved accessibility and HTML complianceThe changes to the
idattribute of the Switch component and the correspondinghtmlForattribute of the label improve accessibility and comply with HTML best practices. The newid"toggle-value" is more appropriate for an HTML id attribute.Consider updating the label text to be more descriptive:
- <label htmlFor="toggle-value"> toggle value </label> + <label htmlFor="toggle-value">Toggle switch state</label>This change would make the purpose of the switch clearer to users.
apps/www/public/registry/components/switch.json (2)
Line range hint
18-33: LGTM: State management and event handling are well-implemented.The component correctly handles both controlled and uncontrolled states, and the
handleClickfunction properly manages state updates and callbacks. This implementation aligns well with React best practices.Consider using the functional update form of
setIsCheckedInternalfor better consistency with React's state update batching:- setIsCheckedInternal(newChecked); + setIsCheckedInternal(prev => !prev);
Line range hint
35-53: LGTM: Rendering and styling are well-implemented with good accessibility considerations.The use of a native
<button>element with appropriate ARIA attributes enhances accessibility. The styling approach using Tailwind CSS classes is consistent with the project's standards.Consider adding a visually hidden text description of the switch state for screen readers:
<button type="button" role="switch" aria-checked={isCheckedValue} className={cn( "peer inline-flex h-5 w-9 shrink-0 cursor-pointer items-center rounded-full border-2 border-transparent shadow-sm transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background disabled:cursor-not-allowed disabled:opacity-50", isCheckedValue ? "bg-primary" : "bg-accent", className, )} onClick={handleClick} ref={ref} {...props} > + <span className="sr-only">{isCheckedValue ? 'On' : 'Off'}</span> <span className={cn( "pointer-events-none block h-4 w-4 rounded-full bg-background shadow-lg ring-0 transition-transform", isCheckedValue ? "translate-x-4" : "translate-x-0", )} /> </button>This addition would improve the component's accessibility by providing a clear text description of the switch state for screen reader users.
packages/ui/src/components/switch.tsx (2)
12-26: LGTM: Component documentation is comprehensiveThe JSDoc style comments provide clear and comprehensive documentation for the Switch component. All important props are well-described, and a simple usage example is included.
Consider adding a brief description of the component's purpose at the beginning of the documentation block. For example:
/** * Switch component - A toggleable on/off state input * ... */
38-71: LGTM: Well-implemented event handling and renderingThe handleClick function and rendering logic are well-implemented. The component correctly uses ARIA attributes for accessibility, and the styling changes appropriately based on the checked state.
Consider extracting the long className string into a separate constant for improved readability. For example:
const switchClassName = "peer inline-flex h-5 w-9 shrink-0 cursor-pointer items-center rounded-full border-2 border-transparent shadow-sm transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 focus-visible:ring-offset-background disabled:cursor-not-allowed disabled:opacity-50"; // Then in the JSX: className={cn(switchClassName, isCheckedValue ? "bg-primary" : "bg-accent", className)}apps/www/content/docs/components/switch.mdx (3)
58-61: LGTM: Updated import and usage exampleThe import statement and usage example have been correctly updated to reflect the new package structure. This change aligns with the refactoring of the Switch component to be self-dependent.
Consider adding a comment above the
Demofunction to explain its purpose, e.g.:// Basic usage of the Switch component export function Demo() { return <Switch />; }
95-96: LGTM: Updated Switch with labels exampleThe example has been correctly updated to include the
idprop for the Switch component and the correspondinghtmlForattribute for the label. This change improves accessibility and demonstrates proper usage of the component with associated labels.Consider updating the label text to be more descriptive, e.g.:
<label htmlFor="airplane-mode-demo">Airplane Mode</label>This would make the example more realistic and informative.
172-181: Excellent addition of the Props sectionThe new Props section provides comprehensive and clear documentation for the Switch component's properties. This addition greatly enhances the usability of the component by clearly outlining all available props, their types, default values, and descriptions.
Consider adding a brief introductory sentence before the table to provide context, e.g.:
## Props The following props can be used to customize the Switch component: | Name | Type | Default | Description | ...This would help set the context for the table that follows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- .husky/pre-commit (1 hunks)
- apps/www/components/SwitchEvent.tsx (1 hunks)
- apps/www/content/docs/components/switch.mdx (7 hunks)
- apps/www/public/registry/components/switch.json (1 hunks)
- packages/ui/src/components/switch.tsx (1 hunks)
🧰 Additional context used
🔇 Additional comments (15)
.husky/pre-commit (1)
1-4: LGTM! The pre-commit hook is well-structured.The script correctly sets up a pre-commit hook using Husky. It ensures that the registry is built and the code is formatted before each commit, which aligns well with the PR objectives of refactoring the Switch component and updating its documentation.
apps/www/components/SwitchEvent.tsx (3)
6-6: Approved: Component name correctionThe component name has been correctly updated from
SwiychEventtoSwitchEvent, fixing the typographical error. This change improves code readability and maintains consistency with the component's purpose.
19-19: Approved: Consistent export statementThe export statement has been correctly updated to use the new component name
SwitchEvent. This change maintains consistency with the earlier component name update and ensures the correct export of the component.
Line range hint
1-20: Overall assessment: Positive improvements to SwitchEvent componentThe changes made to this file have successfully addressed several important aspects:
- Corrected the component name from
SwiychEventtoSwitchEvent.- Improved accessibility by updating the
idandhtmlForattributes.- Ensured consistency in the export statement.
These modifications align well with the PR objectives of refactoring the Switch component and its usage. They enhance code quality, improve accessibility, and maintain consistency throughout the file.
To ensure these changes haven't introduced any unintended side effects, please run the following verification:
This script will help ensure that the component renaming has been applied consistently across the project and that there are no naming conflicts.
✅ Verification successful
Verification Complete: SwitchEvent Component Renaming is Consistent
All checks have passed successfully:
- No remaining instances of
SwiychEventwere found.SwitchEventis used correctly in all import statements.- No naming conflicts with
SwitchEventdetected.The component renaming has been applied consistently across the project without introducing any issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new component name is used consistently across the project echo "Checking for any remaining instances of 'SwiychEvent':" rg "SwiychEvent" --type ts --type tsx echo "Verifying the usage of 'SwitchEvent' in import statements:" rg "import.*SwitchEvent" --type ts --type tsx echo "Checking for any potential naming conflicts with 'SwitchEvent':" rg "(?<![a-zA-Z])SwitchEvent(?![a-zA-Z])" --type ts --type tsxLength of output: 848
Script:
#!/bin/bash # Verify that the new component name is used consistently across the project echo "Checking for any remaining instances of 'SwiychEvent':" rg "SwiychEvent" --glob "*.ts" --glob "*.tsx" echo "Verifying the usage of 'SwitchEvent' in import statements:" rg "import.*SwitchEvent" --glob "*.ts" --glob "*.tsx" echo "Checking for any potential naming conflicts with 'SwitchEvent':" rg "\bSwitchEvent\b" --glob "*.ts" --glob "*.tsx"Length of output: 699
apps/www/public/registry/components/switch.json (4)
Line range hint
6-10: LGTM: Imports and interface definition are well-structured.The new
SwitchPropsinterface correctly extendsReact.ComponentPropsWithoutRef<"button">and includes the necessary props for both controlled and uncontrolled usage. This aligns well with the PR objective of making the Switch component self-dependent.
Line range hint
13-17: LGTM: Component definition and props handling are well-implemented.The use of
React.forwardRefand proper props destructuring, including default values, demonstrates good React practices. This implementation supports both controlled and uncontrolled usage of the Switch component.
Line range hint
55-57: LGTM: Component export and display name are correctly implemented.Setting the display name to "Switch" is a good practice for debugging and development tools. The export statement is correct and consistent with the component implementation.
Line range hint
1-62: Overall, the Switch component refactor is well-implemented and meets the PR objectives.The new implementation successfully removes the dependency on @radix-ui/react-switch, creating a self-dependent Switch component. Key improvements include:
- Proper state management for both controlled and uncontrolled usage.
- Enhanced accessibility with ARIA attributes and a native button element.
- Consistent styling using Tailwind CSS classes.
- Support for forwarded refs.
The component now aligns well with React best practices and the project's coding standards. The minor suggestions provided earlier can further enhance its accessibility and state management.
packages/ui/src/components/switch.tsx (3)
1-10: LGTM: Imports and SwitchProps interface are well-definedThe import statements and SwitchProps interface are correctly implemented. The "use client" directive is appropriate for a client-side component, and the SwitchProps interface extends React.ComponentPropsWithoutRef<"button">, allowing for all standard button props while adding the necessary custom properties for the Switch component.
27-37: LGTM: Robust state management implementationThe component implementation demonstrates excellent handling of both controlled and uncontrolled usage. The use of forwardRef is appropriate, and the state management logic is well-implemented. The defaultChecked prop is correctly used to initialize the internal state.
Line range hint
73-76: LGTM: Correct displayName and exportThe displayName is correctly set to "Switch", which is helpful for debugging. The component is properly exported.
apps/www/content/docs/components/switch.mdx (4)
10-10: LGTM: Import statement correctedThe import statement for
SwitchEventhas been correctly updated, fixing the typo from 'SwiychEvent' to 'SwitchEvent'. This change ensures consistency with the component name and file path.
78-81: LGTM: Updated import for Disabled Switch exampleThe import statement has been correctly updated to use the new package structure. The example still effectively demonstrates the usage of the
disabledprop for the Switch component.
127-130: LGTM: Updated Default Checked Switch exampleThe import statement has been correctly updated to use the new package structure. The example continues to effectively demonstrate the usage of the
defaultCheckedprop for the Switch component.
Line range hint
1-181: Overall excellent documentation updateThe changes to this documentation file effectively capture the refactoring of the Switch component to be self-dependent. The updates include:
- Corrected import statements throughout the file.
- Updated usage examples that demonstrate various features of the component.
- A comprehensive new Props section that clearly outlines all available properties.
These changes significantly improve the usability and understanding of the Switch component for developers. The documentation now accurately reflects the component's implementation and provides clear guidance on its usage.
Great job on maintaining and improving the documentation alongside the component changes!
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
ruru-m07
added
component: update
Fixes #63
Refactor the
Switchcomponent to be self-contained and remove dependency on@radix-ui/react-switch.Switch Component:
@radix-ui/react-switch.Switchcomponent to be self-contained with its own state management and event handling.Switchcomponent's props to includecheckedandonCheckedChange.Documentation:
apps/www/content/docs/components/switch.mdxto reference the self-containedSwitchcomponent.Switchcomponent's props.Dependencies:
@radix-ui/react-switchfrompackages/ui/package.json.pnpm-lock.yamlto remove references to@radix-ui/react-switch.Registry:
apps/www/public/registry/index.jsonandapps/www/public/registry/components/switch.jsonto remove the dependency on@radix-ui/react-switch.apps/www/registry/ui.tsto remove the dependency on@radix-ui/react-switch.For more details, open the Copilot Workspace session.
Summary by CodeRabbit
New Features
className,disabled,id,defaultChecked, andonCheckedChange.<button>element, enhancing accessibility and state management.Bug Fixes
SwiychEventtoSwitchEventand updated related ID attributes for improved accessibility.Documentation