-
Notifications
You must be signed in to change notification settings - Fork 212
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: Refactor Checkbox styles #2542
chore: Refactor Checkbox styles #2542
Conversation
Passing run #6865 ↗︎
Details:
Review all test suite changes for PR #2542 ↗︎ |
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.
Nice work, @RayRedGoose! I made some comments and suggestions.
- Let's go ahead and convert all
px
values torem
- Be sure to check styles like
marginLeft
to make sure they still work with RTL
Otherwise, this looks great. 😄
|
||
const checkboxRippleStyles = createStyles({ | ||
borderRadius: system.shape.round, | ||
boxShadow: `0 0 0 0 ${base.soap200}`, |
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.
Question: A 0 0 0 0
box shadow shouldn't render anything. I see there's a transition
for it, but I don't know how it's transitioning?
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.
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.
I don't think this round should be visible at all
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.
If you agree to remove it, please, approve visual changes in chromatic
3a59f40
to
dfc4ca8
Compare
Co-authored-by: Manuel Carrera <mannycarrera4@users.noreply.github.com>
Co-authored-by: Manuel Carrera <mannycarrera4@users.noreply.github.com>
Co-authored-by: Manuel Carrera <mannycarrera4@users.noreply.github.com>
Co-authored-by: Manuel Carrera <mannycarrera4@users.noreply.github.com>
Co-authored-by: Alan B Smith <a.bax.smith@gmail.com>
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.
✅ 🥊
Summary
Fixes: #2395
Refactors styles in
Checkbox
using new style utilities.Release Category
Components
Release Note
Checkbox
now uses Canvas Tokens and our new styling utilities. The component now supports thecs
prop, but otherwise the API has not changed. It should behave identically as it did in previous versions.