Change EuiOutsideClickDetector to use mousedown and mouseup events (#…

…1761)

* change EuiOutsideClickDetector to use mousedown and mouseup events

* #1761 changelog entry

CHANGELOG.md

@@ -9,6 +9,7 @@
 - Set `h1 through h6, p` tags font reset based on family, size, and weight ([#1760](https://github.com/elastic/eui/pull/1760))
 - Fixed `EuiButton` font size inheritence ([#1760](https://github.com/elastic/eui/pull/1760))
 - Updated button elements in `EuiFilePicker`, `EuiFormControlLayoutClearButton`, `EuiFormControlLayoutCustomIcon`, `EuiListGroupItem`, and `EuiSideNavItem` to type=button ([#1764](https://github.com/elastic/eui/pull/1764))
+- Fixed outside click detection inconsistencies by comparing `mouseup` and `mousedown` event targets rather than using `click` event target ([#1761](https://github.com/elastic/eui/pull/1761))
 
 ## [`9.5.0`](https://github.com/elastic/eui/tree/v9.5.0)
 

src/components/basic_table/__snapshots__/in_memory_table.test.js.snap

@@ -357,8 +357,11 @@ exports[`EuiInMemoryTable behavior pagination 1`] = `
                         <div
                           className="euiPopover euiPopover--anchorUpRight euiPopover--withTitle"
                           id="customizablePagination"
-                          onClick={[Function]}
                           onKeyDown={[Function]}
+                          onMouseDown={[Function]}
+                          onMouseUp={[Function]}
+                          onTouchEnd={[Function]}
+                          onTouchStart={[Function]}
                         >
                           <div
                             className="euiPopover__anchor"

src/components/focus_trap/focus_trap.test.js

@@ -84,15 +84,24 @@ describe('EuiFocusTrap', () => {
       // enzyme doesn't mount the components into the global jsdom `document`
       // but that's where the click detector listener is,
       // pass the top-level mounted component's click event on to document
-      const triggerDocumentClick = e => {
-        const event = new Event('click');
+      const triggerDocumentMouseDown = e => {
+        const event = new Event('mousedown');
+        event.euiGeneratedBy = e.nativeEvent.euiGeneratedBy;
+        document.dispatchEvent(event);
+      };
+
+      const triggerDocumentMouseUp = e => {
+        const event = new Event('mouseup');
         event.euiGeneratedBy = e.nativeEvent.euiGeneratedBy;
         document.dispatchEvent(event);
       };
 
       test('trap remains enabled when false', () => {
         const component = mount(
-          <div onClick={triggerDocumentClick}>
+          <div
+            onMouseDown={triggerDocumentMouseDown}
+            onMouseUp={triggerDocumentMouseUp}
+          >
             <EuiFocusTrap>
               <div data-test-subj="container">
                 <input data-test-subj="input" />
@@ -105,7 +114,8 @@ describe('EuiFocusTrap', () => {
 
         // The existence of `data-focus-lock-disabled=false` indicates that the trap is enabled.
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(1);
-        findTestSubject(component, 'outside').simulate('click');
+        findTestSubject(component, 'outside').simulate('mousedown');
+        findTestSubject(component, 'outside').simulate('mouseup');
         // `react-focus-lock` relies on real DOM events to move focus about.
         // Exposed attributes are the most consistent way to attain its state.
         // See https://github.com/theKashey/react-focus-lock/blob/master/_tests/FocusLock.spec.js for the lib in use
@@ -115,7 +125,10 @@ describe('EuiFocusTrap', () => {
 
       test('trap remains enabled after internal clicks', () => {
         const component = mount(
-          <div onClick={triggerDocumentClick}>
+          <div
+            onMouseDown={triggerDocumentMouseDown}
+            onMouseUp={triggerDocumentMouseUp}
+          >
             <EuiFocusTrap clickOutsideDisables>
               <div data-test-subj="container">
                 <input data-test-subj="input" />
@@ -127,14 +140,18 @@ describe('EuiFocusTrap', () => {
         );
 
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(1);
-        findTestSubject(component, 'input2').simulate('click');
+        findTestSubject(component, 'input2').simulate('mousedown');
+        findTestSubject(component, 'input2').simulate('mouseup');
         // Trap remains enabled
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(1);
       });
 
       test('trap remains enabled after internal portal clicks', () => {
         const component = mount(
-          <div onClick={triggerDocumentClick}>
+          <div
+            onMouseDown={triggerDocumentMouseDown}
+            onMouseUp={triggerDocumentMouseUp}
+          >
             <EuiFocusTrap clickOutsideDisables>
               <div data-test-subj="container">
                 <input data-test-subj="input" />
@@ -149,14 +166,18 @@ describe('EuiFocusTrap', () => {
         );
 
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(1);
-        findTestSubject(component, 'input3').simulate('click');
+        findTestSubject(component, 'input3').simulate('mousedown');
+        findTestSubject(component, 'input3').simulate('mouseup');
         // Trap remains enabled
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(1);
       });
 
       test('trap becomes disabled on outside clicks', () => {
         const component = mount(
-          <div onClick={triggerDocumentClick}>
+          <div
+            onMouseDown={triggerDocumentMouseDown}
+            onMouseUp={triggerDocumentMouseUp}
+          >
             <EuiFocusTrap clickOutsideDisables>
               <div data-test-subj="container">
                 <input data-test-subj="input" />
@@ -168,7 +189,8 @@ describe('EuiFocusTrap', () => {
         );
 
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(1);
-        findTestSubject(component, 'outside').simulate('click');
+        findTestSubject(component, 'outside').simulate('mousedown');
+        findTestSubject(component, 'outside').simulate('mouseup');
         // Trap becomes disabled
         expect(component.find('[data-focus-lock-disabled=false]').length).toBe(0);
       });

src/components/form/super_select/__snapshots__/super_select.test.js.snap

@@ -524,8 +524,11 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = `
     >
       <div
         className="euiPopover euiPopover--anchorDownCenter euiSuperSelect"
-        onClick={[Function]}
         onKeyDown={[Function]}
+        onMouseDown={[Function]}
+        onMouseUp={[Function]}
+        onTouchEnd={[Function]}
+        onTouchStart={[Function]}
       >
         <div
           className="euiPopover__anchor euiSuperSelect__popoverAnchor"
@@ -686,11 +689,15 @@ exports[`EuiSuperSelect props more props are propogated to each option 2`] = `
             >
               <OutsideEventDetector
                 handleEvent={[Function]}
-                onClick={[Function]}
+                onMouseDown={[Function]}
+                onMouseUp={[Function]}
+                onTouchEnd={[Function]}
+                onTouchStart={[Function]}
               >
                 <div
-                  onClick={[Function]}
                   onMouseDown={[Function]}
+                  onMouseUp={[Function]}
+                  onTouchEnd={[Function]}
                   onTouchStart={[Function]}
                 >
                   <FocusLock

src/components/outside_click_detector/outside_click_detector.js

@@ -7,6 +7,17 @@ import PropTypes from 'prop-types';
 import { htmlIdGenerator } from '../../services/accessibility';
 
 export class EuiOutsideClickDetector extends Component {
+  // We are working with the assumption that a click event is
+  // equivalent to a sequential, compound press and release of
+  // the pointing device (mouse, finger, stylus, etc.).
+  // A click event's target can be imprecise, as the value will be
+  // the closest common ancestor of the press (mousedown, touchstart)
+  // and relase (mouseup, touchend) events (often <body />) if
+  // the the target of each event differs.
+  // We need the actual event targets to make the correct decisions
+  // about user intention. So, consider the down/start and up/end
+  // items below as the deconstruction of a click event.
+
   static propTypes = {
     children: PropTypes.node.isRequired,
     onOutsideClick: PropTypes.func.isRequired,
@@ -33,6 +44,8 @@ export class EuiOutsideClickDetector extends Component {
     // stamping the id even though the event originates outside
     // this component's reified DOM tree.
     this.id = htmlIdGenerator()();
+
+    this.capturedDownIds = [];
   }
 
   onClickOutside = event => {
@@ -42,38 +55,65 @@ export class EuiOutsideClickDetector extends Component {
     } = this.props;
 
     if (isDisabled) {
+      this.capturedDownIds = [];
       return;
     }
 
-    if (event.euiGeneratedBy && event.euiGeneratedBy.includes(this.id)) {
+    if (
+      (event.euiGeneratedBy && event.euiGeneratedBy.includes(this.id))
+      || this.capturedDownIds.includes(this.id)
+    ) {
+      this.capturedDownIds = [];
       return;
     }
 
-    onOutsideClick(event);
-  }
+    this.capturedDownIds = [];
+    return onOutsideClick(event);
+  };
 
   componentDidMount() {
-    document.addEventListener('click', this.onClickOutside);
+    document.addEventListener('mouseup', this.onClickOutside);
+    document.addEventListener('touchend', this.onClickOutside);
   }
 
   componentWillUnmount() {
-    document.removeEventListener('click', this.onClickOutside);
+    document.removeEventListener('mouseup', this.onClickOutside);
+    document.removeEventListener('touchend', this.onClickOutside);
   }
 
-  onChildClick = event => {
+  onChildClick = (event, cb) => {
     // to support nested click detectors, build an array
     // of detector ids that have been encountered
     if (event.nativeEvent.hasOwnProperty('euiGeneratedBy')) {
       event.nativeEvent.euiGeneratedBy.push(this.id);
     } else {
       event.nativeEvent.euiGeneratedBy = [this.id];
     }
-    if (this.props.onClick) this.props.onClick(event);
+    if (cb) cb(event);
+  }
+
+  onChildMouseDown = event => {
+    this.onChildClick(event, e => {
+      this.capturedDownIds = e.nativeEvent.euiGeneratedBy;
+      if (this.props.onMouseDown) this.props.onMouseDown(e);
+      if (this.props.onTouchStart) this.props.onTouchStart(e);
+    });
+
+  }
+
+  onChildMouseUp = event => {
+    this.onChildClick(event, e => {
+      if (this.props.onMouseUp) this.props.onMouseUp(e);
+      if (this.props.onTouchEnd) this.props.onTouchEnd(e);
+    });
   }
 
   render() {
     const props = ({ ...this.props.children.props, ...{
-      onClick: this.onChildClick,
+      onMouseDown: this.onChildMouseDown,
+      onTouchStart: this.onChildMouseDown,
+      onMouseUp: this.onChildMouseUp,
+      onTouchEnd: this.onChildMouseUp,
     } });
 
     const child = Children.only(this.props.children);

src/components/outside_click_detector/outside_click_detector.test.js

@@ -24,14 +24,23 @@ describe('EuiOutsideClickDetector', () => {
       // enzyme doesn't mount the components into the global jsdom `document`
       // but that's where the click detector listener is,
       // pass the top-level mounted component's click event on to document
-      const triggerDocumentClick = e => {
-        const event = new Event('click');
+      const triggerDocumentMouseDown = e => {
+        const event = new Event('mousedown');
+        event.euiGeneratedBy = e.nativeEvent.euiGeneratedBy;
+        document.dispatchEvent(event);
+      };
+
+      const triggerDocumentMouseUp = e => {
+        const event = new Event('mouseup');
         event.euiGeneratedBy = e.nativeEvent.euiGeneratedBy;
         document.dispatchEvent(event);
       };
 
       const component = mount(
-        <div onClick={triggerDocumentClick}>
+        <div
+          onMouseDown={triggerDocumentMouseDown}
+          onMouseUp={triggerDocumentMouseUp}
+        >
           <div>
             <EuiOutsideClickDetector onOutsideClick={parentDetector}>
               <div>
@@ -48,7 +57,8 @@ describe('EuiOutsideClickDetector', () => {
         </div>
       );
 
-      component.find('[data-test-subj="target"]').simulate('click');
+      component.find('[data-test-subj="target"]').simulate('mousedown');
+      component.find('[data-test-subj="target"]').simulate('mouseup');
 
       expect(unrelatedDetector).toHaveBeenCalledTimes(1);
       expect(parentDetector).toHaveBeenCalledTimes(0);