Fix: Fix focus for new page number input (#187)

* Fix: Fix focus in new page number input * Chore: Defining class variables with JSDoc
box · Jun 28, 2017 · 5f82f15 · 5f82f15
1 parent e6e6c3b
commit 5f82f15
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 13 deletions.
diff --git a/src/lib/Controls.js b/src/lib/Controls.js
@@ -4,9 +4,31 @@ import { CLASS_HIDDEN } from './constants';
 
 const SHOW_PREVIEW_CONTROLS_CLASS = 'box-show-preview-controls';
 const CONTROLS_BUTTON_CLASS = 'bp-controls-btn';
+const CONTROL_PAGE_NUM_WRAPPER_CLASS = 'bp-doc-page-num-wrapper';
 const CONTROLS_AUTO_HIDE_TIMEOUT_IN_MILLIS = 1500;
 
 class Controls {
+    /**
+     * Controls container element
+     *
+     * @property {HTMLElement}
+     */
+    containerEl;
+
+    /**
+     * Controls element
+     *
+     * @property {HTMLElement}
+     */
+    controlsEl;
+
+    /**
+     * Array of buttons for cleanup purposes
+     *
+     * @property {Array}
+     */
+    buttonRefs = [];
+
     /**
      * Indicates if the control bar should be hidden or not
      *
@@ -21,23 +43,26 @@ class Controls {
      */
     isFocused = false;
 
+    /**
+     * Indicates if the browser supports touch events
+     *
+     * @property {boolean}
+     */
+    hasTouch = Browser.hasTouch();
+
     /**
      * [constructor]
      *
      * @param {HTMLElement} container - The container
      * @return {Controls} Instance of controls
      */
     constructor(container) {
-        // Maintain a list of buttons for cleanup
-        this.buttonRefs = [];
-
-        // Container for the buttons
         this.containerEl = container;
 
-        this.controlsWrapperEl = this.containerEl.appendChild(document.createElement('div'));
-        this.controlsWrapperEl.className = 'bp-controls-wrapper';
+        const controlsWrapperEl = this.containerEl.appendChild(document.createElement('div'));
+        controlsWrapperEl.className = 'bp-controls-wrapper';
 
-        this.controlsEl = this.controlsWrapperEl.appendChild(document.createElement('div'));
+        this.controlsEl = controlsWrapperEl.appendChild(document.createElement('div'));
         this.controlsEl.className = 'bp-controls';
 
         this.containerEl.addEventListener('mousemove', this.mousemoveHandler);
@@ -46,7 +71,7 @@ class Controls {
         this.controlsEl.addEventListener('focusin', this.focusinHandler);
         this.controlsEl.addEventListener('focusout', this.focusoutHandler);
 
-        if (Browser.hasTouch()) {
+        if (this.hasTouch) {
             this.containerEl.addEventListener('touchstart', this.mousemoveHandler);
             this.controlsEl.addEventListener('click', this.clickHandler);
         }
@@ -63,7 +88,7 @@ class Controls {
         this.controlsEl.removeEventListener('focusin', this.focusinHandler);
         this.controlsEl.removeEventListener('focusout', this.focusoutHandler);
 
-        if (Browser.hasTouch()) {
+        if (this.hasTouch) {
             this.containerEl.removeEventListener('touchstart', this.mousemoveHandler);
             this.controlsEl.removeEventListener('click', this.clickHandler);
         }
@@ -84,7 +109,7 @@ class Controls {
         return (
             !!element &&
             (element.classList.contains(CONTROLS_BUTTON_CLASS) ||
-                element.parentNode.classList.contains(CONTROLS_BUTTON_CLASS))
+                element.parentNode.classList.contains(CONTROL_PAGE_NUM_WRAPPER_CLASS))
         );
     }
 

diff --git a/src/lib/__tests__/Controls-test.js b/src/lib/__tests__/Controls-test.js
@@ -34,10 +34,11 @@ describe('lib/Controls', () => {
         it('should create the correct DOM structure', () => {
             expect(controls.containerEl).to.equal(document.getElementById('test-controls-container'));
 
-            expect(controls.controlsWrapperEl.parentNode).to.equal(controls.containerEl);
-            expect(controls.controlsWrapperEl.classList.contains('bp-controls-wrapper')).to.true;
+            const controlsWrapperEl = controls.controlsEl.parentNode;
+            expect(controlsWrapperEl.parentNode).to.equal(controls.containerEl);
+            expect(controlsWrapperEl.classList.contains('bp-controls-wrapper')).to.true;
 
-            expect(controls.controlsEl.parentNode).to.equal(controls.controlsWrapperEl);
+            expect(controls.controlsEl.parentNode).to.equal(controlsWrapperEl);
             expect(controls.controlsEl.classList.contains('bp-controls')).to.true;
         });
     });
@@ -46,13 +47,16 @@ describe('lib/Controls', () => {
         it('should remove the correct event listeners', () => {
             const containerElEventListener = sandbox.stub(controls.containerEl, 'removeEventListener');
             const controlsElEventListener = sandbox.stub(controls.controlsEl, 'removeEventListener');
+            controls.hasTouch = true;
 
             controls.destroy();
             expect(containerElEventListener).to.be.calledWith('mousemove', controls.mousemoveHandler);
+            expect(containerElEventListener).to.be.calledWith('touchstart', 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 remove click listeners for any button references', () => {
@@ -87,6 +91,9 @@ describe('lib/Controls', () => {
 
             element.className = '';
             expect(controls.isPreviewControlButton(element)).to.be.false;
+
+            parent.className = 'bp-doc-page-num-wrapper';
+            expect(controls.isPreviewControlButton(element)).to.be.true;
         });
     });