diff --git a/CHANGELOG.md b/CHANGELOG.md index f0f8e751729..17711923b6f 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap index c8edbf6f09f..db6f9fbfc90 100644 --- a/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap +++ b/src/components/basic_table/__snapshots__/in_memory_table.test.js.snap @@ -357,8 +357,11 @@ exports[`EuiInMemoryTable behavior pagination 1`] = `
{ // 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( -
+
@@ -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( -
+
@@ -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( -
+
@@ -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( -
+
@@ -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); }); diff --git a/src/components/form/super_select/__snapshots__/super_select.test.js.snap b/src/components/form/super_select/__snapshots__/super_select.test.js.snap index 0f802dec6d9..c7ccb964e75 100644 --- a/src/components/form/super_select/__snapshots__/super_select.test.js.snap +++ b/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`] = ` >
) 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,25 +55,33 @@ 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')) { @@ -68,12 +89,31 @@ export class EuiOutsideClickDetector extends Component { } 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); diff --git a/src/components/outside_click_detector/outside_click_detector.test.js b/src/components/outside_click_detector/outside_click_detector.test.js index 86254ef9f4c..f7e586bf115 100644 --- a/src/components/outside_click_detector/outside_click_detector.test.js +++ b/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( -
+
@@ -48,7 +57,8 @@ describe('EuiOutsideClickDetector', () => {
); - 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);