From b49a9b7d562d932cd6827c73ff9df8d766030c05 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 5 Apr 2020 13:49:05 +0200 Subject: [PATCH] Afterall, we can solve it :) --- .../click-away-listener.md | 19 ------------ .../ClickAwayListener/ClickAwayListener.js | 16 ++++++++-- .../ClickAwayListener.test.js | 30 +++++++++++++++++++ packages/material-ui/src/Tooltip/Tooltip.js | 3 +- 4 files changed, 45 insertions(+), 23 deletions(-) diff --git a/docs/src/pages/components/click-away-listener/click-away-listener.md b/docs/src/pages/components/click-away-listener/click-away-listener.md index 13bc68f033b11c..84dddb3231ae01 100644 --- a/docs/src/pages/components/click-away-listener/click-away-listener.md +++ b/docs/src/pages/components/click-away-listener/click-away-listener.md @@ -17,22 +17,3 @@ For instance, if you need to hide a menu dropdown when people click anywhere els Notice that the component only accepts one child element. You can find a more advanced demo on the [Menu documentation section](/components/menus/#menulist-composition). - -## Limitations - -If a page contains an element/component that gets removed from DOM when clicked, the `ClickAwayListener` will not be able to raise the `onClickAway` event due to a check trying to prevent raising the event for already removed `ClickAwayListener` children. - -The workaround for this is to either use css/style to hide the clicked element or delay the removal of the element. -Check the example below - -```jsx -// Instead of removing from DOM -{showButton && ()} - -// Use styles prop -) -``` diff --git a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js index 688f6147095e4c..b307f8d416d01e 100644 --- a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js +++ b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js @@ -63,10 +63,20 @@ const ClickAwayListener = React.forwardRef(function ClickAwayListener(props, ref return; } - // Multi window support - const doc = ownerDocument(nodeRef.current); + let insideDOM; - if (doc.documentElement.contains(event.target) && !nodeRef.current.contains(event.target)) { + // If not enough, can use https://github.com/DieterHolvoet/event-propagation-path/blob/master/propagationPath.js + if (event.composedPath) { + insideDOM = event.composedPath().indexOf(nodeRef.current) > -1; + } else { + const doc = ownerDocument(nodeRef.current); + // TODO v6 remove dead logic https://caniuse.com/#search=composedPath. + insideDOM = + !(doc.documentElement && doc.documentElement.contains(event.target)) || + nodeRef.current.contains(event.target); + } + + if (!insideDOM) { onClickAway(event); } }); diff --git a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js index 87063c1eb501e9..6c6ae16a3d92a6 100644 --- a/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js +++ b/packages/material-ui/src/ClickAwayListener/ClickAwayListener.test.js @@ -140,4 +140,34 @@ describe('', () => { fireEvent.click(document.body); expect(handleClickAway.callCount).to.equal(0); }); + + it.only('should support removed element child', () => { + const Demo = () => { + const [visible, setVisible] = React.useState(true); + + console.log('visible', visible); + + return visible ? ( + + ) : null; + }; + const handleClickAway = spy(); + const { getByText } = render( +
+ + +
+ +
, + ); + fireEvent.click(getByText('Will be removed from DOM')); + expect(handleClickAway.callCount).to.equal(1); + }); }); diff --git a/packages/material-ui/src/Tooltip/Tooltip.js b/packages/material-ui/src/Tooltip/Tooltip.js index 5b0e7a128ef59f..d93060f96371b9 100644 --- a/packages/material-ui/src/Tooltip/Tooltip.js +++ b/packages/material-ui/src/Tooltip/Tooltip.js @@ -444,6 +444,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { ...other, ...children.props, className: clsx(other.className, children.props.className), + ref: handleRef, }; const interactiveWrapperListeners = {}; @@ -502,7 +503,7 @@ const Tooltip = React.forwardRef(function Tooltip(props, ref) { return ( - {React.cloneElement(children, { ref: handleRef, ...childrenProps })} + {React.cloneElement(children, childrenProps)}