Block editor: iframe/writing flow: change tab index to 0 #46323
Merged
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
ellatrix
added
[Type] Bug
|
Size Change: +1 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
alexstine
reviewed
Dec 7, 2022
| @@ -44,7 +44,7 @@ export function useWritingFlow() { | |||
| useArrowNav(), | |||
| useRefEffect( | |||
| ( node ) => { | |||
| node.tabIndex = -1; | |||
| node.tabIndex = 0; | |||
There was a problem hiding this comment.
Hmm, I wonder if this adds an empty tab stop. Not real familiar with how this code works, but doesn't this refer to the virtual DOM div that gets an adjusted tabindex? Maybe this is just the initial value or something.
There was a problem hiding this comment.
This is on the writing flow wrapper, so it shouldn't add an extra tab stop because we intercept tab before and after the writing flow container. Alternatively, I think we can set set this tab index only when multiple blocks are selected (because this is where we move focus), but I don't think it hurts to add it permanently.
alexstine
approved these changes
Dec 13, 2022
There was a problem hiding this comment.
@ellatrix Yep, it does look like if you tab from the Options button through the writing flow wrapper, there is now an empty tab stop. I was thinking this might happen. Not really sure how to go about fixing it unless when focus passes through the wrapper, we focus an element vs. the div.
Anyway, this PR fixes the underlying issue and I think this can be revisited as writing flow concepts will likely change down the road.
Sorry for the delay, long backlog after trip.
|
One other honorable mention, the site editor still does not work for some reason. Not sure what is going on there but this does work for the post editor and the changes you made for the iFrame. I'll likely have to make a Zoom video as I doubt anyone could figure it out visually. There is an unbelievable amount of focus loss in the site editor and not sure why considering the same issues do not exist even with the new iFrame in the post editor. |
ellatrix
force-pushed
the
try/iframe-body-tabindex-0
branch
2 times, most recently
from
2017667 to
a47e4b9
Compare
dmsnell
pushed a commit
that referenced
this pull request
Dec 15, 2022
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.
What?
Tries to fix #46258. Changes the tab index of writing flow (which is the body element in case of an iframe) to 0 (from -1).
I've tested with VoiceOver on Mac, but can't really reproduce the issue, so I'm working in the dark.
@alexstine Does this resolve the issue you're experiencing?
Why?
See #46258.
How?
Testing Instructions
See #46258 for detailed instructions.
Testing Instructions for Keyboard
Screenshots or screencast