Add block inspector virtual bubbling option #24991
Conversation
tellthemachines
requested review from
ellatrix and
youknowriad
as code owners
|
Size Change: +33 B (0%) Total Size: 1.17 MB
ℹ️ View Unchanged
|
ZebulanStanphill
added
the
Needs Accessibility Feedback
There was a problem hiding this comment.
This does the trick. Tested both the post editor and navigation editor. The post editor still behaves the same.
The block inspector in the nav screen now doesn't close unexpectedly, focus is retained, and Escape can still be used to close the popover with focus returning to the opening button.
| { bubblesVirtually ? ( | ||
| <InspectorControls.Slot bubblesVirtually /> | ||
| ) : ( | ||
| <InspectorControls.Slot /> | ||
| ) } |
There was a problem hiding this comment.
Could these be written without the ternary?
| { bubblesVirtually ? ( | |
| <InspectorControls.Slot bubblesVirtually /> | |
| ) : ( | |
| <InspectorControls.Slot /> | |
| ) } | |
| <InspectorControls.Slot bubblesVirtually={ bubblesVirtually } /> |
There was a problem hiding this comment.
Oh, yes, of course!
talldan
added
[a11y] Keyboard & Focus
[Focus] Accessibility (a11y)
|
I think the main issue is that for DOM specific events where we rely on DOM bubbling instead of React event bubbling, we should rely on DOM events In other words, I think the issues is not in the inspector and the inspector should always use bubblesVirtually. The issue instead is in |
@youknowriad Though |
|
@talldan Can we rely on event capturing maybe? |
|
We could give it a try, but there's a risk it might break existing uses of
Don't we already have some issues with nested popovers/modals not closing when expected? Maybe worth checking if this would actually fix them 😅 |
|
@youknowriad @tellthemachines Here's a quick test of that - 65c5a6b It looks like there are some issues with the nested popovers in #24304, but maybe as you say some kind of event capturing could solve that. |
conceptually, it seems that withFocusOutside shouldn't rely on React event bubbling anyway. I'm having a hard time thinking about any potential use-case there. Doesn't mean it doesn't exist though. |
|
@talldan Can we try that on a PR and see how it looks? |
|
I didn't mention but omitting event bubbling in the inspector breaks the inspector for folks using RichText on the inspector. |
Description
Fixes #24809
The block inspector in the Navigation screen is inside a Dropdown, and its default
bubblesVirtuallycauses Dropdown'swithFocusOutsidewrapper to not receive focus/blur events correctly. This PR adds abubblesVirtuallyprop toBlockInspector, defaulting totrue, so that it can be set tofalsewhenever the inspector needs to be used inside a dropdown or popover.How has this been tested?
Tested in browser, checked keyboard navigation follows the correct sequence.
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: