-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add Size small for toogle #571
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces updates to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
🪧 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 (
|
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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/core/Toggle/Toggle.stories.tsx (1)
28-31
: Consider enhancing story documentationWhile the implementation is correct, consider adding:
- A description for the Small story to document the use case
- Parameters to control the checked/disabled states for comprehensive testing
export const Small = Template.bind({}); +Small.parameters = { + docs: { + description: 'Small variant of the Toggle component, designed for compact UI layouts.', + }, +}; Small.args = { size: "small", };src/core/Toggle.tsx (3)
19-21
: Extract size-specific styles to constantsThe inline Tailwind classes contain magic numbers and make the code harder to maintain. Consider extracting these into named constants or a configuration object.
+const TOGGLE_SIZES = { + small: { + root: 'w-[42px] h-[24px]', + thumb: 'w-[21px] h-[21px] translate-x-1 data-[state=checked]:translate-x-[20px]', + }, + default: { + root: 'w-[56px] h-32', + thumb: 'h-[28px] w-[28px] translate-x-2 data-[state=checked]:translate-x-[26px]', + }, +} as const; + - const rootSize = size === "small" ? "w-[42px] h-[24px]" : "w-[56px] h-32"; - const thumbSize = - size === "small" ? "w-[21px] h-[21px] translate-x-1 data-[state=checked]:translate-x-[20px]" : "h-[28px] w-[28px] translate-x-2 data-[state=checked]:translate-x-[26px]"; + const sizeKey = size || 'default'; + const rootSize = TOGGLE_SIZES[sizeKey].root; + const thumbSize = TOGGLE_SIZES[sizeKey].thumb;
27-29
: Consider adding data-size attributeAdding a data attribute for size would improve testability and allow for easier styling overrides.
<Switch.Root className={cn( "p-0 bg-neutral-600 rounded-full relative inline-block transition-colors data-[disabled]:bg-gui-unavailable data-[disabled]:cursor-not-allowed data-[state=checked]:bg-orange-600 focus:outline-none focus-visible:outline-offset-0 focus-visible:outline-4 focus-visible:outline-gui-focus", className, rootSize, )} + data-size={size || 'default'} id={id} {...props} >
Version in package.json needs to be updated to match PR description
The version in package.json is currently "15.1.4-dev.0746ac8", which is a development version. According to the PR description, this should be "15.1.4" for the release.
🔗 Analysis chain
Line range hint
1-1
: Verify version bump in package.jsonThe PR description mentions a version bump from 15.1.3 to 15.1.4. Let's verify this change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check package.json version rg '"version":\s*"15\.1\.' package.jsonLength of output: 76
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
src/core/Toggle/__snapshots__/Toggle.stories.tsx.snap
is excluded by!**/*.snap
📒 Files selected for processing (3)
package.json
(1 hunks)src/core/Toggle.tsx
(1 hunks)src/core/Toggle/Toggle.stories.tsx
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- package.json
🔇 Additional comments (1)
src/core/Toggle.tsx (1)
34-36
: Verify accessibility for small toggle
The reduced size might affect touch targets and accessibility. Ensure the small toggle meets WCAG requirements for interactive elements.
✅ Verification successful
Small toggle meets accessibility requirements
The Toggle component is built using Radix UI's Switch component, which is inherently accessible and follows WCAG guidelines. The small toggle dimensions (42x24px with 21x21px thumb) meet the minimum touch target size requirements:
- The root size of 42x24px exceeds the WCAG 2.1 minimum target size of 24x24px
- The component inherits Radix UI's built-in accessibility features including:
- Proper ARIA attributes
- Keyboard navigation
- Screen reader support
The implementation aligns with the project's accessibility guidelines as documented in the README, which emphasizes AA standards compliance.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any existing accessibility tests
rg -l "accessibility|a11y|WCAG" --type ts --type tsx
Length of output: 82
Script:
#!/bin/bash
# Check if there are any existing accessibility tests with correct file type
rg -l "accessibility|a11y|WCAG" --type-add 'tsx:*.tsx' --type tsx --type ts
# Check the dimensions of the small toggle
rg "thumbSize|small.*toggle" --type-add 'tsx:*.tsx' --type tsx -C 2
# Look for any aria attributes or role definitions
rg "aria-|role=" --type-add 'tsx:*.tsx' --type tsx -C 2 src/core/Toggle.tsx
Length of output: 907
Script:
#!/bin/bash
# Check the Switch component import to see if it's from an accessible UI library
rg "import.*Switch" --type-add 'tsx:*.tsx' --type tsx src/core/Toggle.tsx -C 2
# Look for any documentation about accessibility
fd README.md --exec cat {} \;
# Check if there are any test files for the Toggle component
fd -e test.tsx -e spec.tsx -e test.ts -e spec.ts Toggle --exec cat {} \;
Length of output: 14270
b9c0742
to
e7e5a90
Compare
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.
Looks great, very small comments. Also, did we lose the Small story? It was there when I first checked but not when I pulled the latest. Only thing that needs to be changed here is the package.json
version, rest are suggestions
82f7fa6
to
84d6f05
Compare
969e483
to
bbb8e64
Compare
…play Update based on review and add type add default value for Toogle size
bbb8e64
to
68a8247
Compare
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.
🎚️
Jira Ticket Link / Motivation
To be used - WEB-4036 as it require a smaller size and based on the design element here
Summary of changes
This pull request includes updates to the
Toggle
component to introduce a newsize
prop, allowing for different sizes of the toggle switch. Additionally, it includes a version bump for the package and updates to the snapshots to reflect the new changes.Updates to
Toggle
component:src/core/Toggle.tsx
: Added asize
prop to theToggleProps
type and updated theToggle
component to handle different sizes for the root and thumb elements.Updates to stories and snapshots:
src/core/Toggle/Toggle.stories.tsx
: Added a new story for the small size toggle.src/core/Toggle/__snapshots__/Toggle.stories.tsx.snap
: Updated snapshots to include the new small size toggle and reflect changes to theToggle
component. [1] [2] [3]Version bump:
package.json
: Updated the version from15.1.3
to15.1.4
.How do you manually test this?
Reviewer Tasks (optional)
Merge/Deploy Checklist
Frontend Checklist
Summary by CodeRabbit
New Features
size
property for theToggle
component, allowing users to customize its dimensions.Toggle
component in Storybook to demonstrate the smaller size variant.Chores
package.json
file to reflect a new development version.