From 4941d0b6cb577f6be19ce27725956e58b16489ac Mon Sep 17 00:00:00 2001 From: mic4ael Date: Tue, 10 Jul 2018 08:39:00 +0200 Subject: [PATCH 1/4] docs(EventStack): fix a numbering mistake --- src/lib/eventStack/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/eventStack/README.md b/src/lib/eventStack/README.md index 03be2b6ed4..59bdd0ad23 100644 --- a/src/lib/eventStack/README.md +++ b/src/lib/eventStack/README.md @@ -4,7 +4,7 @@ The `EventStack` solves two design problems: 1. Reduces the number of connected listeners to DOM nodes compared to `element.addListener()`. -1. Respects event ordering. Example, two modals are open and you only want the top modal to close on document click. +2. Respects event ordering. Example, two modals are open and you only want the top modal to close on document click. ## EventStack From 2ca6090d780779c98c89537e21f0c51ba308b94a Mon Sep 17 00:00:00 2001 From: mic4ael Date: Tue, 10 Jul 2018 08:40:37 +0200 Subject: [PATCH 2/4] fix(EventStack): don't remove eventPool on unsub if not necessary Closes #2905 --- src/lib/eventStack/EventTarget.js | 9 ++------- test/specs/lib/eventStack/EventStack-test.js | 14 ++++++++++++++ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/src/lib/eventStack/EventTarget.js b/src/lib/eventStack/EventTarget.js index a94387be7e..f41589b6ee 100644 --- a/src/lib/eventStack/EventTarget.js +++ b/src/lib/eventStack/EventTarget.js @@ -49,13 +49,8 @@ export default class EventTarget { if (pool) { const newPool = pool.removeHandlers(eventType, eventHandlers) - if (newPool.hasHandlers(eventType)) { - this.removeTargetHandler(eventType) - this.pools.set(poolName, newPool) - } else { - this.removeTargetHandler(eventType) - this.pools.delete(poolName) - } + this.removeTargetHandler(eventType) + this.pools.set(poolName, newPool) if (this.pools.size > 0) this.addTargetHandler(eventType) } diff --git a/test/specs/lib/eventStack/EventStack-test.js b/test/specs/lib/eventStack/EventStack-test.js index 0b2db6b2cf..0f79f5f0ae 100644 --- a/test/specs/lib/eventStack/EventStack-test.js +++ b/test/specs/lib/eventStack/EventStack-test.js @@ -104,6 +104,20 @@ describe('EventStack', () => { keyHandler.should.have.not.been.called() }) + it('unsubscribes and leaves the default eventPool intact', () => { + const clickHandler = sandbox.spy() + const anotherClickHandler = sandbox.spy() + + eventStack.sub('click', clickHandler) + eventStack.unsub('mouseup', clickHandler) + eventStack.sub('click', anotherClickHandler) + + domEvent.click(document) + + clickHandler.should.have.been.calledOnce() + anotherClickHandler.should.have.been.calledOnce() + }) + it('unsubscribes from same event multiple times', () => { const handler = sandbox.spy() From fbc213401236b611cbd1335a75b71e667bc150bf Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Tue, 10 Jul 2018 13:04:34 +0300 Subject: [PATCH 3/4] rename handlers in tests Signed-off-by: Oleksandr Fediashov --- test/specs/lib/eventStack/EventStack-test.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/specs/lib/eventStack/EventStack-test.js b/test/specs/lib/eventStack/EventStack-test.js index 0f79f5f0ae..42ae2c9bda 100644 --- a/test/specs/lib/eventStack/EventStack-test.js +++ b/test/specs/lib/eventStack/EventStack-test.js @@ -105,17 +105,17 @@ describe('EventStack', () => { }) it('unsubscribes and leaves the default eventPool intact', () => { - const clickHandler = sandbox.spy() - const anotherClickHandler = sandbox.spy() + const firstHandler = sandbox.spy() + const secondHandler = sandbox.spy() - eventStack.sub('click', clickHandler) - eventStack.unsub('mouseup', clickHandler) - eventStack.sub('click', anotherClickHandler) + eventStack.sub('click', firstHandler) + eventStack.unsub('mouseup', firstHandler) + eventStack.sub('click', secondHandler) domEvent.click(document) - clickHandler.should.have.been.calledOnce() - anotherClickHandler.should.have.been.calledOnce() + firstHandler.should.have.been.calledOnce() + secondHandler.should.have.been.calledOnce() }) it('unsubscribes from same event multiple times', () => { From 0dd66a28ea23227afc50a20fd7b33269f9996949 Mon Sep 17 00:00:00 2001 From: Oleksandr Fediashov Date: Tue, 10 Jul 2018 13:50:27 +0300 Subject: [PATCH 4/4] refactor hasHandlers Signed-off-by: Oleksandr Fediashov --- src/lib/eventStack/EventPool.js | 9 +++------ src/lib/eventStack/EventTarget.js | 7 ++++++- test/specs/lib/eventStack/EventPool-test.js | 10 ++++++---- test/specs/lib/eventStack/EventStack-test.js | 4 ++-- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/src/lib/eventStack/EventPool.js b/src/lib/eventStack/EventPool.js index ffab8e9643..caa2f6cddc 100644 --- a/src/lib/eventStack/EventPool.js +++ b/src/lib/eventStack/EventPool.js @@ -53,13 +53,10 @@ export default class EventPool { } /** - * @param {String} eventType + * @return {Boolean} */ - hasHandlers(eventType) { - const handlerSet = this.handlerSets.get(eventType) - - if (handlerSet) return handlerSet.hasHandlers() - return false + hasHandlers() { + return this.handlerSets.size > 0 } /** diff --git a/src/lib/eventStack/EventTarget.js b/src/lib/eventStack/EventTarget.js index f41589b6ee..a3c41be007 100644 --- a/src/lib/eventStack/EventTarget.js +++ b/src/lib/eventStack/EventTarget.js @@ -49,8 +49,13 @@ export default class EventTarget { if (pool) { const newPool = pool.removeHandlers(eventType, eventHandlers) + if (newPool.hasHandlers()) { + this.pools.set(poolName, newPool) + } else { + this.pools.delete(poolName) + } + this.removeTargetHandler(eventType) - this.pools.set(poolName, newPool) if (this.pools.size > 0) this.addTargetHandler(eventType) } diff --git a/test/specs/lib/eventStack/EventPool-test.js b/test/specs/lib/eventStack/EventPool-test.js index d65157d2b7..8095e8f2f6 100644 --- a/test/specs/lib/eventStack/EventPool-test.js +++ b/test/specs/lib/eventStack/EventPool-test.js @@ -52,14 +52,16 @@ describe('EventPool', () => { }) describe('hasHandlers', () => { - const pool = EventPool.createByType('default', 'click', [() => {}]) - it('returns "true" if has handlers', () => { - pool.hasHandlers('click').should.have.be.true() + const pool = EventPool.createByType('default', 'click', [() => {}]) + + pool.hasHandlers().should.have.be.true() }) it('returns "false" if has not handlers', () => { - pool.hasHandlers('mousedown').should.have.be.false() + const pool = new EventPool('default', new Map()) + + pool.hasHandlers().should.have.be.false() }) }) diff --git a/test/specs/lib/eventStack/EventStack-test.js b/test/specs/lib/eventStack/EventStack-test.js index 42ae2c9bda..048d86184d 100644 --- a/test/specs/lib/eventStack/EventStack-test.js +++ b/test/specs/lib/eventStack/EventStack-test.js @@ -89,7 +89,7 @@ describe('EventStack', () => { handler.should.have.been.calledOnce() }) - it('unsubscribes but leaves eventTarget if it contains handlers', () => { + it('unsubscribes and leaves eventTarget if it contains handlers', () => { const clickHandler = sandbox.spy() const keyHandler = sandbox.spy() @@ -104,7 +104,7 @@ describe('EventStack', () => { keyHandler.should.have.not.been.called() }) - it('unsubscribes and leaves the default eventPool intact', () => { + it('unsubscribes and leaves an eventPool if contains handlers', () => { const firstHandler = sandbox.spy() const secondHandler = sandbox.spy()