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

createPortal: support option to stop propagation of events in React tree #11387

Open
kib357 opened this issue Oct 27, 2017 · 121 comments
Open

createPortal: support option to stop propagation of events in React tree #11387

kib357 opened this issue Oct 27, 2017 · 121 comments

Comments

@kib357
Copy link

kib357 commented Oct 27, 2017

Do you want to request a feature or report a bug?
Feature, but also a bug cause new API breaks old unstable_rendersubtreeintocontainer

What is the current behavior?
We cannot stop all events propagation from portal to its React tree ancestors. Our layers mechanism with modals/popovers completely broken. For example, we have a dropdown button. When we click on it, click opens popover. We also want to close this popover when clicking on same button. With createPortal, click inside popover fires click on button, and it's closing. We can use stopPropagation in this simple case. But we have tons of such cases, and we need use stopPropagation for all of them. Also, we cannot stop all events.

What is the expected behavior?
createPortal should have an option to stop synthetic events propagation through React tree without manually stopping every event. What do you think?

@kib357
Copy link
Author

kib357 commented Oct 27, 2017

Also, propagation of mouseOver/Leave looks completely unexpected.
image

@gaearon
Copy link
Collaborator

gaearon commented Oct 27, 2017

Can you move portal outside of the button?

e.g.

return [
  <div key="main">
    <p>Hello! This is first step.</p>
    <Button key="button" />
  </div>,
  <Portal key="portal" />
];

Then it won't bubble through the button.

@kib357
Copy link
Author

kib357 commented Oct 28, 2017

It was my first thought, but!) Imagine, that we have mouseEnter handler in such component container:

image

With unstable_rendersubtreeintocontainer i need nothing to do with events in ButtonWithPopover component – mouseEnter simply works when mouse really enters div and button DOM element, and not fired when mouse is over popover. With portal, event fires when mouse over popover – and actually NOT over div in this moment. So, i need to stop propagation. If i do it in ButtonWithPopover component, i will break event firing when mouse is over button. If i do it in popover and i'm using some common popover component for this application, i also can break logic in other app parts.

I really don't understand purpose of bubbling through React tree. If i need events from portal component – i simply can pass handlers through props. We were do it with unstable_rendersubtreeintocontainer and it worked perfect.

