Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TrapFocus] Steals focus from nested portal #15694

Closed
2 tasks done
alexkirsz opened this issue May 14, 2019 · 20 comments · Fixed by #21610
Closed
2 tasks done

[TrapFocus] Steals focus from nested portal #15694

alexkirsz opened this issue May 14, 2019 · 20 comments · Fixed by #21610
Labels
bug 🐛 Something doesn't work component: FocusTrap The React component.

Comments

@alexkirsz
Copy link
Contributor

alexkirsz commented May 14, 2019

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

Expected Behavior 🤔

TrapFocus ensures focus is kept on elements within its React element spanning tree.

Current Behavior 😯

Since TrapFocus keeps the focus on elements within its DOM hierarchy, focusable elements within its React hierarchy but outside its DOM hierarchy (e.g. inside of a Popper or a portal) can't be focused.

Steps to Reproduce 🕹

Example:
https://codesandbox.io/s/llv0m7r5jq

Click on the [Open Dialog] button, then on the [Open Popper] button. In the console, you'll see that the TextField within the Popper is focused (as it should since it's autoFocus), then instantly blurred back by the TrapFocus.

Context 🔦

I'm building an autocomplete component with a "popout" behaviour similar to the one demonstrated at the bottom of react-select's Advanced doc.

The autocomplete button itself is rendered within a dialog, and I'm using a popper to display the text input and options. Right now I'm disabling the portal behaviour so it's kept inside the modal's DOM hierarchy, but this causes issues with the select's option list overflowing from the modal. My final solution seems very hacky and prone to unexpected failure, so I'd love this to be handled by MUI itself.

Your Environment 🌎

For the stack, see codesandbox.

Tech Version
Browser Chrome 74.0.3729.131

Duplicates

@alexkirsz alexkirsz changed the title [Modal] TrapFocus steals focus from nested Popper [Modal] TrapFocus steals focus from nested portal May 14, 2019
@jeremy-coleman
Copy link

jeremy-coleman commented May 14, 2019

Try a html dialog mayb?

@eps1lon eps1lon added bug 🐛 Something doesn't work component: modal This is the name of the generic UI component, not the React module! labels May 14, 2019
@oliviertassinari
Copy link
Member

@alexkirsz The behavior you are describing sounds correct to me. TrapFocus only cares about the DOM structure. The logic isn't applied at the React level. I'm not sure the upcoming FocusScope component in React handles this case either: facebook/react#15487.

I would challenge your problem in the first place. Why do you need to render the input outside of the dialog? Did you consider using a Popover instead of a Popper? This component uses FocusTrap internally. I will handle the case correctly.

This makes me think of a #15450 (comment) where the problem is about interoperability. We have a couple of ways to work around the problem. I'm interested in finding the best option.

@oliviertassinari oliviertassinari removed their assignment May 15, 2019
@oliviertassinari oliviertassinari removed the bug 🐛 Something doesn't work label May 15, 2019
@alexkirsz
Copy link
Contributor Author

alexkirsz commented May 15, 2019

@alexkirsz The behavior you are describing sounds correct to me. TrapFocus only cares about the DOM structure. The logic isn't applied at the React level. I'm not sure the upcoming FocusScope component in React handles this case either: facebook/react#15487.

@oliviertassinari Thanks for the FocusScope link, I wasn't aware of the ongoing efforts with regards to focus handling in React!

elements nested in portals will traverse as expected via the React fiber tree rather than the DOM tree.

This certainly sounds like it would fix my particular issue! :)

I would challenge your problem in the first place. Why do you need to render the input outside of the dialog? Did you consider using a Popover instead of a Popper? This component uses FocusTrap internally. I will handle the case correctly.

I'm having to apply some custom positioning logic as I want the dropdown to extend to the bottom of the window, but no further. Since I believe both Modal and DialogContent define an overflowY, the dropdowns ends up half-hidden and causes a scrollbar to appear. Popper.js also doesn't have an option for height constraints so I implemented my own modifier instance. I think I initially tried with a Popover, but I can't remember why I would have moved away so I'll probably end up trying it again.

FYI, the component currently looks like this:

image

The option list uses react-virtualized under the hood since I often need to display very large lists of items.

