refactor: change focus-ring condition (#65)

vaadin · Sep 23, 2020 · fb90387 · fb90387
1 parent b4ee15d
commit fb90387
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 54 deletions.
diff --git a/test/control-state.html b/test/control-state.html
@@ -138,7 +138,7 @@
         });
       });
 
-      describe('_tabPressed and focus-ring', () => {
+      describe('focus-ring', () => {
         var focusin = () => {
           focusElement.dispatchEvent(new CustomEvent('focusin', {composed: true, bubbles: true}));
         };
@@ -147,20 +147,6 @@
           focusElement.dispatchEvent(new CustomEvent('focusout', {composed: true, bubbles: true}));
         };
 
-        it('should set and unset _tabPressed when press TAB', () => {
-          MockInteractions.keyDownOn(document.body, 9);
-          expect(customElement._tabPressed).to.be.true;
-          MockInteractions.keyUpOn(document.body, 9);
-          expect(customElement._tabPressed).to.be.false;
-        });
-
-        it('should set and unset _tabPressed when press SHIFT+TAB', () => {
-          MockInteractions.keyDownOn(document.body, 9, 'shift');
-          expect(customElement._tabPressed).to.be.true;
-          MockInteractions.keyUpOn(document.body, 9, 'shift');
-          expect(customElement._tabPressed).to.be.false;
-        });
-
         it('should set _isShiftTabbing when pressing shift-tab', () => {
           const event = MockInteractions.keyboardEventFor('keydown', 9, 'shift');
           customElement.dispatchEvent(event);
@@ -179,13 +165,6 @@
           expect(customElement._isShiftTabbing).not.to.be.ok;
         });
 
-        it('should not change _tabPressed on any other key except TAB', () => {
-          MockInteractions.keyDownOn(document.body, 65);
-          expect(customElement._tabPressed).to.be.false;
-          MockInteractions.keyUpOn(document.body, 65);
-          expect(customElement._tabPressed).to.be.false;
-        });
-
         it('should set the focus-ring attribute when TAB is pressed and focus is received', () => {
           MockInteractions.keyDownOn(document.body, 9);
           focusin();
@@ -202,6 +181,13 @@
           expect(customElement.hasAttribute('focus-ring')).to.be.false;
         });
 
+        it('should not set the focus-ring attribute when mousedown happens after keydown', () => {
+          MockInteractions.keyDownOn(document.body, 9);
+          document.body.dispatchEvent(new MouseEvent('mousedown'));
+          focusin();
+          expect(customElement.hasAttribute('focus-ring')).to.be.false;
+        });
+
         it('should refocus the field', (done) => {
           customElement.dispatchEvent(new CustomEvent('focusin'));
           MockInteractions.keyDownOn(customElement, 9, 'shift');

diff --git a/vaadin-control-state-mixin.html b/vaadin-control-state-mixin.html
@@ -5,6 +5,29 @@
 -->
 
 <script>
+(function() {
+  // We consider the keyboard to be active if the window has received a keydown
+  // event since the last mousedown event.
+  let keyboardActive = false;
+
+  // Listen for top-level keydown and mousedown events.
+  // Use capture phase so we detect events even if they're handled.
+  window.addEventListener(
+    'keydown',
+    () => {
+      keyboardActive = true;
+    },
+    {capture: true}
+  );
+
+  window.addEventListener(
+    'mousedown',
+    () => {
+      keyboardActive = false;
+    },
+    {capture: true}
+  );
+
   /**
    * @namespace Vaadin
    */
@@ -161,19 +184,6 @@
           this.setAttribute('focus-ring', '');
         });
       }
-
-      this._boundKeydownListener = this._bodyKeydownListener.bind(this);
-      this._boundKeyupListener = this._bodyKeyupListener.bind(this);
-    }
-
-    /**
-     * @protected
-     */
-    connectedCallback() {
-      super.connectedCallback();
-
-      document.body.addEventListener('keydown', this._boundKeydownListener, true);
-      document.body.addEventListener('keyup', this._boundKeyupListener, true);
     }
 
     /**
@@ -182,9 +192,6 @@
     disconnectedCallback() {
       super.disconnectedCallback();
 
-      document.body.removeEventListener('keydown', this._boundKeydownListener, true);
-      document.body.removeEventListener('keyup', this._boundKeyupListener, true);
-
       // in non-Chrome browsers, blur does not fire on the element when it is disconnected.
       // reproducible in `<vaadin-date-picker>` when closing on `Cancel` or `Today` click.
       if (this.hasAttribute('focused')) {
@@ -205,28 +212,13 @@
 
       // focus-ring is true when the element was focused from the keyboard.
       // Focus Ring [A11ycasts]: https://youtu.be/ilj2P5-5CjI
-      if (focused && this._tabPressed) {
+      if (focused && keyboardActive) {
         this.setAttribute('focus-ring', '');
       } else {
         this.removeAttribute('focus-ring');
       }
     }
 
-    /**
-     * @param {KeyboardEvent} e
-     * @private
-     */
-    _bodyKeydownListener(e) {
-      this._tabPressed = e.keyCode === 9;
-    }
-
-    /**
-     * @private
-     */
-    _bodyKeyupListener() {
-      this._tabPressed = false;
-    }
-
     /**
      * Any element extending this mixin is required to implement this getter.
      * It returns the actual focusable element in the component.
@@ -325,4 +317,5 @@
       }
     }
   };
+})();
 </script>