Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: improve focus selector perf with focus-visible and focus-within polyfill #24154
feat: improve focus selector perf with focus-visible and focus-within polyfill #24154
Changes from 9 commits
c8f83e0
f23ceb7
ba99928
221b523
be7ee86
4afce7a
26dbf9d
8b9742b
ff125d3
a74be74
62fa5f1
4ff8d9b
990ac96
515a4b3
d5aae20
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The story with
createCustomFocusIndicatorStyle()
&useFocusWithin()
looks a bit dangerous for me as the dependency onuseFocusWithin()
is implicit. Do you have something on your mind to make it explicit?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.
tbh I am not happy at all that this
focus-within
requires an extra hook, but globally there is no way to know what parent of the focused element should have this class.The 'safety' mechanism currently is that
createCustomFocusIndicatorStyles
needs explicitfocus-within
selector to be used. The styles make sure that there is no any focus styles are applied unlessuseFocusWithin
is called.If anyone uses
:focus-within
directly (I think inputs do it) then they should know that there is no keyboard/mouse heuristic there.If you have any proposals, I'd be happy to hear them, I'm not 100% happy with the current situation either
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 don't have a good proposal, the one that I have will be a regression to the previous behavior in terms of performance...