From f2d51e0584e5a308799029a4b9ce1d0be3800813 Mon Sep 17 00:00:00 2001 From: Joerg Baier Date: Wed, 24 Apr 2019 09:23:09 -0500 Subject: [PATCH 1/4] Update Modal.js --- src/modules/Modal/Modal.js | 28 +++++++++++++++++++++++----- 1 file changed, 23 insertions(+), 5 deletions(-) diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index bca7fb89b4..9ebaa88b1c 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -153,6 +153,7 @@ class Modal extends Component { ref = createRef() dimmerRef = createRef() + mouseDownEvent = null componentWillUnmount() { debug('componentWillUnmount()') @@ -178,11 +179,20 @@ class Modal extends Component { this.trySetState({ open: false }) } - handleDocumentClick = (e) => { - debug('handleDocumentClick()') + handleDocumentMouseDown = (e) => { + this.mouseDownEvent = e + } + + handleDocumentMouseUp = (e) => { + debug('handleDocumentMouseUp()') const { closeOnDimmerClick } = this.props - if (!closeOnDimmerClick || doesNodeContainClick(this.ref.current, e)) return + if ( + !closeOnDimmerClick || + doesNodeContainClick(this.ref.current, this.mouseDownEvent) || + doesNodeContainClick(this.ref.current, e) + ) + return _.invoke(this.props, 'onClose', e, this.props) this.trySetState({ open: false }) @@ -209,7 +219,11 @@ class Modal extends Component { this.setState({ scrolling: false }) this.setPositionAndClassNames() - eventStack.sub('click', this.handleDocumentClick, { + eventStack.sub('mousedown', this.handleDocumentMouseDown, { + pool: eventPool, + target: this.dimmerRef.current, + }) + eventStack.sub('mouseup', this.handleDocumentMouseUp, { pool: eventPool, target: this.dimmerRef.current, }) @@ -221,7 +235,11 @@ class Modal extends Component { debug('handlePortalUnmount()', { eventPool }) cancelAnimationFrame(this.animationRequestId) - eventStack.unsub('click', this.handleDocumentClick, { + eventStack.unsub('mousedown', this.handleDocumentMouseDown, { + pool: eventPool, + target: this.dimmerRef.current, + }) + eventStack.unsub('mouseup', this.handleDocumentMouseUp, { pool: eventPool, target: this.dimmerRef.current, }) From d980236e1b3062aa74b2f78562c90ca171b4d6a1 Mon Sep 17 00:00:00 2001 From: Joerg Baier Date: Wed, 24 Apr 2019 10:07:29 -0500 Subject: [PATCH 2/4] fix spec --- test/specs/addons/Confirm/Confirm-test.js | 16 +++++++++------ test/specs/modules/Modal/Modal-test.js | 25 +++++++++++++++-------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/test/specs/addons/Confirm/Confirm-test.js b/test/specs/addons/Confirm/Confirm-test.js index 9ca92ec155..32841bc07f 100644 --- a/test/specs/addons/Confirm/Confirm-test.js +++ b/test/specs/addons/Confirm/Confirm-test.js @@ -32,13 +32,13 @@ describe('Confirm', () => { autoGenerateKey: false, propKey: 'header', ShorthandComponent: Modal.Header, - mapValueToProps: content => ({ content }), + mapValueToProps: (content) => ({ content }), }) common.implementsShorthandProp(Confirm, { autoGenerateKey: false, propKey: 'content', ShorthandComponent: Modal.Content, - mapValueToProps: content => ({ content }), + mapValueToProps: (content) => ({ content }), }) describe('children', () => { @@ -124,22 +124,26 @@ describe('Confirm', () => { }) it('is called on dimmer click', () => { - domEvent.click('.ui.dimmer') + domEvent.mouseDown('.ui.dimmer') + domEvent.mouseUp('.ui.dimmer') spy.should.have.been.calledOnce() }) it('is called on click outside of the modal', () => { - domEvent.click(document.querySelector('.ui.modal').parentNode) + domEvent.mouseDown(document.querySelector('.ui.modal').parentNode) + domEvent.mouseUp(document.querySelector('.ui.modal').parentNode) spy.should.have.been.calledOnce() }) it('is not called on click inside of the modal', () => { - domEvent.click(document.querySelector('.ui.modal')) + domEvent.mouseDown(document.querySelector('.ui.modal')) + domEvent.mouseUp(document.querySelector('.ui.modal')) spy.should.not.have.been.calledOnce() }) it('is not called on body click', () => { - domEvent.click('body') + domEvent.mouseDown('body') + domEvent.mouseUp('body') spy.should.not.have.been.calledOnce() }) diff --git a/test/specs/modules/Modal/Modal-test.js b/test/specs/modules/Modal/Modal-test.js index a98ba3f633..1a56c4eb5d 100644 --- a/test/specs/modules/Modal/Modal-test.js +++ b/test/specs/modules/Modal/Modal-test.js @@ -53,13 +53,13 @@ describe('Modal', () => { autoGenerateKey: false, propKey: 'header', ShorthandComponent: ModalHeader, - mapValueToProps: content => ({ content }), + mapValueToProps: (content) => ({ content }), }) common.implementsShorthandProp(Modal, { autoGenerateKey: false, propKey: 'content', ShorthandComponent: ModalContent, - mapValueToProps: content => ({ content }), + mapValueToProps: (content) => ({ content }), }) // Heads up! @@ -319,28 +319,32 @@ describe('Modal', () => { it('is called on dimmer click', () => { wrapperMount() - domEvent.click('.ui.dimmer') + domEvent.mouseDown('.ui.dimmer') + domEvent.mouseUp('.ui.dimmer') spy.should.have.been.calledOnce() }) it('is called on click outside of the modal', () => { wrapperMount() - domEvent.click(document.querySelector('.ui.modal').parentNode) + domEvent.mouseDown(document.querySelector('.ui.modal').parentNode) + domEvent.mouseUp(document.querySelector('.ui.modal').parentNode) spy.should.have.been.calledOnce() }) it('is not called on click inside of the modal', () => { wrapperMount() - domEvent.click(document.querySelector('.ui.modal')) + domEvent.mouseDown(document.querySelector('.ui.modal')) + domEvent.mouseUp(document.querySelector('.ui.modal')) spy.should.not.have.been.called() }) it('is not called on body click', () => { wrapperMount() - domEvent.click(document.body) + domEvent.mouseDown(document.body) + domEvent.mouseUp(document.body) spy.should.not.have.been.calledOnce() }) @@ -368,14 +372,16 @@ describe('Modal', () => { it('is not called on dimmer click when closeOnDimmerClick is false', () => { wrapperMount() - domEvent.click('.ui.dimmer') + domEvent.mouseDown('.ui.dimmer') + domEvent.mouseUp('.ui.dimmer') spy.should.not.have.been.called() }) it('is not called on body click when closeOnDocumentClick is false', () => { wrapperMount() - domEvent.click(document.body) + domEvent.mouseDown(document.body) + domEvent.mouseUp(document.body) spy.should.not.have.been.called() }) }) @@ -541,7 +547,8 @@ describe('Modal', () => { requestAnimationFrame(() => { assertBodyClasses('scrolling') - domEvent.click('.ui.dimmer') + domEvent.mouseDown('.ui.dimmer') + domEvent.mouseUp('.ui.dimmer') assertBodyClasses('scrolling', false) From 863a1c16eafd0204c360feaf40ff667f75bd8b36 Mon Sep 17 00:00:00 2001 From: Joerg Baier Date: Fri, 26 Apr 2019 11:55:11 -0500 Subject: [PATCH 3/4] match Popup PR --- src/modules/Modal/Modal.js | 17 +++++++------ test/specs/addons/Confirm/Confirm-test.js | 12 ++++------ test/specs/modules/Modal/Modal-test.js | 29 ++++++++++++----------- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index 9ebaa88b1c..88f1f6fad1 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -153,7 +153,7 @@ class Modal extends Component { ref = createRef() dimmerRef = createRef() - mouseDownEvent = null + latestDocumentMouseDownEvent = null componentWillUnmount() { debug('componentWillUnmount()') @@ -180,16 +180,19 @@ class Modal extends Component { } handleDocumentMouseDown = (e) => { - this.mouseDownEvent = e + this.latestDocumentMouseDownEvent = e } - handleDocumentMouseUp = (e) => { - debug('handleDocumentMouseUp()') + handleDocumentClick = (e) => { + debug('handleDocumentClick()') const { closeOnDimmerClick } = this.props + const previousDocumentMouseDownEvent = this.latestDocumentMouseDownEvent + delete this.latestDocumentMouseDownEvent + if ( !closeOnDimmerClick || - doesNodeContainClick(this.ref.current, this.mouseDownEvent) || + doesNodeContainClick(this.ref.current, previousDocumentMouseDownEvent) || doesNodeContainClick(this.ref.current, e) ) return @@ -223,7 +226,7 @@ class Modal extends Component { pool: eventPool, target: this.dimmerRef.current, }) - eventStack.sub('mouseup', this.handleDocumentMouseUp, { + eventStack.sub('click', this.handleDocumentClick, { pool: eventPool, target: this.dimmerRef.current, }) @@ -239,7 +242,7 @@ class Modal extends Component { pool: eventPool, target: this.dimmerRef.current, }) - eventStack.unsub('mouseup', this.handleDocumentMouseUp, { + eventStack.unsub('click', this.handleDocumentClick, { pool: eventPool, target: this.dimmerRef.current, }) diff --git a/test/specs/addons/Confirm/Confirm-test.js b/test/specs/addons/Confirm/Confirm-test.js index 32841bc07f..d59a48c8ec 100644 --- a/test/specs/addons/Confirm/Confirm-test.js +++ b/test/specs/addons/Confirm/Confirm-test.js @@ -124,26 +124,22 @@ describe('Confirm', () => { }) it('is called on dimmer click', () => { - domEvent.mouseDown('.ui.dimmer') - domEvent.mouseUp('.ui.dimmer') + domEvent.click('.ui.dimmer') spy.should.have.been.calledOnce() }) it('is called on click outside of the modal', () => { - domEvent.mouseDown(document.querySelector('.ui.modal').parentNode) - domEvent.mouseUp(document.querySelector('.ui.modal').parentNode) + domEvent.click(document.querySelector('.ui.modal').parentNode) spy.should.have.been.calledOnce() }) it('is not called on click inside of the modal', () => { - domEvent.mouseDown(document.querySelector('.ui.modal')) - domEvent.mouseUp(document.querySelector('.ui.modal')) + domEvent.click(document.querySelector('.ui.modal')) spy.should.not.have.been.calledOnce() }) it('is not called on body click', () => { - domEvent.mouseDown('body') - domEvent.mouseUp('body') + domEvent.click('body') spy.should.not.have.been.calledOnce() }) diff --git a/test/specs/modules/Modal/Modal-test.js b/test/specs/modules/Modal/Modal-test.js index 1a56c4eb5d..2c8f9894ce 100644 --- a/test/specs/modules/Modal/Modal-test.js +++ b/test/specs/modules/Modal/Modal-test.js @@ -319,32 +319,36 @@ describe('Modal', () => { it('is called on dimmer click', () => { wrapperMount() - domEvent.mouseDown('.ui.dimmer') - domEvent.mouseUp('.ui.dimmer') + domEvent.click('.ui.dimmer') spy.should.have.been.calledOnce() }) it('is called on click outside of the modal', () => { wrapperMount() - domEvent.mouseDown(document.querySelector('.ui.modal').parentNode) - domEvent.mouseUp(document.querySelector('.ui.modal').parentNode) + domEvent.click(document.querySelector('.ui.modal').parentNode) spy.should.have.been.calledOnce() }) - it('is not called on click inside of the modal', () => { + it('is not called on mousedown inside and mouseup outside of the modal', () => { wrapperMount() domEvent.mouseDown(document.querySelector('.ui.modal')) - domEvent.mouseUp(document.querySelector('.ui.modal')) + domEvent.click(document.querySelector('.ui.modal').parentNode) + spy.should.not.have.been.called() + }) + + it('is not called on click inside of the modal', () => { + wrapperMount() + + domEvent.click(document.querySelector('.ui.modal')) spy.should.not.have.been.called() }) it('is not called on body click', () => { wrapperMount() - domEvent.mouseDown(document.body) - domEvent.mouseUp(document.body) + domEvent.click(document.body) spy.should.not.have.been.calledOnce() }) @@ -372,16 +376,14 @@ describe('Modal', () => { it('is not called on dimmer click when closeOnDimmerClick is false', () => { wrapperMount() - domEvent.mouseDown('.ui.dimmer') - domEvent.mouseUp('.ui.dimmer') + domEvent.click('.ui.dimmer') spy.should.not.have.been.called() }) it('is not called on body click when closeOnDocumentClick is false', () => { wrapperMount() - domEvent.mouseDown(document.body) - domEvent.mouseUp(document.body) + domEvent.click(document.body) spy.should.not.have.been.called() }) }) @@ -547,8 +549,7 @@ describe('Modal', () => { requestAnimationFrame(() => { assertBodyClasses('scrolling') - domEvent.mouseDown('.ui.dimmer') - domEvent.mouseUp('.ui.dimmer') + domEvent.click('.ui.dimmer') assertBodyClasses('scrolling', false) From e2c7b5049c8ac0d69e499dce9fb093236a2e6ab1 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Sun, 5 May 2019 14:37:37 +0200 Subject: [PATCH 4/4] Update Modal.js --- src/modules/Modal/Modal.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/modules/Modal/Modal.js b/src/modules/Modal/Modal.js index 88f1f6fad1..803735a68a 100644 --- a/src/modules/Modal/Modal.js +++ b/src/modules/Modal/Modal.js @@ -186,13 +186,12 @@ class Modal extends Component { handleDocumentClick = (e) => { debug('handleDocumentClick()') const { closeOnDimmerClick } = this.props - - const previousDocumentMouseDownEvent = this.latestDocumentMouseDownEvent - delete this.latestDocumentMouseDownEvent + const currentDocumentMouseDownEvent = this.latestDocumentMouseDownEvent + this.latestDocumentMouseDownEvent = null if ( !closeOnDimmerClick || - doesNodeContainClick(this.ref.current, previousDocumentMouseDownEvent) || + doesNodeContainClick(this.ref.current, currentDocumentMouseDownEvent) || doesNodeContainClick(this.ref.current, e) ) return