-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Color Picker: Match figma metrics. #35039
Conversation
border: ${ CONFIG.borderWidthFocus } solid rgba( 255, 255, 255, 0 ); | ||
box-shadow: inset 0px 0px 0px ${ CONFIG.borderWidthFocus } #ffffff; | ||
border: none; | ||
box-shadow: 0 0 2px 0 rgba( 0, 0, 0, 0.25 ); |
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.
Would it make sense to use ${ CONFIG.radiusBlockUi } (2px) instead of hardcoded 2px?
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.
Yes! I'll make that happen.
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.
Well, sorry no, that makes sense for replacing border-radius: 2px
, but not for the shadow. We want a separate box shadow value for this one, something a la ${ CONFIG.elevation1 } or something.
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.
Got it, in that case feel free to merge the PR 👍
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.
Thanks. Can I get a green check?
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.
Awesome improvement it also fixes the rendering glitches we had on chrome 👍
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 didn't see any regressions to the color picker while testing 👍
Description
Followup to #34598. This one changes the handle to use a box-shadow with a fallback transparent outline for Windows High Contrast mode. This lets us match the Figma specs:
Checklist:
*.native.js
files for terms that need renaming or removal).