From d1b17ee34b221c69666d5602291e364fdd24381d Mon Sep 17 00:00:00 2001 From: Greg Thompson Date: Tue, 26 Mar 2019 10:58:29 -0500 Subject: [PATCH] backport #1761 --- CHANGELOG.md | 4 +- .../in_memory_table.test.js.snap | 5 +- .../__snapshots__/super_select.test.js.snap | 5 +- .../outside_click_detector.js | 56 ++++++++++++++++--- .../outside_click_detector.test.js | 18 ++++-- 5 files changed, 73 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e64415ef9c..3bcf2d543eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ ## [`master`](https://github.com/elastic/eui/tree/master) -No public interface changes since `6.10.5`. +**Bug fixes** + +- 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)) ## [`6.10.5`](https://github.com/elastic/eui/tree/v6.10.5) 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 7b63c057950..7840fcab6d5 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 @@ -314,8 +314,11 @@ exports[`EuiInMemoryTable behavior pagination 1`] = `
) 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(); - } + 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);