[ClickAwayListener] Handle portaled element

[ccmartell]

• The issue is present in the latest release.
• I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

When a select component is within a popper element and disablePortal is true, the dropdown items are positioned absolutely in relation to the popper element and not the page. This is also true for any absolutely positioned container that uses transform (which is what popper is using).

Expected Behavior 🤔

Expected behavior would be that the position of the dropdown would match the location of the select element and display correctly.

Steps to Reproduce 🕹

I have mocked the issue here

Steps:

1. Place absolutely positioned component and add a transform (or just use popper)
2. Add select component

Context 🔦

When using a clickaway listener with popper to display a popup, a portaled menu will cause the clickaway listener to fire, closing the popper and losing our work. We then were able to use disablePortal in the MenuProps, but that causes this issue. We have done a workaround using Popover as it doesn't have a transform and displays the Select menu correctly.

Your Environment 🌎

Tech	Version
Material-UI	v4.7.0
React	v16.11.0
Browser	Chrome
TypeScript	No

[oliviertassinari]

Your problem is related to a "couple" of others 😆:

• [Popover] On mobile Chrome while zoomed, opening popover causes page to jump #17636: A change of the Popover's positioning logic to use Popper would fix the placement issue.
• [Menu] Clicking outside a menu blocks click from bubbling up #11243: However, even if we fix the placement issue, the backdrop logic won't be correct. Either the backdrop needs to be portal (to block interactions with the rest of the page) or we need to use a click away listener.
  • If we want to support a portal backdrop, I would suggest the addition of a event.muiHandled logic to prevent the click away listener to trigger (cf. swipeable drawer) or a stop propagation or a prevent default.
  • If you nest two click away listeners, and if you click outside of both, both will trigger. I doubt it should be the case if they are nested. I would suggest the addition of a event.muiHandled logic to prevent it (cf. swipeable drawer).

[oliviertassinari]

We have a full reproduction example in #17990 that we can leverage. Based on it, I could come up with this potential solution.

Before

After

I would propose the following fix, let me know what you think about it:

diff --git a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
index c689731b6..8e38b6635 100644
--- a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
+++ b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
@@ -16,10 +16,17 @@ function mapEventPropToEvent(eventProp) {
  * For instance, if you need to hide a menu when people click anywhere else on your page.
  */
 const ClickAwayListener = React.forwardRef(function ClickAwayListener(props, ref) {
-  const { children, mouseEvent = 'onClick', touchEvent = 'onTouchEnd', onClickAway } = props;
+  const {
+    children,
+    disableReactTree = false,
+    mouseEvent = 'onClick',
+    onClickAway,
+    touchEvent = 'onTouchEnd',
+  } = props;
   const movedRef = React.useRef(false);
   const nodeRef = React.useRef(null);
   const mountedRef = React.useRef(false);
+  const syntheticEvent = React.useRef(false);

   React.useEffect(() => {
     mountedRef.current = true;
@@ -40,6 +47,9 @@ const ClickAwayListener = React.forwardRef(function ClickAwayListener(props, ref
   const handleRef = useForkRef(children.ref, handleOwnRef);

   const handleClickAway = useEventCallback(event => {
+    const insideReactTree = syntheticEvent.current;
+    syntheticEvent.current = false;
+
     // Ignore events that have been `event.preventDefault()` marked.
     if (event.defaultPrevented) {
       return;
@@ -67,7 +77,8 @@ const ClickAwayListener = React.forwardRef(function ClickAwayListener(props, ref
     if (
       doc.documentElement &&
       doc.documentElement.contains(event.target) &&
-      !nodeRef.current.contains(event.target)
+      !nodeRef.current.contains(event.target) &&
+      (disableReactTree || !insideReactTree)
     ) {
       onClickAway(event);
     }
@@ -106,7 +117,39 @@ const ClickAwayListener = React.forwardRef(function ClickAwayListener(props, ref
     return undefined;
   }, [handleClickAway, mouseEvent]);

-  return <React.Fragment>{React.cloneElement(children, { ref: handleRef })}</React.Fragment>;
+  const handleSyntheticMouse = event => {
+    syntheticEvent.current = true;
+
+    const childrenProps = children.props;
+    if (childrenProps[mouseEvent]) {
+      childrenProps[mouseEvent](event);
+    }
+  };
+
+  const handleSyntheticTouch = event => {
+    syntheticEvent.current = true;
+
+    const childrenProps = children.props;
+    if (childrenProps[touchEvent]) {
+      childrenProps[touchEvent](event);
+    }
+  };
+
+  const childrenProps = {};
+
+  if (mouseEvent !== false) {
+    childrenProps[mouseEvent] = handleSyntheticMouse;
+  }
+
+  if (touchEvent !== false) {
+    childrenProps[touchEvent] = handleSyntheticTouch;
+  }
+
+  return (
+    <React.Fragment>
+      {React.cloneElement(children, { ref: handleRef, ...childrenProps })}
+    </React.Fragment>
+  );
 });

 ClickAwayListener.propTypes = {
@@ -114,6 +157,11 @@ ClickAwayListener.propTypes = {
    * The wrapped element.
    */
   children: elementAcceptingRef.isRequired,
+  /**
+   * If `true`, the React tree is ignored and only the DOM tree is considered.
+   * This prop changes how portaled elements are handled.
+   */
+  disableReactTree: PropTypes.bool,
   /**
    * The mouse event to listen to. You can disable the listener by providing `false`.
    */

cc @Izhaki as it might impact your hook API
cc @dmtrKovalenko for context

[eps1lon]

I don't think this is a good first issue.

This discussion should happen in the react core. Special casing where we want to ignore the component tree for event bubbling is (as with most special cases) not a good idea.

We should rather explore alternative APIs or contribute these use cases to the appropriate issues in the react repositories. Otherwise we introduce more suprises and bundle size into the API.

[oliviertassinari]

@eps1lon Do you mean that we shouldn't expose a disableReactTree prop?

Yeah, I think that we can wait to see a compelling use case for this prop 👍. Including the React tree should likely be the default behavior, and changed, independently from the DOM tree.

[Izhaki]

I need ClickAwayListener to intercept a mouseup outside and element with drag - nothing to do with MUI.

And the idea of having a dependency in mui/core for this seems to me a bit off.

The point I'm trying to make is that this functionality is possibly far more generic to be included in MUI. I'd consider just extracting it to its own package (I've seen at least 3 bespoke implementation of it already in other libraries, who would probably be delighted to see a package for it. currently one needs to have come across mui to know it exist).