[ClickAwayListener] failed to rasing onClickAway if you have an element that is removed from DOM when clicked

[seare-kidane]

If you have a React component/html element that's going to be hidden when clicked, the ClickAwayListener will fail to trigger the onClickAway due to the following condition

    if (
      doc.documentElement &&
      doc.documentElement.contains(event.target) && // <-- event.target no longer in the DOM
      !nodeRef.current.contains(event.target)
    ) {
      onClickAway(event);
    }

• 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 Behaviour 😯

When clicking on an element that gets hidden on click, the ClickAwayListener ignores the click and doesn't trigger onClickAway event.

const handleClick = () => { setShowButton(false); };
const handleClickAway = () => {
  //This is not being called when clicking on the button below
  setClickAwayStatus('Registered onClickAway event from ClickAwayListener');
};

{showButton && (<Button onClick={handleClick}>
  Click to hide
</Button>)}

<p>{clickAwayStatus}</p>

<ClickAwayListener onClickAway={handleClickAway}>
  <div>
    {/* ... */}
  </div>
</ClickAwayListener>

Expected Behaviour 🤔

The onClickAway should be triggered and not care about whether the event source is removed from the DOM

Steps to Reproduce 🕹

Steps:

1. Open the codesandbox link: https://codesandbox.io/s/festive-dust-fftxt
2. Click on anywhere except the button labelled "Click to hide"
3. Notice the the paragraph text changes to "Registered onClickAway event from ClickAwayListener"
4. After 2 seconds the paragraph text should reset to "Waiting for event"
5. Now click on the button labelled "Click to hide"
6. Notice the paragraph text is still "Waiting for event" and will never reset to the previous text since the ClickAwayListener onClickAway event wasn't triggered.

Context 🔦

I'm using the ClickAwayListener with Popper to display a popup menu, and wants it to close when ever I click outside the opened menu. The behaviour works as long as you don't have components or html elements that are removed from DOM on click. I want the menu to close even if i click on other elements that are removed from DOM.

Your Environment 🌎

Tech	Version
Material-UI	v4.9.5
React	v16.12.0
Browser	Google Chrome: v80.0.3987.132 (64-bit)

[oliviertassinari]

@seare-kidane The logic originates from #463. I think that we should close this issue as "won't fix". The dismiss event on click behavior, if we allow it, would be much worse if it happened inside the click away container.

What do you think about the following resolution? Do you want to submit a pull request? :)
We can find the same strategy in Pomax/react-onclickoutside#68

diff --git a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
index 859f1af69..6efa5f026 100644
--- a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
+++ b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js
@@ -67,7 +67,7 @@ const ClickAwayListener = React.forwardRef(function ClickAwayListener(props, ref
     const doc = ownerDocument(nodeRef.current);

     if (
       doc.documentElement &&
+      // Make sure the target is still in the DOM, otherwise we can't reason.
       doc.documentElement.contains(event.target) &&
       !nodeRef.current.contains(event.target)
     ) {

I would also add a documented limitation about it.

[seare-kidane]

@oliviertassinari It seems removing the doc.documentElement.contains(event.target) condition could cause more worse issues. But I want to dig more to understand the reported issues better.
But, for now at least, we should add it to known issues in the docs and provide a workaround.

Even though it's going to pollute the DOM, the workaround I found is to use styles prop with conditional display css style to hide & show a component and keep it in the DOM.

// Instead of removing from DOM
{showButton && (<Button onClick={handleClick}>
  Click to hide
</Button>)}

// Use styles prop
<Button style={showButton ? { display: 'block' } : { display: 'none' }}  onClick={handleClick}>
  Click to hide
</Button>)

What do you think about modifying the ClickAwayListener doc page?

[oliviertassinari]

@seare-kidane +1 for mentioning this limitation on top of the proposed change in #20197 (comment). Delaying the removal of the element is another viable alternative.

[seare-kidane]

@oliviertassinari Sorry for my late response, it's been a tough time 😞. Anyways, delaying the removal of the element should also work and regarding the proposed change in #20197 (comment), I don't think removing doc.documentElement is necessary, since the it was not the source of the issue.

[oliviertassinari]

@seare-kidane Thanks for the update. As far as I understand it, the check of doc.documentElement serves no purpose, we can save it. Do you think that you will have time to work on a pull request? :)