If i will open a modal window from some button deep in react tree, i will receive unexpected firing of events under modal. stopPropagation will also stop propagation in DOM, and i will not get events that i really expect to be fired(

@craigkovatch
Copy link

craigkovatch commented Jan 3, 2018

@gaearon I would suggest this is more of a bug than a feature request. We have a number of new bugs caused by mouse events bubbling up through portals (where we were previously using unstable_rendersubtreeintocontainer). Some of these can't be fixed even with an extra div layer to filter mouse events because e.g. we rely on mousemove events propagating up to the document to implement draggable dialogs.

Is there a way to workaround this before this is addressed in a future release?

@jquense
Copy link
Contributor

jquense commented Jan 3, 2018

I think it's being called a feature request, because the current bubble behavior of portals is both expected and intended. The goal is that subtree act like real child of their parents.

What would be helpful is additional use cases or situations (like the ones you're seeing) that you don't feel are served by the current implementation, or are difficult to workaround

@craigkovatch
Copy link

I understand that this behavior is intended, but I think it's a significant bug that it's not disable-able.

@neytema
Copy link

neytema commented Jan 3, 2018

In my mind library working with DOM should preserve DOM implementation behavior not break it.

For example:

class Container extends React.Component {
  shouldComponentUpdate = () => false;
  render = () => (
    <div
      ref={this.props.containerRef}
      // Event propagation on this element not working
      onMouseEnter={() => { console.log('handle mouse enter'); }}
      onClick={() => { console.log('handle click'); }}
    />
  )
}

class Root extends React.PureComponent {
  state = { container: null };
  handleContainer = (container) => { this.setState({ container }); }

  render = () => (
    <div>
      <div
        // Event propagation on this element not working also
        onMouseEnter={() => { console.log('handle mouse enter'); }}
        onClick={() => { console.log('handle click'); }}
      >
        <Container containerRef={this.handleContainer} />
      </div>
      {this.state.container && ReactDOM.createPortal(
        <div>Portal</div>,
        this.state.container
      )}
    </div>
  );
}

When I work with DOM, I expect to receive events like DOM implementation does it. In my example events are propagated through Portal, working around it's DOM parents, and this can be considered as a bug.

@jquense
Copy link
Contributor

jquense commented Jan 3, 2018

Folks thanks for the discussion, however I don't think it's all that helpful to argue whether something is a bug or not. Instead i'd be more productive to discuss the use cases and examples that are not met by the current behavior, so we can better understand if the current way is the best way for the future.

In general we want the API to handle a diverse set of use-cases while hopefully not overly limiting others. I can't speak for the core team, but I'd imagine that making it configurable is not a likely solution. Generally React leans for a consistent api over configurable ones.

I also understand that this behavior is not how the DOM works, but i don't think that's in itself a good reason to say it shouldn't be that way. Lots of react-dom's behavior is different from how the DOM works, may events are already different from the native version. onChange for instance is completely unlike the native change event, and all react events bubble regardless of type, unlike the DOM.

@craigkovatch
Copy link

craigkovatch commented Jan 3, 2018

Instead i'd be more productive to discuss the use cases and examples that are not met by the current behavior

Here's two examples that are broken for us in our migration to React 16.

First, we have a draggable dialog which is launched by a button. I attempted to add a "filtering" element on our Portal use which called StopPropagation on any mouse* an key* events. However, we rely on being able to bind a mousemove event to the document in order to implement the dragging functionality -- this is common because if the user moves the mouse at any significant rate, the cursor leaves the bounds of the dialog and you need to be able to capture the mouse movement at a higher level. Filtering these events breaks this functionality. But with Portals, the mouse and key events are bubbling up from inside the dialog to the button that launched it, causing it to display different visual effects and even dismiss the dialog. I don't think it's realistic to expect every component that will be launched via a Portal to bind 10-20 event handlers to stop this event propagation.

Second, we have a popup context menu which can be launched by either a primary- or secondary-mouse click. One of the internal consumers of our library has mouse handlers attached to the element that launches this menu, and of course the menu also has click handlers for handling item selection. The menu is now reappearing on every click as the mousedown/mousedown events are bubbling back up to the button that launches the menu.

I can't speak for the core team, but I'd imagine that making it configurable is not a likely solution. Generally React leans for a consistent api over configurable ones.

I implore you (and the team) to reconsider this position in this particular case. I think event bubbling will be interesting for certain use cases (although I can't think of any offhand). But I think it will be crippling in others, and it introduces significant inconsistency in the API. While unstable_rendersubtreeintocontainer was never super-supported, it was what everyone used to render outside of the immediate tree, and it didn't work this way. It was officially deprecated in favor of Portals, but Portals break the functionality in this critical way, and there doesn't seem to be an easy workaround. I think this can be fairly described as quite inconsistent.

I also understand that this behavior is not how the DOM works, but i don't think that's in itself a good reason to say it shouldn't be that way.

I understand where you're coming from here, but I think in this case (a) it's a fundamental behavior which (b) currently has no workaround, so I think "the DOM doesn't work this way" is a strong argument, if not a completely convincing one.

@craigkovatch
Copy link

And to be clear: my request that this be considered a bug is mostly so that it gets prioritized for a fix sooner rather than later.

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Jan 4, 2018

My mental model of a Portal is that it behaves as if it is on the same place in the tree, but avoids problems such as "overflow: hidden" and avoids scrolling for drawing/layout purposes.

There are many similar "popup" solutions that happen inline without a Portal. E.g. a button that expands a box right next to it.

Take as an example the "Pick your reaction" dialog here on GitHub. That is implemented as a div right next to the button. That works fine now. However, if it wants to have a different z-index, or be lifted out of an overflow: scroll area that contains these comments, then it will need to change DOM position. That change is not safe to do unless other things like event bubbling is also preserved.

Both styles of "popups" or "pop outs" are legit. So how would you solve the same problem when the component is inline in the layout as opposed to floating outside of it?

@methyl
Copy link

methyl commented Jan 4, 2018

The workaround that worked for me is calling stopPropagation directly under portal rendering:

return createPortal(
      <div onClick={e => e.stopPropagation()}>{this.props.children}</div>,
      this.el
    )

That works great for me since I have single abstraction component that uses portals, otherwise you will need to fix up all your createPortal calls.

@craigkovatch
Copy link

craigkovatch commented Jan 4, 2018

@methyl this assumes you know every event that you need to block from bubbling up the tree. And in the case I mentioned with draggable dialogs, we need mousemove to bubble up to document, but not to bubble up the render tree.

@craigkovatch
Copy link

craigkovatch commented Jan 4, 2018

Both styles of "popups" or "pop outs" are legit. So how would you solve the same problem when the component is inline in the layout as opposed to floating outside of it?

@sebmarkbage I'm not sure this question makes sense. If I had this problem inlining the component, I wouldn't inline it.

@jquense
Copy link
Contributor

jquense commented Jan 4, 2018

I think some of problem here is the some use cases of renderSubtreeIntoContainer are being ported to createPortal when the two methods are doing conceptually different things. The concept of Portal was being overloaded I think.

I agree that in the Modal dialog case, you almost never want the modal to act like a child of the button that opened it. The trigger component is only rendering it because it controls the open state. I think tho it's a mistake to say that the portal implementation is therefore wrong, instead of saying that createPortal in the button is not the right tool for this. In this case the Modal is not a child of the trigger, and shouldn't be rendered as if it were. One possible solution is to keep using renderSubtreeIntoContainer, another user-land option is to have a ModalProvider near the app root that handles rendering modals, and passes down (via context) a method to render an arbitrary modal element need to the root

@craigkovatch
Copy link

craigkovatch commented Jan 4, 2018

renderSubtreeIntoContainer can't be called from inside of render or lifecycle methods in React 16, which pretty much precludes its use for the cases I've been discussing (in fact, all our components which were doing this completely broke in the migration to 16). Portals are the official recommendation: https://reactjs.org/blog/2017/09/26/react-v16.0.html#breaking-changes

I agree that the concept of Portals might have ended up overloaded. Not sure I love the solution of a global component and context for it, though. It seems like this could be easily solved by a flag on createPortal specifying whether events should bubble through. It would be an opt-in flag which would preserve API compatibility with 16+.

@kib357
Copy link
Author

kib357 commented Jan 4, 2018

I will try to clarify our use-case of the portals and why we would love to see an option for stopping events propagation. In ManyChat app, we are using portals to create a 'layers'. We have the layer system for the whole app that used by several types of components: popovers, dropdowns, menus, modals. Every layer can expose a new layer, e.g. button on a second level of menu can trigger the modal window where the other button can open the popover. In most cases layer is the new branch of UX that solves it's own task. And when new layer is open, the user should interact with this new layer, not with others in bottom. So, for this system, we've created a common component for rendering to layer:

class RenderToLayer extends Component {
  ...
  stop = e => e.stopPropagation()

  render() {
    const { open, layerClassName, useLayerForClickAway, render: renderLayer } = this.props

    if (!open) { return null }

    return createPortal(
      <div
        ref={this.handleLayer}
        style={useLayerForClickAway ? clickAwayStyle : null}
        className={layerClassName}
        onClick={this.stop}
        onContextMenu={this.stop}
        onDoubleClick={this.stop}
        onDrag={this.stop}
        onDragEnd={this.stop}
        onDragEnter={this.stop}
        onDragExit={this.stop}
        onDragLeave={this.stop}
        onDragOver={this.stop}
        onDragStart={this.stop}
        onDrop={this.stop}
        onMouseDown={this.stop}
        onMouseEnter={this.stop}
        onMouseLeave={this.stop}
        onMouseMove={this.stop}
        onMouseOver={this.stop}
        onMouseOut={this.stop}
        onMouseUp={this.stop}

        onKeyDown={this.stop}
        onKeyPress={this.stop}
        onKeyUp={this.stop}

        onFocus={this.stop}
        onBlur={this.stop}

        onChange={this.stop}
        onInput={this.stop}
        onInvalid={this.stop}
        onSubmit={this.stop}
      >
        {renderLayer()}
      </div>, document.body)
  }
  ...
}

This component stops propagation for all event types from React docs, and it allowed us to update to React 16.

@jacobp100
Copy link

Does this need to be tied to portals? Rather than sandboxing portals, what if there was just a (for example) <React.Sandbox>...</React.Sandbox>?

@craigkovatch
Copy link

Even that seems needlessly complex to me. Why not simply add an optional boolean flag to createPortal allowing the bubbling behavior to be blocked?

@craigkovatch
Copy link

@gaearon this is a pretty unfortunate situation for a certain slice of us -- could you or someone dear to you have a look at this? :)

@jquense
Copy link
Contributor

jquense commented Jan 25, 2018

I'd add that my current thinking is that both use cases should be supported. There are really use cases where you need context to flow from the current parent to the subtree but to not have that subtree act as a logical child in terms of the DOM. Complex modals are the best example, you just almost never want the events from a form in a modal window to propagate up to the trigger button, but almost certainly need the context passed through (i18n, themes, etc)

I will say that that usecase could be mostly solved with a ModalProvider closer to the app root that renders via createPortal high enough that event propagation doesn't affect anything, but that starts to feel like a workaround as opposed to a well designed architecture. It also makes library provided modals more annoying for users since they are no longer self contained.

@jquense
Copy link
Contributor

jquense commented Jan 25, 2018

i would add tho in terms of API i don't think createPortal should do both, the modal case really wants to just use ReactDOM.render (old skool) because it's pretty close to a distinct tree except that context propagation is often needed

@craigkovatch
Copy link

We just had to fix an extremely difficult-to-diagnose bug in our outer application's focus management code as a result of using the workaround that @kib357 posted.

Specifically: calling stopPropagation on the synthetic focus event to prevent it from bubbling out of the portal causes stopPropagation to also be called on the native focus event in React's captured handler on #document, which meant it did not make it to another captured handler on <body>. We fixed by moving our handler up to #document, but we had specifically avoided doing that in the past so as not to step on React's toes.

The new bubbling behavior in Portals really feels like the minority case to me. Be that opinion or truth, could we please get some traction on this issue? Maybe @gaearon? It's four months old and causing real pain. I think this could fairly be described as a bug given that it's a breaking API change in React 16 with no completely-safe workaround.

@sebmarkbage
Copy link
Collaborator

@craigkovatch I'm still curious how you would solve my inline example. Let's say the popup is pushing the size of the box down. Inlining something is important since it is pushing something down in the layout given its size. It can't just hover over.

You could potentially measure the popover, insert a blank placeholder with the same size and try to align it on top but that's not what people do.

So if your popover need to expand the content in place, like right next to the button, how would you solve it? I suspect that the pattern that works there, will work in both cases and we should just recommend the one pattern.

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Feb 16, 2018

I think in general this is the pattern that works in both scenarios:

class Foo extends React.Component {
  state = {
    highlight: false,
    showFlyout: false,
  };

  mouseEnter() {
    this.setState({ highlight: true });
  }

  mouseLeave() {
    this.setState({ highlight: false });
  }

  showFlyout() {
    this.setState({ showFlyout: true });
  }

  hideFlyout() {
    this.setState({ showFlyout: false });
  }

  render() {
    return <>
      <div onMouseEnter={this.mouseEnter} onMouseLeave={this.mouseLeave} className={this.state.highlight ? 'highlight' : null}>
        Hello
        <Button onClick={this.showFlyout} />
      </div>
      {this.state.showFlyout ? <Flyout onHide={this.hideFlyout} /> : null}
    </>;
  }
}

If the Flyout is a portal, then it works and it doesn't get any mouse over events when hovering over the portal. But more importantly, it also works if it is NOT a portal, and it needs to be an inline flyout. No stopPropagation needed.

So what is it about this pattern that doesn't work for your use case?

@craigkovatch
Copy link

@sebmarkbage we are using Portals in a completely different fashion, rendering into a container mounted as the final child of <body> which is then positioned, sometimes with a z-index. The React documentation suggests this is closer to the design intention; i.e. rendering into a totally different place in the DOM. It doesn't seem to me that our use cases are similar enough for discussion to belong on this thread. But if you want to brainstorm/troubleshoot together, I'd be more than happy to discuss further in another forum.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 9, 2022
@justjake
Copy link

justjake commented Jan 9, 2022

Bump.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 9, 2022
@dcporter
Copy link

dcporter commented Jan 9, 2022

Bump!

@pie6k
Copy link

pie6k commented Mar 18, 2022

My use case is quite extreme and exotic, but I'd really want to disable react portal event bubbling:

TLDR: I use react portal to render multiple windows in Electron app

Use case:

  • electron App
  • we have multiple windows
  • new window is created with const newWindow = window.open()
  • we create react portal to this new window createPortal(<Stuff />, newWindow.document.body)
  • we render content from main window to this new window via portal
  • this has a lot of advantages as we can have declarative approach to multi-window app, so you can just do <NewElectronWindow width={...}><Stuff />, having 1 JS thread while having multiple windows, shared data between windows (like mobx), etc
  • event happening in new window gets bubbled to main window, which is totally exotic, as NewElectronWindow parents in React tree gets events that happened in different window creating quite 'unusual' edge-cases (like console.warn showing no stack trace)

@deniskabana
Copy link

Bump... I have a few modal windows I am managing from inside a huge container that has a lot of mouse events (drag, click, etc.) that I know nothing of at runtime. Can not stop event bubbling on unknown/all events...

@princ3od
Copy link

princ3od commented Feb 4, 2023

The issue still appears today. I can not stop propagation mouseeenter event from children to their parent.

@pie6k
Copy link

pie6k commented Feb 5, 2023

I agree. Portals IMO should have some sort of bubblingMode option with 3 modes: follow Dom, follow react, don't follow.

I use React portal to render stuff into another window (window.open()) and bubbling events in this case is really not needed/expected

@craigkovatch
Copy link

craigkovatch commented Feb 5, 2023

Bubbling across portal being opt-in would have been much better

@devongovett
Copy link
Contributor

devongovett commented May 16, 2023

We ran across this issue yet again in React Aria. Seriously considering adding this to all of our overlay components to hopefully solve it once and for all. 😐

function Portal({children, container}) {
  let element = (
    <div
      onCopy={e => e.stopPropagation()}
      onCopyCapture={e => e.stopPropagation()}
      onCut={e => e.stopPropagation()}
      onCutCapture={e => e.stopPropagation()}
      onPaste={e => e.stopPropagation()}
      onPasteCapture={e => e.stopPropagation()}
      onCompositionEnd={e => e.stopPropagation()}
      onCompositionEndCapture={e => e.stopPropagation()}
      onCompositionStart={e => e.stopPropagation()}
      onCompositionStartCapture={e => e.stopPropagation()}
      onCompositionUpdate={e => e.stopPropagation()}
      onCompositionUpdateCapture={e => e.stopPropagation()}
      onFocus={e => e.stopPropagation()}
      onFocusCapture={e => e.stopPropagation()}
      onBlur={e => e.stopPropagation()}
      onBlurCapture={e => e.stopPropagation()}
      onChange={e => e.stopPropagation()}
      onChangeCapture={e => e.stopPropagation()}
      onBeforeInput={e => e.stopPropagation()}
      onBeforeInputCapture={e => e.stopPropagation()}
      onInput={e => e.stopPropagation()}
      onInputCapture={e => e.stopPropagation()}
      onReset={e => e.stopPropagation()}
      onResetCapture={e => e.stopPropagation()}
      onSubmit={e => e.stopPropagation()}
      onSubmitCapture={e => e.stopPropagation()}
      onInvalid={e => e.stopPropagation()}
      onInvalidCapture={e => e.stopPropagation()}
      onLoad={e => e.stopPropagation()}
      onLoadCapture={e => e.stopPropagation()}
      onError={e => e.stopPropagation()}
      onErrorCapture={e => e.stopPropagation()}
      onKeyDown={e => e.stopPropagation()}
      onKeyDownCapture={e => e.stopPropagation()}
      onKeyPress={e => e.stopPropagation()}
      onKeyPressCapture={e => e.stopPropagation()}
      onKeyUp={e => e.stopPropagation()}
      onKeyUpCapture={e => e.stopPropagation()}
      onAbort={e => e.stopPropagation()}
      onAbortCapture={e => e.stopPropagation()}
      onCanPlay={e => e.stopPropagation()}
      onCanPlayCapture={e => e.stopPropagation()}
      onCanPlayThrough={e => e.stopPropagation()}
      onCanPlayThroughCapture={e => e.stopPropagation()}
      onDurationChange={e => e.stopPropagation()}
      onDurationChangeCapture={e => e.stopPropagation()}
      onEmptied={e => e.stopPropagation()}
      onEmptiedCapture={e => e.stopPropagation()}
      onEncrypted={e => e.stopPropagation()}
      onEncryptedCapture={e => e.stopPropagation()}
      onEnded={e => e.stopPropagation()}
      onEndedCapture={e => e.stopPropagation()}
      onLoadedData={e => e.stopPropagation()}
      onLoadedDataCapture={e => e.stopPropagation()}
      onLoadedMetadata={e => e.stopPropagation()}
      onLoadedMetadataCapture={e => e.stopPropagation()}
      onLoadStart={e => e.stopPropagation()}
      onLoadStartCapture={e => e.stopPropagation()}
      onPause={e => e.stopPropagation()}
      onPauseCapture={e => e.stopPropagation()}
      onPlay={e => e.stopPropagation()}
      onPlayCapture={e => e.stopPropagation()}
      onPlaying={e => e.stopPropagation()}
      onPlayingCapture={e => e.stopPropagation()}
      onProgress={e => e.stopPropagation()}
      onProgressCapture={e => e.stopPropagation()}
      onRateChange={e => e.stopPropagation()}
      onRateChangeCapture={e => e.stopPropagation()}
      onSeeked={e => e.stopPropagation()}
      onSeekedCapture={e => e.stopPropagation()}
      onSeeking={e => e.stopPropagation()}
      onSeekingCapture={e => e.stopPropagation()}
      onStalled={e => e.stopPropagation()}
      onStalledCapture={e => e.stopPropagation()}
      onSuspend={e => e.stopPropagation()}
      onSuspendCapture={e => e.stopPropagation()}
      onTimeUpdate={e => e.stopPropagation()}
      onTimeUpdateCapture={e => e.stopPropagation()}
      onVolumeChange={e => e.stopPropagation()}
      onVolumeChangeCapture={e => e.stopPropagation()}
      onWaiting={e => e.stopPropagation()}
      onWaitingCapture={e => e.stopPropagation()}
      onAuxClick={e => e.stopPropagation()}
      onAuxClickCapture={e => e.stopPropagation()}
      onClick={e => e.stopPropagation()}
      onClickCapture={e => e.stopPropagation()}
      onContextMenu={e => e.stopPropagation()}
      onContextMenuCapture={e => e.stopPropagation()}
      onDoubleClick={e => e.stopPropagation()}
      onDoubleClickCapture={e => e.stopPropagation()}
      onDrag={e => e.stopPropagation()}
      onDragCapture={e => e.stopPropagation()}
      onDragEnd={e => e.stopPropagation()}
      onDragEndCapture={e => e.stopPropagation()}
      onDragEnter={e => e.stopPropagation()}
      onDragEnterCapture={e => e.stopPropagation()}
      onDragExit={e => e.stopPropagation()}
      onDragExitCapture={e => e.stopPropagation()}
      onDragLeave={e => e.stopPropagation()}
      onDragLeaveCapture={e => e.stopPropagation()}
      onDragOver={e => e.stopPropagation()}
      onDragOverCapture={e => e.stopPropagation()}
      onDragStart={e => e.stopPropagation()}
      onDragStartCapture={e => e.stopPropagation()}
      onDrop={e => e.stopPropagation()}
      onDropCapture={e => e.stopPropagation()}
      onMouseDown={e => e.stopPropagation()}
      onMouseDownCapture={e => e.stopPropagation()}
      onMouseEnter={e => e.stopPropagation()}
      onMouseLeave={e => e.stopPropagation()}
      onMouseMove={e => e.stopPropagation()}
      onMouseMoveCapture={e => e.stopPropagation()}
      onMouseOut={e => e.stopPropagation()}
      onMouseOutCapture={e => e.stopPropagation()}
      onMouseOver={e => e.stopPropagation()}
      onMouseOverCapture={e => e.stopPropagation()}
      onMouseUp={e => e.stopPropagation()}
      onMouseUpCapture={e => e.stopPropagation()}
      onSelect={e => e.stopPropagation()}
      onSelectCapture={e => e.stopPropagation()}
      onTouchCancel={e => e.stopPropagation()}
      onTouchCancelCapture={e => e.stopPropagation()}
      onTouchEnd={e => e.stopPropagation()}
      onTouchEndCapture={e => e.stopPropagation()}
      onTouchMove={e => e.stopPropagation()}
      onTouchMoveCapture={e => e.stopPropagation()}
      onTouchStart={e => e.stopPropagation()}
      onTouchStartCapture={e => e.stopPropagation()}
      onPointerDown={e => e.stopPropagation()}
      onPointerDownCapture={e => e.stopPropagation()}
      onPointerMove={e => e.stopPropagation()}
      onPointerMoveCapture={e => e.stopPropagation()}
      onPointerUp={e => e.stopPropagation()}
      onPointerUpCapture={e => e.stopPropagation()}
      onPointerCancel={e => e.stopPropagation()}
      onPointerCancelCapture={e => e.stopPropagation()}
      onPointerEnter={e => e.stopPropagation()}
      onPointerEnterCapture={e => e.stopPropagation()}
      onPointerLeave={e => e.stopPropagation()}
      onPointerLeaveCapture={e => e.stopPropagation()}
      onPointerOver={e => e.stopPropagation()}
      onPointerOverCapture={e => e.stopPropagation()}
      onPointerOut={e => e.stopPropagation()}
      onPointerOutCapture={e => e.stopPropagation()}
      onGotPointerCapture={e => e.stopPropagation()}
      onGotPointerCaptureCapture={e => e.stopPropagation()}
      onLostPointerCapture={e => e.stopPropagation()}
      onLostPointerCaptureCapture={e => e.stopPropagation()}
      onScroll={e => e.stopPropagation()}
      onScrollCapture={e => e.stopPropagation()}
      onWheel={e => e.stopPropagation()}
      onWheelCapture={e => e.stopPropagation()}
      onAnimationStart={e => e.stopPropagation()}
      onAnimationStartCapture={e => e.stopPropagation()}
      onAnimationEnd={e => e.stopPropagation()}
      onAnimationEndCapture={e => e.stopPropagation()}
      onAnimationIteration={e => e.stopPropagation()}
      onAnimationIterationCapture={e => e.stopPropagation()}
      onTransitionEnd={e => e.stopPropagation()}
      onTransitionEndCapture={e => e.stopPropagation()}
    >
      {children}
    </div>
  );

  return ReactDOM.createPortal(element, container);
}

Unfortunately, even if we add that to our overlays, it won't prevent propagation out of other portals that people might use with React Aria, so we also have to add checks like this in most of our event handlers:

if (!e.currentTarget.contains(e.target)) {
  return;
}

This fixes event handlers that we control, but doesn't fix custom event handlers added by consumers, who would also need to do that unless the portal itself also stopped propagation as above.

React itself is the common thread here, so ideally React portals themselves should not stop propagation by default. Otherwise it's easy to get wrong and cause bugs by using different combinations of portal components and components with event handlers from different libraries that may or may not have these checks.

@craigkovatch
Copy link

I'm currently working on a bug from five years ago that goes all the way back to this issue. Really sad this has been here since Oct 2017 with no official response at all. I think this is the worst API decision decision in React and am confused why there's still no ...whatever the opposite of an escape hatch is

@zombieJ
Copy link
Contributor

zombieJ commented Oct 27, 2023

Hmmm...
I could not remember how many times I track and found this issue...

@YassienW
Copy link

YassienW commented Nov 6, 2023

Stopping propagation is NOT a solution, sometimes you want to handle events further down the line or on the document. It makes perfect sense to be able to propagate events according to their natural location in the DOM (i'd argue even more sense than what we have now)

Like many others in this thread I'm now facing an issue where nested elements that use portals propagate events and affect each other in weird illogical ways, with no straight forward way to fix it.

@jasosims
Copy link

jasosims commented May 2, 2024

This behavior has caused numerous bugs in my component library. Event propagation should mirror the way native events propagate in the DOM. I can't think of any reason why I'd want events to propagate through the component tree when it differs from the DOM hierarchy (i.e. across portal boundaries), but if there is one, it should be necessary to opt into that behavior, as it's counterintuitive and breaks expectations about how events work.

As others have mentioned, stopping event propagation is not a viable solution. There are numerous reasons why event propagation should (almost) never be stopped. If you want things like dropdowns and popovers to close when clicking outside of them, the only reasonable approach is to add an event listener to document.body. If something receives a click and stops propagation, it won't work.

If you run into a situation where you need to ignore an event when it's already been handled by something else, you can add a property to the event (I use event.eventHandled = true, or in React components, event.nativeEvent.eventHandled = true), then ignore events that have that property, as needed. Indeed, that's also how I fix each of the bugs caused by the decision to have events bubble up the component tree without respect for the DOM hierarchy.

I think this behavior should be off by default and enabled on a per-portal basis (perhaps as an argument to createPortal). It has only ever caused problems for me, and I've never encountered a situation where it would actually be useful.

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

No branches or pull requests