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

Overlay immediately closes with rootClose in shadow root #90

Open
angrycat9000 opened this issue Aug 8, 2023 · 8 comments · May be fixed by #91
Open

Overlay immediately closes with rootClose in shadow root #90

angrycat9000 opened this issue Aug 8, 2023 · 8 comments · May be fixed by #91

Comments

@angrycat9000
Copy link

angrycat9000 commented Aug 8, 2023

Describe the bug

Overlay with rootClose closes as soon as it is opened if it is hosted inside of a custom element shadow root.

I'm using custom elements to isolate some react based components that are hosted in an older application. The Overlay immediately closes when opened if it is hosted inside of a shadow root of a custom element. The same code works correctly when hosted in the light dom.

To Reproduce

Steps to reproduce the behavior:

  1. Create a custom element with a shadow root that hosts a react render root
  2. Render a Overlay with rootClose and onHide set. Also have the container option point to an element in the shadow root
  3. Click the button to open the overlay.

Reproducible Example

https://codesandbox.io/s/bold-dan-5kfh5r - Declares a custom element in index.js and has it render the same React component as is rendered in the light DOM. The custom element embedded version does not work, the light DOM version does.

  • Click the Show in shadow root button. Note the popover flashes and then disappears.
  • Click Show in light DOM button. Note that the popover appears and remains visibile.

Expected behavior

The popover remains open until an additional click.

Screenshots

Screencast.from.08-08-2023.10.04.38.AM.webm

Environment (please complete the following information)

  • Operating System: Linux
  • Browser, Version: Chrome 115.0.5790.110
    image

Additional context

This seems to be related to a change between React 17 and React 18. Was not able to reproduce it using React 17 or the legacy ReactDOM.render method in React 18.

@kyletsang
Copy link
Collaborator

The original workaround that @jquense added to fix this doesn't work in the shadow DOM because currentEvent is undefined

https://github.com/react-restart/ui/blob/44dc346744b37c33bd0d0309e5cd827751fce9c4/src/useClickOutside.ts#L104C9-L104C9

As a result, the click handler is processing twice

@kyletsang kyletsang linked a pull request Aug 9, 2023 that will close this issue
@jquense
Copy link
Member

jquense commented Aug 9, 2023

Funny I just saw that material UI has a different work around for the same issue in their ClickAway component, maybe we can try that?

@kyletsang
Copy link
Collaborator

Funny I just saw that material UI has a different work around for the same issue in their ClickAway component, maybe we can try that?

Unfortunately couldn't get tests to pass with the MUI approach. I'll need to drop this for now, since I'm short on time ATM.

@jquense
Copy link
Member

jquense commented Aug 17, 2023

fair enough, i don't think this is a super pressing use case anyway!

@mklekowski
Copy link

mklekowski commented May 29, 2024

Hi, any update? Do you need some help to make test passing? I'd like to make it work as soon as possible as the whole react-bootstrap is not working in the shadowDOM.

This issue is also visible for Dropdowns. In the shadow tree, dropdown is opened and immediately closed.

@angrycat9000
Copy link
Author

FYI @mklekowski I ended up working around this by not using rootClose and then adding my own event listeners for Escape and a click outside the overlay.

@mklekowski
Copy link

Actually, I have an issue with DropdownButton, when clicking two events are triggered, first for opening and second for closing. Probably it's also possible to disable rootClose and implement it manually, but I prefer to fix it in the library directly.

@jquense can you look at the issue?

@mklekowski
Copy link

Also, the code is using Window/event which is deprecated. Maybe it can be implemented in different way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants