Tabbing to a phantom element?

[suspiration]

I have this codesandbox: https://codesandbox.io/s/material-ui-accessibility-workaround-forked-gd7ok?file=/src/App.tsx

Once you've opened the dialog, there is a textfield and a button. I set FocusTrap inside the dialog around the textfield and button. When I tab between the elements, it goes to the textfield, then button, then to some phantom element before wrapping back to textfield. Any idea? Thanks in advanced!

[stefcameron]

Hi, @suspiration , thanks for reaching out! I check it out and the problem seems to be the fragment you have as the child of the FocusTrap. The "phantom" element is the body itself, which gets focused after the close button, so the trap is effectively not working in this case. I haven't debugged it, but I think you end-up in the silent "else" clause of this IF statement, which means the focus trap doesn't even get created.

If you change your fragment for a <div>...</div>, it works great.

Fragments are special, and FocusTrap expects a single child element. Clearly, the fragment satisfies all the conditions of a single child in the render() function, but I think we end-up with [undefined] here: https://github.com/focus-trap/focus-trap-react/blob/master/src/focus-trap-react.js#L158

And then it would explain why https://github.com/focus-trap/focus-trap-react/blob/master/src/focus-trap-react.js#L75 ends-up with nodesExist === false and we never setup the trap.

First thought is FocusTrap could throw at line 158 if element === undefined, indicating a fragment, and we just don't support that (since we never have anyway).

Otherwise, FocusTrap does support multiple container elements, but that's not what we have here inside the fragment. We just have multiple "leaf" children. Nothing to build a focus trap around. So we'd have to maybe look up for the nearest parent and use that as the container, but I'm not sure what consequences there may be out in the wild.

I'm leaning on the side of throwing an exception to point out that fragments aren't supported.

If you have any thoughts, please share!

[suspiration]

Thanks @stefcameron that worked!

I think throwing an error and adding in documentation that fragments cannot be used as a single child element would be helpful for other users who run into the same issue. I'm going to leave this open for you to add the changes against, unless you want it closed.

[stefcameron]

Thanks @stefcameron that worked!

My pleasure!

I think throwing an error and adding in documentation that fragments cannot be used as a single child element would be helpful for other users who run into the same issue. I'm going to leave this open for you to add the changes against, unless you want it closed.

👍

[stefcameron]

Fixing with #269