-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
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
[docs] More general docs polishing #30371
Conversation
@oliviertassinari I tried to act on all comments you left on the previous polishment PR. Also, as I changed the styling customization of the Menu to the theme, rather than using the |
@@ -38,10 +38,10 @@ const Tab = styled(TabUnstyled)` | |||
background-color: ${blue[400]}; | |||
} | |||
|
|||
&.${buttonUnstyledClasses.focusVisible} { |
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.
Don't we encourage the community to use focusVisible
? cc @michaldudak
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 changed to only &:focus
because using the focusVisible
class wasn't working 😕 The styles weren't being applied to the component when focused. Not sure if it's really a problem or just me not knowing how to work on it.
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 did not review all of the changes but I would trust the tests.
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.
👍 LGTM
Co-authored-by: siriwatknp <siriwatkunaporn@gmail.com>
Following up on some left-overs from the previous PR (as suggested) while enjoying the time to tweak some stuff a bit more:
theme.typography.fontWeightMedium
to use the plain weight500
.sx
prop so I moved it all to the theme.Deploy previews: