-
-
Notifications
You must be signed in to change notification settings - Fork 78.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
Focus ring helper and utilities #33125
Conversation
Loving the way you did this. Couldn't we jump on the opportunity to improve our overall focus styles, regarding non-text contrasts? @MartijnCuppens's suggestion in #29422 (comment) using two solid shadows (also pretty much what's done in Boosted, however using Might also wait for another PR, but IMHO it'd make a lot of sense. |
I'm all for having a consistent focus style, but wouldn't this be considered a breaking change for some people? I mean, we've seen numerous times people asking how to drop focus styles because they probably don't know that they shouldn't, unless they provide some kind of replacement. |
Well, this is another consideration that's unrelated to the kind of focus styles we set. Dropping the focus styles while ensuring accessibility requires However depending on how we'd like to implement a new focus style, we need caution to prevent BC. That might not be possible, didn't try that much. |
Taking off Beta 3—let's discuss further and figure out a plan :). |
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.
Coming back to this, it's really helpful and should be shipped, so
We'll try to improve our focus styles later 👍
433a148
to
ebd385f
Compare
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.
Looks good!
Could be a great opportunity to handle double shadows to improve focus indicator contrasts too, and maybe use :focus-visible
as a progressive enhancement?
I keep this in mind for later 👌
Oh and we already had this discussion, obviously 😅 |
Thanks @ffoodd! Going to wait for v5.3.0 for this—want to see where I can build this into our other components with additional CSS variables. |
eff75c9
to
f170f19
Compare
9bbb19f
to
643bc14
Compare
@julien-deramond Tried resurrecting this one—still needs a bit of work to apply the updated focus shadow to other components. Maybe that could wait until later though, like in v6, and we just have an extra helper/utilities for 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.
I haven't looked at all the source code yet but do we plan to have also a focus-visible-ring
utility?
|
||
## Example | ||
|
||
Click into the example below and press <kbd>Tab</kbd> to see the focus ring in action. |
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.
Maybe we can tell that users can also simply click on the link to see the custom focus ring. IDK if it can be helpful but we could also remind folks that :focus
is for keyboard and mouse in the description.
:focus
styling$input-btn-focus-*
variables to new global variables (come v6, we'll simplify these variables).nav
,.nav-pills
, and.nav-tabs
)Closes #32357.
Live previews