Reach-ui: Consider `portal={false}` default behavior for components with popovers

[chaance]

⭐ Enhancement

Current Behavior

All of our components with popovers currently default to opening in a portal. There are pros and cons to this approach.

AFAICT, the biggest pro is that you don't need to worry about inheritance or potential positioning weirdness with nearby elements. As far as cons:

• In some applications, tab order is still relevant and breaks when trying to move from a control into the popover. See https://github.com/reach/reach-ui/issues/284.
• If a user scrolls when a portal'd popover is open, the browser has to catch up and update its position on the fly. This creates a janky experience across the board.
• Some custom styling is much simpler when rendering the popover inline. See https://reacttraining.com/reach-ui/combobox#custom-styling

Desired Behavior + Suggested Solution

Consider making portal={false} default in affected components. (MenuButton, Combobox, Listbox)

Who does this impact? Who is this for?

All users, but the most significant impact is likely screen reader users.

[diegohaz]

In some applications, tab order is still relevant and breaks when trying to move from a control into the popover. See #284.

I guess this can be mitigated by introducing focus traps around the portaled content: https://twitter.com/diegohaz/status/1229837604977348612

I haven't dig into it that much, but I think it's worth a try.

[chaance]

@diegohaz We do trap focus in MenuButton, but in Listbox and Combobox we allow focus to leave the popover to close the popover and continue navigation. I'm on the fence about which direction to go with MenuButton and Listbox, as native behavior is inconsistent. But a focus trap might be better for that stated reason. For combobox I'm not sure we can trap focus because focus should typically stay on the input.

[diegohaz]

I mean, the focus trap would be used, in this case, to "transfer" focus to the correct element, and not to retain focus within the popover.

I've made some experiments here: https://codesandbox.io/s/reakit-portal-popover-tab-sequence-d4vb3

But it still doesn't work on iOS/VoiceOver as the swipe gesture doesn't really move DOM focus, so the onFocus handler on the traps is never called.

[chaance]

All good to know! I still need to spend some time with the swipe gesture, focus issues were more of a hypothesis than anything else. I'll definitely dig into your example, thanks for sharing 👍

[diegohaz]

No problems! There's also this video I recorded for Automattic which talks exactly about that: https://www.youtube.com/watch?v=tUlB13vUcac