@oliviertassinari
Copy link
Member

oliviertassinari commented May 18, 2019

@alexkirsz Thanks for providing more details. Looking at the visual output. I believe you should be using the Popper component for rendering the list of items, not the input element. So I think that we can close the issue.

Regarding the component you have built, it something Material-UI should provide directly. I don't think that we should ask our users to do it every single time. We will work on providing a better solution. If you can share the source, once it meets your expectation, it would be awesome :).

@alexkirsz
Copy link
Contributor Author

@oliviertassinari Do you mean replacing the “Select engine” button with the TextField? Thing is, the TextField is only used for filtering between the different options, but its value doesn’t fully represent an option. Hence the “popout” behaviour.

@oliviertassinari
Copy link
Member

I'm sorry, I don't follow you. My points were 1. Your codesandbox can be solved with a different approach. 2. Going forward, Material-UI, as a library, wants to support this problem with a built-in component.

@alexkirsz
Copy link
Contributor Author

I was referring to the following:

Looking at the visual output. I believe you should be using the Popper component for rendering the list of items, not the input element.

The specifics of the component require the TextField component to be nested within a portal, to achieve the "popout" behaviour wherein you can click on a button and a popper will appear with both a filter input and the filtered option list. So I don't see another way of achieving the same visual output without rendering both components within a portal :/

@oliviertassinari
Copy link
Member

Did you try this Popper demo https://next.material-ui.com/components/autocomplete/#downshift?

@alexkirsz
Copy link
Contributor Author

I tried both react-select and downshift, but I had issues with customizing my component in both. The biggest issue I had with downshift was the fact that it was re-rendering the whole component on every single state change, which caused serious performance issues with large lists.

@oliviertassinari
Copy link
Member

@alexkirsz Interesting.

@oliviertassinari oliviertassinari added component: Autocomplete and removed component: modal This is the name of the generic UI component, not the React module! labels May 18, 2019
@verbart
Copy link

verbart commented Jul 24, 2019

Have same problem - form fields doesn't have focus when use it in Popper has opened from Dialog

@oliviertassinari

This comment has been minimized.

@verbart

This comment has been minimized.

@verbart
Copy link

verbart commented Jul 24, 2019

If Popper has disablePortal prop, form fields inside it works good

@verbart
Copy link

verbart commented Jul 24, 2019

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: modal This is the name of the generic UI component, not the React module! component: Popper The React component. See <Popup> for the latest version. and removed component: Autocomplete labels Jul 24, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 24, 2019

@verbart Thanks, it looks like the same problem than the author of this issue has. disabledPortal allows to partially work around the problem, as well as disableEnforceFocus.

Do we have a better solution than to wait for the release of the <FocusScope> component?

elements nested in portals will traverse as expected via the React fiber tree rather than the DOM tree.

facebook/react#15487

@verbart
Copy link

verbart commented Jul 24, 2019

@oliviertassinari Thanks, did not know about the "disableEnforceFocus" property. While it best saves the situation.

@oliviertassinari oliviertassinari added component: Portal The React component. and removed component: Popper The React component. See <Popup> for the latest version. labels Aug 3, 2019
@emirotin
Copy link

Sorry if wrong issue but I think what I'm experiencing is related.

We have a Drawer (which renders Modal -> Portal -> TrapFocus).
In the Drawer we have a link that opens LiveChat dialog (technically it's an iframe rendered in an absolutely positioned div).
Now it's impossible to type in the LiveChat inputs because the focus is instantly stolen. If I close the drawer then LiveChat works just fine.

@eps1lon
Copy link
Member

eps1lon commented Sep 25, 2019

@emirotin That sounds like #17497

@itse4elhaam
Copy link

Sorry if wrong issue but I think what I'm experiencing is related.

We have a Drawer (which renders Modal -> Portal -> TrapFocus). In the Drawer we have a link that opens LiveChat dialog (technically it's an iframe rendered in an absolutely positioned div). Now it's impossible to type in the LiveChat inputs because the focus is instantly stolen. If I close the drawer then LiveChat works just fine.

Facing a similar issue, any fixes?

shadcn-ui/ui#2247

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: FocusTrap The React component.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants