Fix focus loss when creating pages from link control search results #40342
Conversation
This file contains 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
youknowriad
added
[Type] Bug
|
Size Change: -2 B (0%) Total Size: 1.23 MB
ℹ️ View Unchanged
|
|
@youknowriad, the LinkControl component closes after the page is created and focus is lost in my tests. ScreencastCleanShot.2022-04-14.at.21.18.26.mp4 |
|
Thanks for testing @Mamaduka I might have done something wrong when extracting the changes. I'll check this again tomorrow. |
|
Ok, so this is actually working as intended, but I still think this is a worthwhile change. Let me explain, so on the React 18 PR, without this change, the "link control" closes as soon as you "start" creating the page and not when it "finishes" creating the page. I think that's an issue because you don't see the feedback ... Also, the closing happens because for a small period of time (when the page is "creating") the focus is lost. The focus loss happens in trunk too but it's hard to see it because creating the page is generally fast and when the creation is finished, the focus is moved to the paragraph. |
Mamaduka
approved these changes
Apr 15, 2022
There was a problem hiding this comment.
Thanks for providing more details, @youknowriad.
I can confirm that this fixed the issue by throttling the network and tracking the focus element.
gziolo
added
the
Backport to WP 6.7 Beta/RC
adamziel
pushed a commit
that referenced
this pull request
Apr 19, 2022
adamziel
removed
the
Backport to WP 6.7 Beta/RC
|
This was cherry-picked to |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 creating pages from link control. This PR solves the focus loss there.
Why?
Focus loss is an a11y issue even if the inserter is kept open right now.
How?
We already have a dedicated logic in the component to avoid focus loss when switching from "editing link" to "showing link" but there's a third state "loading state while creating a page" that was not covered by the focus loss preventing logic. This PR just triggers the effect when the
isCreatingPageboolean becomes true.Testing Instructions
1- Write a paragraph
2- Click the "add link" button
3- Write something in the input
4- Select "Create page" from the dropdown
5- The LinkControl component should still have focus and be visible.
(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 :) )