Fix focus loss happening when installing blocks from the directory#40340
Merged
Conversation
|
Size Change: +19 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
ntsekouras
approved these changes
Apr 14, 2022
ntsekouras
left a comment
Contributor
There was a problem hiding this comment.
Thanks Riad! Looks good!
Member
|
I'm not 100% sure it's an issue with the plugin as of today. It sounds more specific to React 18. Anyway, we can backport all 3 fixes if @youknowriad confirms, we should include it in WordPress 6.0 Beta 2. |
Contributor
Author
|
I think it's not crucial to backport these fixes because they're not "very" visible unless you use React 18 but they're small fixes anyway. I'm ok without backporting them :) |
adamziel
pushed a commit
that referenced
this pull request
Apr 19, 2022
Contributor
|
This was cherry-picked to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
Extracted from #32765
What?
For some reason, in the React 18 upgrade PR #32765,
useFocusOutsideworks better than trunk (I don't know why yet). Which means there were some failing e2e tests that actually show real bugs we have on trunk, more precisely "focus loss" bugs.One of them is in the inserter when installing blocks from the directory: On trunk, on chrome, you can try installing a block from there, you'll notice that when you click a block, the button becomes disabled and the focus is lost (but for some reason the inserter remains open).
This PR solves the focus loss.
Why?
Focus loss is an a11y issue even if the inserter is kept open right now.
How?
We already have a solution for buttons that switch from enabled to disabled to avoid focus loss. We rely on
aria-disabledinstead of really disabling the buttons which keeps the button focusable even if it's disabled temporarily. The button component has a dedicated__experimentalIsFocusablefor these use-cases. We may consider making this API stable separately.Testing Instructions
1- Open the inserter
2- Search for a block in the directory (like masonry)
3- Click the button
4- The focus should stay in the button during and after the installation completes and the inserter stays open.
(We already have an e2e test that covers this but it's passing in trunk because of the mystery bug in useFocusOutside, it will start failing in the React 18 upgrade if we do nothing :) )