[TrapFocus] Make possible to avoid focusing wrapper

[dmtrKovalenko]

• I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Currently TrapFocus component is focusing the modal root element (paper) when we are looping out the controls in the focus.

This is a problem for some kind of dialogs, for example, date-picker. We should avoid focusing on the root element of date picker. Focusing on the root element will only confuse the screenreader because there is no label on the date picker itself.
Anyway, that is not recommended to make "big" elements focusable, because it is not clear which element is focused. For example, this is the default behavior of angular material.

It would be nice to have an option like disableRootFocus or something like that that prevents focusing the root element in the dialog.

Examples 🌈

Motivation 🔦

From wia-aria modal dialog example guide

• The larger a focusable element is, the more difficult it is to visually identify the location of focus, especially for users with a narrow field of view.
• The dialog has a visual border, so creating a clear visual indicator of focus when the entire dialog has focus is not very feasible.

[oliviertassinari]

@dmtrKovalenko Thanks for opening this issue. For context, and for the next person that will have a look at the problem, the current behavior is the result of a tradeoff taken in #14545 to save bundle size. There is likely an opportunity to do better ✨.

[oliviertassinari]

If it can inspire a solution:

• https://github.com/Hacker0x01/react-datepicker/blob/9d6590e46b89e684d5438796cbaf3ec29cd3ed08/src/tab_loop.jsx
• https://github.com/davidtheclark/tabbable/blob/4f88b5b0c0b3a6d2372a4f45bbffea368ec92060/src/index.js#L1

[PaulSavignano]

@oliviertassinari I'll look into it.

[eps1lon]

Can we take a step back and first identify where we use this focus trap? Seems to me it always traps focus inside a widget where we definitely can focus the widget container. Then the question moves from "how to disable root focus?" to "configure what container TrapFocus should focus!".

I say this because the approach in #22062 is fundamentally flawed since it assumes that determining what element is tabbable is something we can do by reading the DOM. However, what elements are tabbable (in sequential focus order) can determined by the user agent (https://html.spec.whatwg.org/multipage/interaction.html#sequentially-focusable). We can ask if an element is in sequential focus order by checking tag names or tabIndex. But returning "No" from that question does not imply that the element is not tabbable.

Specifically for the date pickers I recognized the current flaws of the focus trap if it wraps a custom transition component. Sometimes they add wrapper divs, sometimes they don't. We should be able to tell the FocusTrap what the root is e.g.

const rootRef = React.createRef();
<FocusTrap rootRef={rootRef}>
  <TransitionComponent>
    <div data-testid="backdrop">
      <div role="dialog" aria-labelledby="..." ref={rootRef} tabIndex={-1} />
    </div>
  </TransitionComponent/>
</FocusTrap>

[gregnb]

If it can inspire a solution:

• https://github.com/Hacker0x01/react-datepicker/blob/9d6590e46b89e684d5438796cbaf3ec29cd3ed08/src/tab_loop.jsx
• https://github.com/davidtheclark/tabbable/blob/4f88b5b0c0b3a6d2372a4f45bbffea368ec92060/src/index.js#L1

A product I'm working on is being audited by a FAANG company for A11y. Right now, they are dinging us for the extra tab key a user has to hit to cycle the loop. The audit team has labeled this as a "loss of focus" (these come from the sentinels). At some point, we'll have to remediate the issue for our project.

With that said, are we still open to a solution that leverages tabbable? i wouldn't mind taking it up

[oliviertassinari]

@gregnb There was a first attempt at solving the problem in #21857. It's definitely something we need to resume. Ideally, I think that the container would only be focused if there are no tabbable items inside it.

[eps1lon]

Reverted #23364 until we have the full picture.

From the issue description it also sounds like #23364 didn't directly address the issue. We can probably fix this particular issue with a more focused and harmless approach.

[oliviertassinari]

A related discussion #23827.

didn't directly address the issue. We can probably fix this particular issue

@eps1lon What's "the issue"? We might not consider the same one.

more focused and harmless approach.

Happy to hear more about this alternative solution :D.