Edit Post: Make sidebar header focusable for button focus normalization #21031
Conversation
aduth
added
[Type] Bug
|
Size Change: -68 B (0%) Total Size: 856 kB
ℹ️ View Unchanged
|
| // the sidebar is unmounted, the corresponding "focus return" behavior to | ||
| // shift focus back to the heading toolbar would not be run. | ||
| // | ||
| // See: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button#Clicking_and_focus |
There was a problem hiding this comment.
I know we tried this before but forgot the conclusion, why don't we try to normalize it on the Button click handler (or related events).
There was a problem hiding this comment.
Asking cause it's not the first time we have the button click focus issues and certainly not the last time.
There was a problem hiding this comment.
I've been thinking about this as well.
I think one of the main reluctances I'd have is in breaking expectations about how elements are expected to work, but I'm not sure how well this holds up:
- The abstraction of a button shouldn't necessarily be known to be tied to expectations around a particular HTML element.
- The "expected" behavior is clearly not very expected, or at least it's highly inconsistent between browsers and consequently prone to error.
On the technical front, I've been wondering as well just how this would work:
- If we dispatch a
new window.Event, how well does would it interoperate with React's event system, and in particular with portaled rendering which can't rely on DOM event bubbling?
I've also been thinking about a few other options:
- Is
<input type="button">susceptible to this as well? And if not, could we implement<Button>this way? I expect by virtue of the differences between the two (largely ability to use non-text HTML content in<button>), it may be infeasible. - What about something like
<span role="button">? As I understand it, there are a lot of accessibility challenges in usingrole="button"effectively, but if it were technically possible, we could absorb that complexity into the implementation of the core component. - Wrap every
<button>with<span tabindex="-1">, essentially applying the behavior implemented here universally. There are some known issues here (huge proliferation of wrapping elements) and some unknown issues (potential side-effects of so many additional focusable elements).
There was a problem hiding this comment.
I think @diegohaz might have good thoughts on this problem as something he probably already faced with reakit
There was a problem hiding this comment.
On Reakit we opted to ensure consistency across browsers on a Tabbable component (which is used by Button): https://github.com/reakit/reakit/blob/c9f55d7d87f12b58ee409e8a392a9c36c2aadf52/packages/reakit/src/Tabbable/Tabbable.ts#L141-L148
It's important to note that, on Firefox/macOS, calling preventDefault() there also prevents it to get the :active state when clicked (ariakit/ariakit#432). Something like data-active could be used instead. That's the only bad side-effect people noticed so far.
There was a problem hiding this comment.
@diegohaz Thanks for sharing! It hadn't occurred to me as potentially being as simple as calling .focus() on the button node. About the implementation and your point about preventDefault: Can you explain why preventDefault is necessary at all?
There was a problem hiding this comment.
That's because Safari/Firefox on macOS will blur the button if it has focus on mouse down. preventDefault prevents that from happening. But there may be another way to work around that.
aduth
added
the
Backport to WP 6.7 Beta/RC
jorgefilipecosta
removed
the
Backport to WP 6.7 Beta/RC
priethor
added
the
[Focus] Accessibility (a11y)
Fixes #20759
This pull request seeks to resolve an issue where clicking the "Close Sidebar" in Firefox for macOS has two problems:
There are a number of contributing factors here, all of which ultimately stem from a particular browser quirk in how Firefox for macOS treats clicks on button elements. Essentially, the problem is that clicking the sidebar dismiss button never registers as the sidebar ever having received focus, so it does not know to either return focus to the header, nor to dismiss the bottom panel of the sidebar.
In other browsers, the normal expected behavior is:
withFocusReturnstate:gutenberg/packages/components/src/higher-order/with-focus-return/index.js
Lines 105 to 108 in d4f159a
gutenberg/packages/components/src/higher-order/with-focus-return/index.js
Lines 73 to 84 in d4f159a
gutenberg/packages/edit-post/src/components/sidebar/index.js
Lines 33 to 41 in d4f159a
There is some prior art to this proposed solution, specifically in how the sibling inserter captures focus via click on the inserter button:
gutenberg/packages/block-editor/src/components/block-list/insertion-point.js
Lines 166 to 172 in d4f159a
Testing Instructions:
Repeat Steps to Reproduce from #20759, verifying that the panel at the bottom of the sidebar no longer remains visible, and that focus is shifted to the sidebar toggle button in the header toolbar.