Navigable regions: keyboard shortcut to navigate regions doesn't work from the WP logo any longer

[afercia]

Description

This appears to be a regression on trunk.

Step-by-step reproduction instructions

First, test on WordPress 6.5:

• Edit a post.
• Move focus to the WP logo in the top bar.
• Press Control + backtick or alternatively Control + Option + N
• Observe that each time you press the keyboard shortcut, focus is moved to one of the main sections of the editor, as expected.
• Test on latest trunk.
• Observe pressing the keyboard shortcut from the WP logo doesn't do anything.
• Press the Tab key to move focus to the plus icon button of the main inserter.
• Press the keyboard shortcut and observe it works as expected.
• In the Site editor it seems to work as expected.

Basically it stopped working only in the Post editor and only from the WP logo

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

• [X] Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

• [X] Yes

[afercia]

I spent some time trying to debug this issue but couldn't find the root cause. I'd appreciate any help and guidance Cc @youknowriad

I think this regression is an unintended consequence of https://github.com/WordPress/gutenberg/pull/62118

I noticed that removing the bubblesVirtually prop from the BackButton.Slot makes the keyboard shortcut work again. But it breaks the layout, because of the translate styling used for the Distraction free mode animation. The Post editor and Site editor have a different components hierarchy. That makes me think the keydown event doesn't bubble as expected but it is still a mystery to me because other shortcuts from EditorKeyboardShortcutsRegister do work as expected.

[youknowriad]

Indeed, it seems the moved to the slot introduced this bug. That said, I have a couple of suggestions to fix this bug. Both IMO are needed and both would fix the bug in a different way.

1- For me region navigation doesn't really make sense other than applied globally to the page. (Same as the command palette for instance), in that sense, the way we implement the shortcut today is broken, when you call useNavigateRegions, the shortcut is only enabled when you're within that ref. I think that's wrong and I should be able to trigger the region navigation from anywhere. So I'd consider passing bindGlobal: true to the useShortcut call for the region navigation hook (just like we do for the command palette for instance)

2- The difference between the post and the site editor, is that the post editor relies on the useNavigateRegions that is called within InterfaceSkeleton but the site editor calls useNavigateRegions globally as well. I think all "apps/pages" that want to use region navigation should call it at the top level (outside InterfaceSkeleton

[afercia]

I should be able to trigger the region navigation from anywhere. So I'd consider passing bindGlobal: true to the useShortcut call for the region navigation hook (just like we do for the command palette for instance)

Oh yes, I did notice the Command palette shortcut works when focus is on the WP logo and I was wondering why. That explains it, thanks for the explanation 🙇🏽

[afercia]

@youknowriad I'm afraid we spoke too soon. To my understanding, bindGlobal is a prop of useShortcut / useKeyboardShortcut. Instead, the navigate region uses its own implementation in useNavigateRegions.

Question: why the BackButton slot uses bubblesVirtually in the first place? Seems to me it was originally introduced in https://github.com/WordPress/gutenberg/pull/22323 but I'm not sure it is really needed.

In https://github.com/WordPress/gutenberg/pull/63611 I'm removing bubblesVirtually from the slot and cleaning up things. It seems to fix the issue but I'd appreciate some feedback on why it fixes it.

[youknowriad]

Question: why the BackButton slot uses bubblesVirtually in the first place? Seems to me it was originally introduced in https://github.com/WordPress/gutenberg/pull/22323 but I'm not sure it is really needed.

I don't think we should focus on the bubbleVirtually or remove it. It's the recommendation that all new slots use bubbleVirtually because of how it allows context propagation. It was the case before, we just updated the post editor implementation to use the slot as well (unified with site editor).

I think we should consider using the same approach the command palette shortcut uses, whether that's bindGlobal or something. I think the region navigation should work regardless of where the focus is (or isn't). For instance if the focus is in "body", region navigation doesn't work and that's problematic to me.

[afercia]

Yes I understand the keyboard shortcut should work from anywhere. However:

For instance if the focus is in "body", region navigation doesn't work and that's problematic to me.

Focus should never be outside of the navigable regions. Worth reminding the navigable regions are ARIA landmarks and all perceivable content should be inside landmarks.

W3C ARIA Landmarks Example > General Principles

if using landmarks, all perceivable content should reside in a semantically meaningful landmark in order that content is not missed by the user.