Trap doesn't work if the children are rendered into a React portal

[chandlerprall]

Test case - https://codesandbox.io/s/9y9lmmko84

A stated use case for React's portals it to avoid overflow issues with things like modals & tooltips. In Elastic UI we use quite a few portals, and have a case where our focused-trapped modal (in a portal) tries to open a focus-trapped context menu (in another portal), but the context menu items won't receive mouse clicks because the on-outside-click detection doesn't understand the portal.

FWIW I don't think this is solvable by focus-trap-react without underlying changes to react-dom, but I wanted to start the conversation here. Since outside click detection is often implemented by asking if the parent DOM element contains the even target, I think ReactDOM should expose a similar contains method that continues through portals, just as event bubbling does.

Any similar change would require focus-trap to allow a configuration parameter to specify a contains function, which could retain the current logic as the default for backwards compatibility.

Please let me know your thoughts on this (if the would-be required changes here and focus-trap make sense, etc), and I'm happy to open the related issue on the React github.

[davidtheclark]

Hi @chandlerprall. Your use case does make sense, definitely. I think it would be preferable to figure out a solution in focus-trap that could work with React Portals but is not dependent on React.

One possibility would be to add the following two features to focus-trap:

• Allow a focus-trap to have multiple container elements. So if you knew your focus trap would span #modal and #tooltip, and those don't share a common parent, you could include both.
• If a focus-trap's container element is specified as a string, instead of an actual DOM node, look it up lazily, when necessary to perform a check, instead of right away. Let's say #tooltip actually doesn't exist on the page when #modal opens (as would be the case with React Portals): you could still specify focusTrap(['#modal', '#tooltip']). As long as one of those existed, the trap would start; then when you tab or click, focus-trap checks at that point whether #tooltip exists and accepts it if it does.

Does ^^ sound to you like it would work?

[davidtheclark]

Hm, actually I think that won't work. Because the Portalized element will always be treated as though it comes after the other element, which could result in a wonky tab order.

Not sure what the right approach would be. Suggestions?

[chandlerprall]

Before I think more on solutions, I want to make sure I'm aware of all the requirements; I see two pieces of functionality that need to work/be accounted for:

1. ask multiple DOM roots if they contains a given element
2. track multiple DOM roots so they can be passed to tappable

anything I'm missing?

[davidtheclark]

The problem with Portalized elements is that their position in the document order does not match their perceived position, which means that (except with modals) Tab won't move focus the way you'd expect, either outside or inside a focus-trap.

Let's say there's no focus-trap involved at all, and I have 4 focusable elements: A, B, C, D. B is a button and if you click it a popover opens above B, containing focusable elements E and F. If you Tab through the document now (with the popover open) you'd expect, based on visual appearance, for the focus order to be A, B, E, F, C, D. That's what you'll get, too, if the popover is absolutely positioned within the typical document order, right after B. However, if the popover's contents are out in a Portal, the tab order is going to turn out to be A, B, C, D, E, F.

It's worth explaining this problem without a focus-trap because it translates to a problem for focus-trap. If [A, B, C, D] is one part of the trap and [E, F] is another, how does focus-trap know the following:

• If I Tab from D, where should focus go? A or E?
• If I Tab from B, should it go to C or E?
• If I shift+Tab from A, should it go to D or F?
• If I Tab from F, should it go to A or E?
• If I shift+Tab from E, should it go to F or B?

You'd come up with different answers based on whether you were matching behavior that a user expects, given what they're seeing, or accepting behavior imposed by the Portal.

This is all making me worry that React Portal's might be inherently inaccessible (for focus) except as modals (or with very fine-grained, implementation-specific, and careful focus control)?

[chandlerprall]

I see what you mean; in my situation the complication is avoided because the popup itself is a focus trap

<Modal>
  <FocusTrap>
    <Popover>
      <Portal>
        <FocusTrap>
          {content}

and I think not having that second focus on the popover content would be bad UX (would the use ever expect to tab through a popover context in addition to the surrounding content?).

[davidtheclark]

If your popup is already a focus trap, could you pause the first trap (modal) before activating the second (popup)? Similar to the "nested" example on this page: http://davidtheclark.github.io/focus-trap/demo/?

[chandlerprall]

Not really; the components have no way to communicate that to each other. I could create a wrapper around our use of FocusTrap that passes that knowledge around via context, pausing and unpausing the ancestral trap when a descendant trap is mounted / unmounted. That context would pass through portals as well.

[chandlerprall]

Is this context-watching something that focus-trap-react could build in? I'm not sure if it would break any intended use cases.

[davidtheclark]

@chandlerprall Yeah, a built-in system for automatically pausing & unpausing when there are nested focus traps would be a great new feature for this library. I don't think it should break any intended use cases — as far as I know, it would only fix behavior and make nesting focus traps easier. Are you interested in working on a PR?

[chandlerprall]

Sure, I can put that PR together! I'll use the old React context API so it won't need to change the React dependency requirement.

[chandlerprall]

Sorry! Time flies when things get reprioritized. This is still on my list and I'm planning on working on the discussed changes next week.

[marissaokada]

Hi @davidtheclark, I'm working with @7mary4 on using this library for a large product's a11y features. We were wondering when the ETA for merging the PR out for this? I saw @chandlerprall 's fix and it looks exactly like the support we were looking for for trapping focus in our product. Thanks!

[chandlerprall]

@MarioKad I don't know what version of focus-trap-react you're on, but v4 (latest is 4.0.1) solved most of our problems. The one outstanding issue, which I created a PR for on focus-trap ( focus-trap/focus-trap#68 ), adds support to pause/unpause a chain of traps. I don't think my PR here ( #30 ) adds any value.

[marissaokada]

@chandlerprall Ah got it. Will update to 4.0.1 to see if that fixes our issue. Thank you!

[davidtheclark]

@chandlerprall Sorry for the delay — just needed to step back from the open source work for a bit. I reviewed the focus-trap PR and it looks great 👍 Have you tried it with focus-trap-react? I think we may want to verify that it works as expected when mounting & unmounting React components before we merge & release.

[chandlerprall]

No worries, take care of yourself! I'll give it a test within our React component library.

[theKashey]

@chandlerprall - try react-focus-lock, it's a bit more portals friendly, as long it uses React event system and focus events "bubbles" thought portals without issues.

Meanwhile, you may use react-dom-reflection to discover all portals within the parent node.