-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
BorderControl: Fix focus styling #40951
Conversation
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 putting up these PRs to fine-tune the UI @aaronrobertshaw! The border radii are looking good to me, but the border width looks slightly inconsistent to me compared to similar components (e.g. InputControl
) where the focus style doesn't change the overall size of the field — I was wondering if setting inset
will fix it (and / or if the border-width needs to be slightly heavier?)
e776ef4
to
55beb85
Compare
This PR now reworks the application of borders to the Screen.Recording.2022-05-10.at.6.57.15.pm.mp4 |
Nice catch. Thank you. I missed tweaking the button's border radii after refactoring the styling in ec58f46. That's been fixed now. |
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.
Excellent, thanks for reworking this @aaronrobertshaw, that's looking really solid now! ✨
✅ Works nicely in the editor and the focus style is consistent with the nearby UnitControls
✅ Tested in Storybook that it looks good in RTL mode
LGTM! 🚀
Fixes: #40897
What?
Makes the inner component focus styles and border radii consistent.
Why?
The small details matter.
How?
Updated focus styles and border radii including RTL styling.
UnitSelect
BorderControlDropdown
InputControl's BackdropUI
Testing Instructions
npm run storybook:dev
Screenshots or screencast
Screen.Recording.2022-05-10.at.11.25.21.am.mp4