fix: do not set focus-ring on mousedown focus (#2963)

vaadin · Nov 1, 2021 · c76f6e2 · c76f6e2
1 parent f5d556c
commit c76f6e2
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 4 deletions.
diff --git a/packages/accordion/test/accordion.test.js b/packages/accordion/test/accordion.test.js
@@ -230,6 +230,7 @@ describe('vaadin-accordion', () => {
         accordion.items[1].disabled = true;
         accordion.items[2].disabled = true;
         heading = getHeading(0);
+        arrowDownKeyDown(heading);
         expect(accordion.items[0].hasAttribute('focus-ring')).to.be.true;
       });
 

diff --git a/packages/component-base/src/focus-mixin.d.ts b/packages/component-base/src/focus-mixin.d.ts
@@ -14,6 +14,8 @@ interface FocusMixinConstructor {
 }
 
 interface FocusMixin {
+  readonly _keyboardActive: boolean;
+
   /**
    * Override to change how focused and focus-ring attributes are set.
    */

diff --git a/packages/component-base/src/focus-mixin.js b/packages/component-base/src/focus-mixin.js
@@ -35,6 +35,14 @@ window.addEventListener(
 export const FocusMixin = dedupingMixin(
   (superclass) =>
     class FocusMixinClass extends superclass {
+      /**
+       * @protected
+       * @return {boolean}
+       */
+      get _keyboardActive() {
+        return keyboardActive;
+      }
+
       /** @protected */
       ready() {
         this.addEventListener('focusin', (e) => {
@@ -78,7 +86,7 @@ export const FocusMixin = dedupingMixin(
 
         // focus-ring is true when the element was focused from the keyboard.
         // Focus Ring [A11ycasts]: https://youtu.be/ilj2P5-5CjI
-        this.toggleAttribute('focus-ring', focused && keyboardActive);
+        this.toggleAttribute('focus-ring', focused && this._keyboardActive);
       }
 
       /**

diff --git a/packages/password-field/src/vaadin-password-field.js b/packages/password-field/src/vaadin-password-field.js
@@ -218,7 +218,12 @@ export class PasswordField extends SlotStylesMixin(TextField) {
     );
   }
 
-  /** @protected */
+  /**
+   * Override method inherited from `FocusMixin` to toggle password visibility.
+   * @param {boolean} focused
+   * @protected
+   * @override
+   */
   _setFocused(focused) {
     super._setFocused(focused);
 
@@ -227,7 +232,7 @@ export class PasswordField extends SlotStylesMixin(TextField) {
     } else {
       const isButtonFocused = this.getRootNode().activeElement === this._revealNode;
       // Remove focus-ring from the field when the reveal button gets focused
-      this.toggleAttribute('focus-ring', !isButtonFocused);
+      this.toggleAttribute('focus-ring', this._keyboardActive && !isButtonFocused);
     }
   }
 

diff --git a/packages/password-field/test/password-field.test.js b/packages/password-field/test/password-field.test.js
@@ -1,7 +1,7 @@
 import { expect } from '@esm-bundle/chai';
 import { sendKeys } from '@web/test-runner-commands';
 import sinon from 'sinon';
-import { fixtureSync, focusout } from '@vaadin/testing-helpers';
+import { fixtureSync, focusout, mousedown } from '@vaadin/testing-helpers';
 import '../src/vaadin-password-field.js';
 
 describe('password-field', () => {
@@ -145,6 +145,15 @@ describe('password-field', () => {
         expect(passwordField.hasAttribute('focus-ring')).to.be.true;
       });
     });
+
+    describe('mousedown', () => {
+      it('should not set focus-ring attribute when focusing on mousedown', () => {
+        // reset FocusMixin flag
+        mousedown(passwordField);
+        passwordField.focus();
+        expect(passwordField.hasAttribute('focus-ring')).to.be.false;
+      });
+    });
   });
 
   describe('revealButtonHidden', () => {