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

Seems reverse portals are not passing events up the tree #13

Open
pie6k opened this issue Aug 4, 2020 · 6 comments
Open

Seems reverse portals are not passing events up the tree #13

pie6k opened this issue Aug 4, 2020 · 6 comments

Comments

@pie6k
Copy link

pie6k commented Aug 4, 2020

If I have some eg onClick events on component inside portal - this event will not be detected if used with reverse portal.

Eg. InPortal is rendered at the root level of the app

Then OutPortal is used inside some container that has it's own click events.

In such case - click events are only passed up directly to the root element, skipping container of OutPortal.

@pimterry
Copy link
Member

pimterry commented Aug 4, 2020

Interesting! I hadn't thought about this. For normal portals in React that's the correct & intended behaviour (see https://reactjs.org/docs/portals.html#event-bubbling-through-portals), but I'm not totally sure the same reasoning applies here.

In normal portals, the DOM-tree position is an implementation detail, and the React-tree position is the 'real' meaningful location of the component.

With reverse portals, I think you're right that the meaningful location is where the component is rendered though, and that's where events should fire. For similar reasons, you should never hear events from an InPortal component that's not rendered to any OutPortal anywhere on the page.

I'd be open to PRs to look at this. I'm not sure how hard this would be to change, or if there'll be other side-effects to doing so, so it'll take some careful investigation.

@alessandro308
Copy link

Do you have any idea on how we can try to achieve that goal? At the moment I don't have in mind any solution but rewrite the createPortal at all in some way...

@pimterry
Copy link
Member

pimterry commented Mar 2, 2022

I think we can do it without changing how the rendering happens, just by hooking into the events system, in theory at least.

We need to somehow listen for events that bubble up to the top of the portal contents, and then manually stop those propagating there through React (so they don't go back to the InPortal) and manually send them to the OutPortal parent instead.

As an example solution, what happens if we do something like:

  • Listen for all raw DOM events on the portal output DOM node (https://stackoverflow.com/a/48388878/68051)
  • Every time an event is fired:
    • Call event.stopPropagation() on the event
    • Call outPortalParentNode.dispatchEvent(event) to send the raw DOM event to the portal parent DOM node instead to continue propagating from there.

In theory it sounds like that would work to me, but I don't know much about the internals of how React tracks & handles events like this, so it needs a bunch of research & testing.

@alessandro308
Copy link

I actually made a POC and it works... Now we need to find a way to make it with all the possible events...
The cons of my implementation is the extra div I need to add as a parent catcher

@dimkajasons
Copy link

Do you have any progress here? I see the open PR, but there is nothing has changed since last year

@pimterry
Copy link
Member

Not really, no, and I'm afraid I don't have much time to investigate this myself right now. PRs very welcome, reading through the code & comments on the existing stalled PR #34 should be a good intro for anybody interested in implementing this.

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

No branches or pull requests

4 participants