refactor!: set focus-ring attribute on programmatic focus (#10049)

Author: web-padawan

packages/a11y-base/src/delegate-focus-mixin.js

@@ -73,18 +73,24 @@ export const DelegateFocusMixin = dedupeMixin(
         if (this.autofocus && !this.disabled) {
           requestAnimationFrame(() => {
             this.focus();
-            this.setAttribute('focus-ring', '');
           });
         }
       }
 
       /**
+       * @param {FocusOptions=} options
        * @protected
        * @override
        */
-      focus() {
+      focus(options) {
         if (this.focusElement && !this.disabled) {
           this.focusElement.focus();
+
+          // Set focus-ring attribute on programmatic focus by default
+          // unless explicitly disabled by `{ focusVisible: false }`.
+          if (!(options && options.focusVisible === false)) {
+            this.setAttribute('focus-ring', '');
+          }
         }
       }
 

packages/a11y-base/src/focus-mixin.js

@@ -54,6 +54,21 @@ export const FocusMixin = dedupeMixin(
         }
       }
 
+      /**
+       * @param {FocusOptions=} options
+       * @protected
+       * @override
+       */
+      focus(options) {
+        super.focus(options);
+
+        // Set focus-ring attribute on programmatic focus by default
+        // unless explicitly disabled by `{ focusVisible: false }`.
+        if (!(options && options.focusVisible === false)) {
+          this.setAttribute('focus-ring', '');
+        }
+      }
+
       /**
        * Override to change how focused and focus-ring attributes are set.
        *

packages/a11y-base/src/keyboard-direction-mixin.d.ts

@@ -32,12 +32,12 @@ export declare class KeyboardDirectionMixinClass {
   /**
    * Focus the item at given index. Override this method to add custom logic.
    */
-  protected _focus(index: number, navigating: boolean): void;
+  protected _focus(index: number, options: FocusOptions, navigating: boolean): void;
 
   /**
    * Focus the given item. Override this method to add custom logic.
    */
-  protected _focusItem(item: Element, navigating: boolean): void;
+  protected _focusItem(item: Element, options: FocusOptions, navigating: boolean): void;
 
   /**
    * Returns whether the item is focusable. By default,

packages/a11y-base/src/keyboard-direction-mixin.js

@@ -38,13 +38,17 @@ export const KeyboardDirectionMixin = (superclass) =>
       return false;
     }
 
-    /** @protected */
-    focus() {
+    /**
+     * @param {FocusOptions=} options
+     * @protected
+     * @override
+     */
+    focus(options) {
       const items = this._getItems();
       if (Array.isArray(items)) {
         const idx = this._getAvailableIndex(items, 0, null, (item) => !isElementHidden(item));
         if (idx >= 0) {
-          this._focus(idx);
+          this._focus(idx, options);
         }
       }
     }
@@ -114,7 +118,7 @@ export const KeyboardDirectionMixin = (superclass) =>
 
       if (idx >= 0) {
         event.preventDefault();
-        this._focus(idx, true);
+        this._focus(idx, { focusVisible: true }, true);
       }
     }
 
@@ -150,30 +154,27 @@ export const KeyboardDirectionMixin = (superclass) =>
      * Focus the item at given index. Override this method to add custom logic.
      *
      * @param {number} index
+     * @param {FocusOptions=} options
      * @param {boolean} navigating
      * @protected
      */
-    _focus(index, navigating = false) {
+    _focus(index, options, navigating = false) {
       const items = this._getItems();
 
-      this._focusItem(items[index], navigating);
+      this._focusItem(items[index], options, navigating);
     }
 
     /**
      * Focus the given item. Override this method to add custom logic.
      *
      * @param {Element} item
+     * @param {FocusOptions=} options
      * @param {boolean} navigating
      * @protected
      */
-    _focusItem(item) {
+    _focusItem(item, options) {
       if (item) {
-        item.focus();
-
-        // Generally, the items are expected to implement `FocusMixin`
-        // that would set this attribute based on the `keydown` event.
-        // We set it manually to handle programmatic focus() calls.
-        item.setAttribute('focus-ring', '');
+        item.focus(options);
       }
     }
 

packages/a11y-base/src/list-mixin.js

@@ -107,7 +107,12 @@ export const ListMixin = (superClass) =>
       return this.orientation !== 'horizontal';
     }
 
-    focus() {
+    /**
+     * @param {FocusOptions=} options
+     * @protected
+     * @override
+     */
+    focus(options) {
       // In initialization (e.g vaadin-select) observer might not been run yet.
       if (this._observer) {
         this._observer.flush();
@@ -116,10 +121,10 @@ export const ListMixin = (superClass) =>
       const items = Array.isArray(this.items) ? this.items : [];
       const idx = this._getAvailableIndex(items, 0, null, (item) => item.tabIndex === 0 && !isElementHidden(item));
       if (idx >= 0) {
-        this._focus(idx);
+        this._focus(idx, options);
       } else {
         // Call `KeyboardDirectionMixin` logic to focus first non-disabled item.
-        super.focus();
+        super.focus(options);
       }
     }
 
@@ -275,13 +280,13 @@ export const ListMixin = (superClass) =>
      * @param {number} idx
      * @protected
      */
-    _focus(idx) {
+    _focus(idx, options) {
       this.items.forEach((e, index) => {
         e.focused = index === idx;
       });
       this._setFocusable(idx);
       this._scrollToItem(idx);
-      super._focus(idx);
+      super._focus(idx, options);
     }
 
     /**

packages/a11y-base/src/tabindex-mixin.js

@@ -95,12 +95,13 @@ export const TabindexMixin = (superclass) =>
      * `tabindex` to -1 does not prevent the element from being
      * programmatically focusable.
      *
+     * @param {FocusOptions=} options
      * @protected
      * @override
      */
-    focus() {
+    focus(options) {
       if (!this.disabled || this.__shouldAllowFocusWhenDisabled()) {
-        super.focus();
+        super.focus(options);
       }
     }
 

packages/a11y-base/test/delegate-focus-mixin.test.js

@@ -82,6 +82,16 @@ describe('DelegateFocusMixin', () => {
       expect(element.hasAttribute('focused')).to.be.false;
     });
 
+    it('should set focus-ring attribute when calling focus() by default', () => {
+      element.focus();
+      expect(element.hasAttribute('focus-ring')).to.be.true;
+    });
+
+    it('should not set focus-ring attribute when calling focus() with focusVisible: false', () => {
+      element.focus({ focusVisible: false });
+      expect(element.hasAttribute('focus-ring')).to.be.false;
+    });
+
     it('should propagate disabled property to the input', () => {
       element.disabled = true;
       expect(input.disabled).to.be.true;

packages/a11y-base/test/focus-mixin.test.js

@@ -56,4 +56,14 @@ describe('FocusMixin', () => {
     focusin(input);
     expect(element.hasAttribute('focus-ring')).to.be.false;
   });
+
+  it('should set focus-ring attribute when calling focus() by default', () => {
+    element.focus();
+    expect(element.hasAttribute('focus-ring')).to.be.true;
+  });
+
+  it('should not set focus-ring attribute when calling focus() with focusVisible: false', () => {
+    element.focus({ focusVisible: false });
+    expect(element.hasAttribute('focus-ring')).to.be.false;
+  });
 });

packages/a11y-base/test/keyboard-direction-mixin.test.js

@@ -177,11 +177,6 @@ describe('KeyboardDirectionMixin', () => {
         expect(element.focused).to.be.equal(items[4]);
       });
 
-      it('should set focus-ring on the focused element on keydown', () => {
-        arrowDownKeyDown(items[0]);
-        expect(items[1].hasAttribute('focus-ring')).to.be.true;
-      });
-
       it('should not move focus on keydown with Ctrl key modifier', () => {
         const spy = sinon.spy(items[1], 'focus');
         arrowDownKeyDown(items[0], ['ctrl']);

packages/a11y-base/test/list-mixin.test.js

@@ -251,6 +251,15 @@ describe('ListMixin', () => {
       expect(spy.calledOnce).to.be.true;
     });
 
+    it('should pass focus options to the item when calling focus() on the list', async () => {
+      list._setFocusable(3);
+      await nextUpdate(list);
+      const spy = sinon.spy(list.items[3], 'focus');
+      list.focus({ focusVisible: false });
+      expect(spy.calledOnce).to.be.true;
+      expect(spy.firstCall.args[0]).to.deep.equal({ focusVisible: false });
+    });
+
     it('should call focus() on the first non-disabled item if all items have tabindex -1', () => {
       list.items.forEach((item) => {
         item.tabIndex = -1;