SPT: Exclude the template preview iframe from tabIndex and pointer events #45229
Conversation
Copons
added
[Type] Bug
[Pri] High
[Status] Needs Review
|
This PR does not affect the size of JS and CSS bundles shipped to the user's browser. Generated by performance advisor bot at iscalypsofastyet.com. |
| 0 1px 5px 0 rgba( 0, 0, 0, 0.2 ); | ||
| pointer-events: none; |
There was a problem hiding this comment.
This line is the only relevant change, rest is just formatting.
| @@ -206,6 +207,7 @@ const BlockFramePreview = ( { | |||
| title={ __( 'Preview', 'full-site-editing' ) } | |||
| className={ classnames( 'editor-styles-wrapper', className ) } | |||
| style={ style } | |||
| tabindex="-1" | |||
There was a problem hiding this comment.
This line is the only relevant change, rest is just the build system updating the docs.
|
Caution: This PR affects files in the Editing Toolkit Plugin on WordPress.com D48650-code has been created so you can easily test it on your sandbox. See this FieldGuide page about developing the Editing Toolkit Plugin for more info: PCYsg-ly5-p2 |
There was a problem hiding this comment.
While this seems to get the job done, Im slightly concerned (although a bit ignorant) about the effect it may have on accessibility and screen reading. If we cannot tab to the preview, does that make it not selectable for screen reading or have any other implications on accessibility? 🤔 Im really not sure.
I have an unconfirmed suspicion that this may be fixed by WordPress/gutenberg#24397 which was created to fix a similar tabbing through previews issue - #44042 . That is included with 8.8 which I believe should be live on edge sites early next week, so it may be worth testing this issue again once that is live. But if that does not fix it and there isn't an a11y concern introduced, I think this is a good approach!
|
I tested it with 8.8 but the PR that supposed to fix this: WordPress/gutenberg#24397 had a bug where it caused an infinite loop of adding-removing the tabindex. I was still able to tab into the preview block links. Filed a PR to fix it: I'll reproduce this and test it with my PR and see if fixes this one. |
|
WordPress/gutenberg#24935 got merged and tested it locally. It doesn't crash the editor anymore. Let's wait for 8.9 and test it on edge sites just to be sure. |
Oh wow! Now that you mention it I can see that next line in the files changed being problematic.
Awesome, thats great to hear! Thanks for picking this up. |
david-szabo97
added
[Status] Blocked / Hold
DO NOT MERGE
and removed
[Status] Needs Review
|
Fixed in Gutenberg v8.9 by WordPress/gutenberg#24935 |
Changes proposed in this Pull Request
I'm not exactly sure why this happens, but how is perfectly explained in #44038: some elements inside the template preview, when focused, throw a
getBoundingClientRect of undefinederror that crashes the editor.The
Popovercomponent seems to be involved, but given how common that is, it doesn't narrow our investigation enough.The approach proposed in this PR is indeed a workaround, and there are a11y concerns.
We are in fact making it impossible to tab into the template preview: screen readers won't be able to "explore" the preview anymore.
Given how the template selector is currenctly structured, I would probably question its a11y anyway (e.g. users need to tab through all templates before getting to the preview).
Testing instructions
Fixes #44038