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

Input in Portal can't be focused inside Dialog #922

Closed
yyz945947732 opened this issue Oct 19, 2021 · 11 comments
Closed

Input in Portal can't be focused inside Dialog #922

yyz945947732 opened this issue Oct 19, 2021 · 11 comments
Labels
Has Workaround This issue isn't fixed but has a workaround Type: 3rd Party Compatibility This issue is to do with compatibility with another library

Comments

@yyz945947732
Copy link

yyz945947732 commented Oct 19, 2021

https://codesandbox.io/s/focused-platform-2itl5?file=/App.js

A element which use Portal inside Dialog will cant't be focused and have style with 'pointer-events : none'. What can i do to solve it?

<Dialog.Root>
  <Dialog.Content>
    <Portal>
      <input />
    </Portal>
  </Dialog.Content>
</Dialog.Root>
@jjenzz
Copy link
Contributor

jjenzz commented Oct 20, 2021

Hi @yyz945947732 👋

Please can you help us understand what your use-case is? Why do you need to portal an input out of the Dialog.Content like this?

@yyz945947732
Copy link
Author

Hi, thanks for the reply.

I want to render a custom Popover component inside Dialog. This Popover is portal to document.body and have an input inside. I want to display the input and enter something when the Popover is displayed, But the focus is traped in Dialog. I tried to wrap the Popover with <DismissableLayer />, but it didn't work.

Is there any way to prevent the focus of elements outside the dialog that uses the portal from being trapped?

@benoitgrelard
Copy link
Collaborator

Is there any particular reason it needs to be a "custom" popover components and you can't use radix's Popover?

@yyz945947732
Copy link
Author

I do plan to refactor this component with radix, but it will take time, so I want to find a temporary solution. So is it appropriate to say that it is not the best practice to use radix with other component libraries, because it may cause some weird problems? At present, to solve this problem can only use radix's popover?

@jjenzz
Copy link
Contributor

jjenzz commented Oct 20, 2021

A modal dialog will not allow focus to escape its content and since your custom portal is outside of the content, attempting to tab to it will be prevented. Our Popover allows this as we have many moving parts that we handle for you (off the top of my head, I'm not sure what moving parts are enabling it to work in our case but @benoitgrelard might know).

There are a couple things you can do for now to get this some of the way there.

  • Set modal={false} on the Dialog.Root
  • Programatically focus the input in your popover when it opens

Example: https://codesandbox.io/s/relaxed-joliot-mu48b?file=/src/App.js

Or:

  • Set the containerRef prop on the Portal to keep it inside the Dialog.Content

Example: https://codesandbox.io/s/staging-bird-pkglq?file=/src/App.js

Or don't use the Portal and position the popover relative to the trigger.


@benoitgrelard I am wondering though why our FocusScope is preventing focus to something in the same React Tree. I would have thought we wouldn't prevent this case, even in modal mode. That might be a fix we can look into?

@benoitgrelard
Copy link
Collaborator

Yes, it's because we allow other focus scopes to take focus, this is done via a stack we manage internally.

@jjenzz
Copy link
Contributor

jjenzz commented Oct 20, 2021

Instead of only allowing other focus scopes to take focus though, could we allow anything in the same React tree to take focus?

@benoitgrelard
Copy link
Collaborator

We don't really specifically allow other focus scopes to take focus, it's rather that when a new focus scope mounts anywhere, it will deactivate the one that's active, and take over, then when it unmounts, the previous one will be reactivated.

The reason it's not working within the react tree is because the code that handles containment is using dom events + .contains. I believe we looked into trying to do it in the react tree but it's quite complex.

We could take another look though.

@yyz945947732
Copy link
Author

yyz945947732 commented Oct 20, 2021

Set the containerRef prop on the Portal to keep it inside the Dialog.Content

I have tried this before like:

// Popover.tsx
const containerObjectRef = React.useRef<HTMLElement>(document.body);

React.useLayoutEffect(() => {
  const triggerParentElement = triggerDOMElement?.parentElement;
  if (triggerParentElement) {
    containerObjectRef.current = triggerParentElement;
  }
}, [triggerDOMElement]);

return <Portal containerRef={containerObjectRef}>{content}</Portal>

but finally gave up this solution because it would cause zindex problems. Since it is not mounted on document.body, it may be covered by some child elements of the mounted container because of the natural stacking order on the webpage.

Example: https://codesandbox.io/s/hardcore-goldstine-bdllk?file=/App.js

Set modal={false} on the Dialog.Root

I'm sorry. Set modal={true} is necessary in my case.

BTW, thank you for the fast response and for taking a deeper look at this issue.

@jjenzz
Copy link
Contributor

jjenzz commented Oct 21, 2021

I'm sorry. Set modal={true} is necessary in my case.

BTW, thank you for the fast response and for taking a deeper look at this issue.

No worries. I'm not sure of an alternative workaround in the meantime in that case so we can look into React tree support when we have some time. Thanks for taking the time to help us understand the issue 🙂


EDIT: I just thought though, you could use our FocusScope and DismissableLayer components to get this working manually yourself in the meantime: https://codesandbox.io/s/thirsty-jang-quxe4?file=/src/App.js

The FocusScope is the component that allows the focus to escape and the DismissableLayer enables escape keypress to close, along with pointer down outside etc. while maintaining correct layering. So, escape key once will close your popover and escape key again will close the dialog.

@yyz945947732
Copy link
Author

Thank you so much.

And I have seen similar problems with other libraries recently, I don't know if these can give some help:

https://github.com/theKashey/react-focus-lock/blob/master/src/Trap.js#L144-L156
https://github.com/mui-org/material-ui/pull/21610/files

There may be an solution here is if <FocusScope /> children's React event's currentTarget does not contain target, then it can be treated as a portal.

But I know that the way radix manages focus is more complicated, because it supports more features than other libraries. So I don’t know if these can help a bit.

@benoitgrelard benoitgrelard added Type: 3rd Party Compatibility This issue is to do with compatibility with another library Has Workaround This issue isn't fixed but has a workaround labels Apr 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Workaround This issue isn't fixed but has a workaround Type: 3rd Party Compatibility This issue is to do with compatibility with another library
Projects
None yet
Development

No branches or pull requests

3 participants