Sidebar: withFocusReturn doesn't work when the Sidebar is already open

[afercia]

withFocusReturn is a pretty neat tool to handle focus, specifically designed for components that "appear" on the page after some user action, to move focus back to the UI control that opened them when they get unmounted.
This is important for keyboard accessibility, to avoid focus losses.

However, withFocusReturn assumes the component is initially not mounted. When the sidebar is already open, closing it using the keyboard does not move focus back to the "Settings" button. That's because when the Sidebar is already open on the initial page load, there's no document.activeElement that withFocusReturn can get and use later.

To reproduce:

• go in the Gutenberg "New Post" page (it's shorter so testing is easier)
• using the keyboard, tab to the Sidebar "X" close button
• press the button either with Enter or Spacebar
• press Tab again: focus is on the document root
• expected: focus should be moved to the Settings toggle button

Worth noting this is not exactly a bug, it's more an improper use of withFocusReturn as it was designed. withFocusReturn works when a component is not initially mounted, so it's suited for modal dialogs, drop-down menus and the like given that they're not already mounted on page load. Instead, they should get mounted after an explicit user action. See also conversation about the Post Visibility "popover" #1886 (review) and #1204

It appears the Sidebar needs a new mechanism to set focus back on the Settings button.

Note: worth noting document.activeElement doesn't work consistently across browsers/platforms when clicking with the mouse on button elements, see: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus. Depending on the specific case, this might be acceptable since handling focus is mainly a concern for keyboard navigation. Something to be aware of though.

[aduth]

We may need to allow a fallback behavior to be specified, to manually control the activeElement property withFocusReturn expects to exist when unmounting. A bit of a step backwards in decoupling disparate components in the application, but in this case there's no other way I see that the Settings button and Sidebar can be linked.

Maybe a selector passed as an option to withFocusReturn higher-order component?

export default withFocusReturn( Sidebar, {
	fallback: '[id^="editor-tools__sidebar-button-"]'
} );

[afercia]

Off the top of my head: what if "problematic" components define their own, specific, focus fallback? If the fallback could be stored somewhere in a way it can be accessed by withFocusReturn, then it would be trivial to check if the activeElement is the body (which is almost always wrong) and use the fallback instead.

[afercia]

Just tested again and still happens.

[jeffpaul]

This ticket was mentioned in Slack in #core-editor by jeffpaul. View the logs.

[afercia]

in this case there's no other way I see that the Settings button and Sidebar can be linked.

Thinking at this, ideally they should be part of the same component 😬 However, that would require some refactoring in the placement of the toggle and sidebar.

[rianrietveld]

This need to be fixed, but also needs more research, so we move this to 5.0

[aduth]

Perhaps as part of the creation of the higher-order component of withFocusReturn, it could assign as a property another component which could be referenced separately as its "focus initiator", used as the fallback.

In this case, Sidebar is the component for which withFocusReturn is applied, so in the Header's rendering of the controlling button, we wrap as:

<Sidebar.FocusInitiator>
	<IconButton
		icon="admin-generic"
		label={ __( 'Settings' ) }
		onClick={ toggleGeneralSidebar }
		isToggled={ isEditorSidebarOpened }
		aria-expanded={ isEditorSidebarOpened }
		shortcut={ shortcuts.toggleSidebar }
	/>
</Sidebar.FocusInitiator>

As far as implementation, there could be a randomly-generated unique ID shared between the initiator component and the withFocusdReturn-enhanced component, assigned as an attribute on a wrapping element of FocusInitiator's children which is then discovered by the focus-return behavior.

The naming implies some two-way relationship of focus, but currently we don't have a continuation of tab stops from the initiator (settings button) to the thing it controls (sidebar). Maybe we should?

[afercia]

Not sure. Remember months ago when I've recommended any toggled UI to be placed immediately after its "initiator" in the DOM? 🙂 That would have made things much simpler. We're now in a situation where major changes can't be made so we have to "repair" managing focus programmatically. Anything that helps in that direction would be very appreciated.

[tofumatt]

That would have made things much simpler.

That's a simplification that ignores the cross-component nature of some interactions as well as the CSS/DOM requirements of things like modals and menus. Keyboard shortcuts are available via a menu item or a keyboard shortcut. Should we show the menu when the keyboard shortcut is used directly from the editor? That would "restore" focus to the menu rather than editor when the modal is left. @aduth's idea implies we could restore focus based on where the keyboard was before the UI was toggled. That's not programmatically repairing focus, it's programmatically improving the handling of focus by being context-aware.

An aside to the issue at hand: the terse reply "not sure" followed by commentary surrounding what we should have done months ago adds nothing to people trying to follow this issue and address it. That makes it harder to follow the issue and fix it, which is indeed bad for accessibility.

[noisysocks]

Noting that this issue pops up when the Keyboard Shortcuts modal is dismissed as well.

[gziolo]

This is going to be an issue with Options modal when #10215 lands.

[designsimply]

Tested and confirmed with WordPress 4.9.8 and Gutenberg 4.1.1 using Firefox 63.0 on macOS 10.13.6 that if I use keyboard navigation to trigger the sidebar "Close settings" button that pressing tab again lands in the content area not on the Settings icon.

[afercia]

Looking into pending issues, just tested again this and it appears there's one more issue: starting with Gutenberg 2.7.0, focus is not moved back to the "Settings" button even when the Sidebar is initially closed. This appears to be a regression, going to label the issue as such.

Also noting this issue is open since more than one year and a half, and still needs to be addressed.