-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix inline block toolbar keyboard navigation #28420
Conversation
Size Change: +5.83 kB (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
@@ -19,9 +24,12 @@ const FormatToolbarContainer = ( { inline, anchorRef } ) => { | |||
focusOnMount={ false } | |||
anchorRef={ anchorRef } | |||
className="block-editor-rich-text__inline-format-toolbar" | |||
__unstableSlotName="block-toolbar" | |||
// Render inline | |||
__unstableSlotName={ null } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm passing null
to the slot name so it renders inline in the React tree, and not on a separate slot as it won't find any slot:
gutenberg/packages/components/src/popover/index.js
Lines 627 to 635 in 5c24a7d
if ( slot.ref ) { | |
content = <Fill name={ __unstableSlotName }>{ content }</Fill>; | |
} | |
if ( anchorRef || anchorRect ) { | |
return content; | |
} | |
return <span ref={ anchorRefFallback }>{ content }</span>; |
I don't know if there's a better way to achieve this, but I think we could have an inline
prop or something on Popover.
There is only one improvement I will suggest. Can you modify the PR to allow Alt+F10 open the formatting toolbar if focus in in Caption field? Currently, only Shift+Tab can do this. Otherwise it is functioning well. Thanks. |
@alexstine Thanks for reviewing it! I've added support for Alt+F10. |
@diegohaz I've given it a test, this now seems perfect. Thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, I don't think we should put popovers inline in the editor canvas. Would be great to find another way to do this.
@ellatrix Thanks for reviewing this. The only other way I can think of is creating focus trap elements that will transfer the focus between the image caption editable content and the toolbar and vice-versa. But this technique is rather complex and there are known issues with VoiceOver (VO will just ignore these focus trap elements when navigating through elements with VO keys unless you have an unreliable timeout on it). The safest way I can think of is relying on the DOM order like in this PR. What do you think is the caveat on this? |
It should align more with how the block toolbar works, which is also not embedded in the content's DOM. Ideally, the UI DOM and the content DOM should be kept separate so the styling is isolated. It's also possible to run into some weird selector issues like :first-child and :last-child if the popover unexpectedly appears in the content. To align better with the block toolbar, I'd imagine you could reverse tab into the inline toolbar just like you would for the block toolbar. Another reverse tab could put you in the block toolbar. In other to accomplish that, the inline toolbar DOM should be rendered right after the block toolbar DOM. |
I created #29874. The inline toolbar is actually accessible by keyboard, it's just misplaced. It should be rendered after the block toolbar instead of before. |
Fixed in #31134 |
Closes #26114
Description
This PR makes the inline toolbars (present on Image, Audio and Video captions for example) accessible through the keyboard. Just like the block toolbars, now it's possible to press Shift+Tab or Alt+F10 to access the nested toolbar.
How has this been tested?
Check Image, Audio, Video, Embed and Gallery (including the gallery image) captions and try to access the caption toolbar with the keyboard by pressing Shift+Tab or using screen reader tools to find the toolbar.
I've also included an e2e test on the Image block.
Types of changes
Bug fix
Checklist: