Persistent List View: Fix the list stealing focus from the canvas on item mount #31058
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
When a block is selected, opening the persistent list view moves the focus away from the canvas to the corresponding list item. To prevent the list view from stealing focus every time the selected block changes if the list view is open, the focus change only happened on list item mount. This approach didn't take into account newly created blocks, which inherently spawn a new list item, which of course mounts and steal focus from the canvas. This fix makes sure that the focus change only happens when the entire list view mounts for the first time. Creating new blocks while the list view is open, will keep the focus in the canvas, as expected.
Copons
added
[Type] Bug
Copons
requested review from
mattwiebe,
vindl,
david-szabo97,
jeyip and
Addison-Stavlo
7 tasks
|
Size Change: +36 B (0%) Total Size: 1.47 MB
ℹ️ View Unchanged
|
TestingRequirements
Browsers
Notes:
|
|
|
|
I tested this with mac voiceover, and when clicking on a list view item, the screenreader would start speaking about the list block being selected. The automatic focus shift, however, would abruptly cut off the list view commentary and suddenly start new commentary about the block in the canvas. The interaction sounded jilted. I haven't kept up with discussions about accessibility for the persistent list view, but if this is something we're planning to handle in the future, then everything else looks good 👍 |
This makes sense. If you are using keyboard navigation then it moves the selection in the list view, but doesn't select it. When you click on it you perform two actions: "focus" and "click". I don't think there is a solution for this, and I don't think this is something we need to solve. |
@jeyip Oh this makes sense! |
kevin940726
reviewed
May 3, 2022
| } ), | ||
| [ | ||
| __experimentalFeatures, | ||
| __experimentalPersistentListViewFeatures, | ||
| blockDropTarget, | ||
| isMounted.current, |
There was a problem hiding this comment.
Just found this and wanted to chime in with a small feedback: Ref values (ref.current) shouldn't be included in the deps array as suggested by the React team (and the lint rule).
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.
Description
When a block is selected, opening the persistent list view moves the focus away from the canvas to the corresponding list item.
To prevent the list view from stealing focus every time the selected block changes if the list view is open, the focus change only happened on list item mount.
This approach didn't take into account newly created blocks, which inherently spawn new list items, which of course mount and steal focus from the canvas.
This fix makes sure that the focus change only happens when the entire list view mounts for the first time.
Creating new blocks while the list view is open, will keep the focus in the canvas, as expected.
How has this been tested?
(Don't open the Inserter which would close the List View)
Repro steps
To clarify the issue, here are some simple repro steps:
(Don't open the Inserter which would close the List View)
What I expected
To see what I typed show up in the paragraph block.
What happened instead
Nothing showed up, because the focus was not on the actual block, but on its corresponding list item.
To be able to actually type something (or generally, to interact with the block), I would have needed to hit enter, "activating" the list item, and moving the focus back to the canvas.
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist:
*.native.jsfiles for terms that need renaming or removal).