-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
fix(checkbox): add padding to accommodate focus state box-shadow #6481
Conversation
Deploy preview for carbon-elements ready! Built with commit 0f6450f |
Deploy preview for carbon-components-react ready! Built without sensitive environment variables with commit 0f6450f https://deploy-preview-6481--carbon-components-react.netlify.app |
Deploy preview for carbon-elements ready! Built with commit 3a81b59 |
I just wanted to note here that depending on the solution for #6471, this PR may need to shift direction. |
Deploy preview for carbon-components-react failed. Built without sensitive environment variables with commit 3a81b59 https://app.netlify.com/sites/carbon-components-react/deploys/5f15a8aee754420008cebbed |
@jendowns Hey Jen! Going to bring this up to the team since it seems to be part of a larger discussion. Will keep you posted! |
@andreancardona totally understandable! To be honest I'm not sure there's a path forward with this particular solution, since box-shadow isn't going to work for Windows high-contrast mode anyway. And other solutions (outline, border) won't require this work around. So taking all of that into account, do you mind if I close this PR and defer to whatever solution you want to go with for #6471? |
@jendowns Yep that works for me! :) |
Closes #6480
Adds 2px top and left padding to the checkbox's wrapper element. This will make space for the focus state's box-shadow, so that the shadow isn't cut off when the checkbox is inside a container that has an overflow. (See screenshots in #6480 for details)
Let me know if you want a diff approach! 👍
Changelog
New