Accessibility: Consolidate the focusOnOpen behavior in the withFocusReturn Higher Order Component

[youknowriad]

I noticed that each time we want to use withFocusReturn HoC. Having a focusOnOpen behavior makes sense. While this was made possible with the Popover component right now, we use the withFocusReturn in other places than Popovers.

In this PR I moved the focusOnOpen behavior from the Popover component to the withFocusReturn HoC. It might make sense to rename this HoC, ideas?

I also I'm applying withFocusReturn to the publish panel partially fixing #4187

[afercia]

@youknowriad thanks. I think it makes perfectly sense for components like modals, dropdown menus, etc. that are not mounted initially.

Not sure it works well for the Publish Panel and the Sidebar.

1
Sidebar: When users keep the sidebar open, on page load the sidebar is mounted. As it works now, withFocusReturn sets the firstTabbable to button.editor-sidebar__panel-tab.is-active and the initial focus is actually on the sidebar "active tab".

Worth noting that, depending on the workflow, this button can also be disabled so, regardless of the above, it won't be focusable.

2
Sidebar: there are scenarios where there's no activeElementOnMount yet. For example, on page load I navigate to the X button of the sidebar:

I click the X and focus is moved back to the document root. I'd expect it to move back to the "cog" button.

3
Publish Panel: When closing the publish panel, clicking on the X, focus is not moved back to the "Publish..." button for me. I guess because the header section that contains the "Publish..." button gets unmounted and then mounted again so activeElementOnMount was the previous button and when it re-renders it's actually a new reference in the DOM?

Overall, for the Sidebar and Publish Panel, I think we can't solve the interaction issues with code. The root cause of the problem is in the interface and it should be solved re-thinking the interface.

[youknowriad]

So your saying, it's ok to handle the focusOnMount and focusReturn together but you think we shouldn't use this HoC in the sidebar/publish panel?

Noting that the sidebar already had the withFocusReturn HoC before this PR.

[youknowriad]

I click the X and focus is moved back to the document root. I'd expect it to move back to the "cog" button.

What if we opened the sidebar using the block settings menu? A sidebar can be opened from several places, so its placement in the DOM shouldn't be relative to a toggle IMO

[afercia]

it's ok to handle the focusOnMount and focusReturn together but you think we shouldn't use this HoC in the sidebar/publish panel?

It depends on the component it will be used with. Modals and dropdown and similar always need to get focus on the first tabbable and return focus on the "opener" when closed. So if withFocusReturn is going to always do the two things, then its usage should be limited to only some types of interactions.

Instead, if we want a more generic component, maybe this could be a "focus manager" with different methods that can be used individually? Worth noting there's still one important piece about focus management that's missing as a generic implementation: constraining tabbing within a component. If I'm not wrong, it is implemented in some specific components but there's no generic constrainTabbing. Maybe this could be a third thing to include in a generic "focus manager"?

Noting that the sidebar already had the withFocusReturn HoC before this PR.

Yep I know, and If I'm not wrong yesterday focus was moved back correctly? maybe something changed in the last hours.

[youknowriad]

Not sure what are the actionable items in the PR here. Talking about this particular HoC and not other accessibility utilities.

[youknowriad]

I still think we should consolidate focusOnOpen and withFocusReturn on the same Higher Order Component regardless of the way we fix the publish flow. I'll try to do this in a separate PR.