-
Notifications
You must be signed in to change notification settings - Fork 37
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
chore(app): Adding small switch size per design #1667
Conversation
@@ -2,15 +2,18 @@ import * as Switch from '@radix-ui/react-switch'; | |||
import React from 'react'; | |||
import {twMerge} from 'tailwind-merge'; | |||
|
|||
export type SwitchSize = 'small' | 'normal'; |
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.
Feels inconsistent for button sizes to be small/medium/large but switch sizes to be small/normal.
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.
A foolish consistency is the hobgoblin of little minds
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.
But honestly, I'm open to suggestions. I don't know that there are to be three variants on this so does making "normal" => "medium" feel right? I guess it's fine. Because worst case if there's a smaller one it's "x-small" | "small" | "medium"?
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.
pushed
<Switch.Thumb | ||
className={twMerge( | ||
'h-[22px] w-[22px] rounded-full bg-white transition-transform duration-100 ease-out', | ||
props.checked ? 'translate-x-20' : 'translate-x-0', | ||
' rounded-full bg-white transition-transform duration-100 ease-out', |
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.
leading space
size === 'small' ? 'h-[14px] w-[14px]' : 'h-[22px] w-[22px]', | ||
size === 'small' && props.checked ? 'translate-x-12' : '', | ||
size === 'normal' && props.checked ? 'translate-x-20' : '', | ||
props.checked ? '' : 'translate-x-0', |
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.
Maybe !props.checked && 'translate-x-0'
?
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.
As you wish
Preview this PR with FeatureBee: https://beta.wandb.ai/?betaVersion=63ba7292d5e7bb34372aa36c93ac7f34aea35473 |
https://www.figma.com/design/AbybbouWhi78Bp99M1FX2B/Workspace-settings-2.0-(May-2024)?node-id=2099-57562&t=edMw7C4Gx3saxonG-0