fix(controls): stop event propagation from controls (#1210)

* fix(controls): stop event propagation from controls * Stop event propagation from preview controls, this is interfering with annotations. * Add new tests for Controls.js constructor * fix(controls): stop event propagation from controls * Remove comment * fix(controls): stop event propagation from controls * remove listener from has touch clause
box · May 19, 2020 · bd51261 · bd51261
1 parent 24dd77b
commit bd51261
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 5 deletions.
diff --git a/src/lib/Controls.js b/src/lib/Controls.js
@@ -44,10 +44,10 @@ class Controls {
         this.controlsEl.addEventListener('mouseleave', this.mouseleaveHandler);
         this.controlsEl.addEventListener('focusin', this.focusinHandler);
         this.controlsEl.addEventListener('focusout', this.focusoutHandler);
+        this.controlsEl.addEventListener('click', this.clickHandler);
 
         if (this.hasTouch) {
             this.containerEl.addEventListener('touchstart', this.mousemoveHandler);
-            this.controlsEl.addEventListener('click', this.clickHandler);
         }
     }
 
@@ -61,10 +61,10 @@ class Controls {
         this.controlsEl.removeEventListener('mouseleave', this.mouseleaveHandler);
         this.controlsEl.removeEventListener('focusin', this.focusinHandler);
         this.controlsEl.removeEventListener('focusout', this.focusoutHandler);
+        this.controlsEl.removeEventListener('click', this.clickHandler);
 
         if (this.hasTouch) {
             this.containerEl.removeEventListener('touchstart', this.mousemoveHandler);
-            this.controlsEl.removeEventListener('click', this.clickHandler);
         }
 
         this.buttonRefs.forEach(ref => {
@@ -175,9 +175,14 @@ class Controls {
      * @param {Event} event - A DOM-normalized event object.
      * @return {void}
      */
-    clickHandler = () => {
-        // If we are not focused in on the page num input, allow hiding after timeout
-        this.shouldHide = true;
+    clickHandler = event => {
+        if (event) {
+            event.stopPropagation();
+        }
+
+        if (this.hasTouch) {
+            this.shouldHide = true;
+        }
     };
 
     /**

diff --git a/src/lib/__tests__/Controls-test.js b/src/lib/__tests__/Controls-test.js
@@ -1,4 +1,5 @@
 /* eslint-disable no-unused-expressions */
+import Browser from '../Browser';
 import Controls from '../Controls';
 import { CLASS_HIDDEN } from '../constants';
 
@@ -32,11 +33,48 @@ describe('lib/Controls', () => {
     });
 
     describe('constructor()', () => {
+        let container;
+        let containerElEventListener;
+        let controlsElEventListener;
+
+        beforeEach(() => {
+            container = document.getElementById('test-controls-container');
+            const controlElement = document.createElement('div');
+            controlsElEventListener = sandbox.stub(controlElement, 'addEventListener');
+            sandbox.stub(container, 'appendChild').returns(controlElement);
+            containerElEventListener = sandbox.stub(container, 'addEventListener');
+        });
+
+        afterEach(() => {
+            container = undefined;
+            containerElEventListener = undefined;
+            controlsElEventListener = undefined;
+        });
+
         it('should create the correct DOM structure', () => {
             expect(controls.containerEl).to.equal(document.getElementById('test-controls-container'));
 
             expect(controls.controlsEl.classList.contains('bp-controls')).to.true;
         });
+
+        it('should add the correct event listeners', () => {
+            sandbox.stub(Browser, 'hasTouch').returns(false);
+            controls = new Controls(container);
+
+            expect(containerElEventListener).to.be.calledWith('mousemove', controls.mousemoveHandler);
+            expect(controlsElEventListener).to.be.calledWith('mouseenter', controls.mouseenterHandler);
+            expect(controlsElEventListener).to.be.calledWith('mouseleave', controls.mouseleaveHandler);
+            expect(controlsElEventListener).to.be.calledWith('focusin', controls.focusinHandler);
+            expect(controlsElEventListener).to.be.calledWith('focusout', controls.focusoutHandler);
+            expect(controlsElEventListener).to.be.calledWith('click', controls.clickHandler);
+        });
+
+        it('should add the correct event listeners when browser has touch', () => {
+            sandbox.stub(Browser, 'hasTouch').returns(true);
+            controls = new Controls(container);
+
+            expect(containerElEventListener).to.be.calledWith('touchstart', controls.mousemoveHandler);
+        });
     });
 
     describe('destroy()', () => {
@@ -249,9 +287,19 @@ describe('lib/Controls', () => {
 
     describe('clickHandler()', () => {
         it('should stop block hiding', () => {
+            controls.hasTouch = true;
+
             controls.clickHandler();
             expect(controls.shouldHide).to.be.true;
         });
+
+        it('should call stopPropagation on event when called', () => {
+            const stopPropagation = sandbox.stub();
+
+            controls.clickHandler({ stopPropagation });
+
+            expect(stopPropagation).to.be.called;
+        });
     });
 
     describe('add()', () => {