Bubbling event from OutPortal

[alessandro308]

Follow-up from https://github.com/httptoolkit/react-reverse-portal/issues/13

It is a POC that the proposed solution actually works. Now we need to find a way to generalise the caught events. The main issue is to understand, without creating an explicit mapping, how to catch the add event listeners for every React event and then dispatch a new event to the node.element.

The events in React are quite a lot: https://reactjs.org/docs/events.html#supported-events

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: pimterry
:x: alessandro308
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[pimterry]

Hi @alessandro308, thanks this looks really promising!

I think there's one issue here though: it's taking React events (the events that React passes to React-managed event handlers like onClick) and passing them into the DOM event API (element.dispatchEvent).

These two events models aren't an exact match, and I don't think that's going to work for all cases. For example, React's onChange handlers are actually called by DOM input events, and many props on events are transformed slightly by React for compatibility between browsers.

I think we probably need to do this entirely underneath React, just using pure DOM events - i.e. using element.addEventListener rather than an onClick prop.

Can you test that out? If that works, it might also give us a solution for the "catch every event" problem, because that's apparently possible with the DOM but not with React.

[alessandro308]

The cons is that we need an extra div to catch the events...

[pimterry]

We do need a wrapper somewhere, I agree, and adding an extra div would be a breaking change that it'd definitely be good to avoid...

There is an existing wrapper div though, and I think we should be able to re-use that for this too. It's available as portalNode.element in most places (it's created and added to the portal node data here).

The way this works (as far as I can remember...) is:

• We manually create a wrapper div (or a <g> for SVGs) when a portal node is created (createHtmlPortalNode)
• When you pass the node to an InPortal, we use React's normal portals to render the InPortal children into that manually created wrapper div.
• When you pass the node to an OutPortal, we replace the contents of that OutPortal with the wrapper div (which therefore brings all the InPortal content with it).
• Because of this implementation, React's portal logic is currently listening to this div, capturing events there, and redirecting them back to the InPortal. We want to capture events here ourselves first, and then re-dispatch them on portalNode.element.parentNode instead.

To be honest it's been a long time since I touched this, but as far as I can tell that existing wrapper is the direct parent of the content at all times, and that's what we should be hooking into for this. Does that make sense?

[alessandro308]

I tried to implement the logic to catch all the events. I think it actually works, even without the wrapper using only the actual existing elements

[pimterry]

Sorry for the delay, this week has been kind of hectic, but I will look at this properly as soon as I can! I think realistically that'll probably be early next week now.

At first glance it all looks sensible to me, I just need to find some time to have a real play around with it and dig into the edge cases.

[Andynopol]

Is there any hope of seeing this implemented?

My main problem is if I have an onClick/onMouseDown/onMouseUp on a wrapper component like so:

<WrapperComponent onMouseDown={onMouseDown}>
   <OutPortal node={node} >
</WrapperComponent>

When I click on the WrapperComponent, the onMouseDown event triggers as expected, but if I trigger in the component placed by the portal, it does not. Even tho in the dom tree, it looks like it should...

[pimterry]

Hi @Andynopol, if you're interested, please feel free to investigate, I'd be very happy to merge a fix for this if possible.

See the comments above for the current state of this PR - there's still a little work & research required to find a working solution.