fix(EventStack): fix erroneous removal of non-empty EventPool (#2992)

* docs(EventStack): fix a numbering mistake * fix(EventStack): don't remove eventPool on unsub if not necessary Closes #2905 * rename handlers in tests Signed-off-by: Oleksandr Fediashov <[email protected]> * refactor hasHandlers Signed-off-by: Oleksandr Fediashov <[email protected]>
Semantic-Org · Jul 14, 2018 · a9e0aae · a9e0aae
1 parent 972b999
commit a9e0aae
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 15 deletions.
diff --git 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
@@ -49,14 +49,14 @@ export default class EventTarget {
     if (pool) {
       const newPool = pool.removeHandlers(eventType, eventHandlers)
 
-      if (newPool.hasHandlers(eventType)) {
-        this.removeTargetHandler(eventType)
+      if (newPool.hasHandlers()) {
         this.pools.set(poolName, newPool)
       } else {
-        this.removeTargetHandler(eventType)
         this.pools.delete(poolName)
       }
 
+      this.removeTargetHandler(eventType)
+
       if (this.pools.size > 0) this.addTargetHandler(eventType)
     }
   }

diff --git 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
 

diff --git 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
@@ -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,6 +104,20 @@ describe('EventStack', () => {
       keyHandler.should.have.not.been.called()
     })
 
+    it('unsubscribes and leaves an eventPool if contains handlers', () => {
+      const firstHandler = sandbox.spy()
+      const secondHandler = sandbox.spy()
+
+      eventStack.sub('click', firstHandler)
+      eventStack.unsub('mouseup', firstHandler)
+      eventStack.sub('click', secondHandler)
+
+      domEvent.click(document)
+
+      firstHandler.should.have.been.calledOnce()
+      secondHandler.should.have.been.calledOnce()
+    })
+
     it('unsubscribes from same event multiple times', () => {
       const handler = sandbox.spy()