refactor: update tabIndex logic to match standard HTML property

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

@@ -66,6 +66,16 @@ export const DelegateFocusMixin = dedupeMixin(
         this._boundOnFocus = this._onFocus.bind(this);
       }
 
+      /**
+       * Override to indicate that the component should not set tabindex attribute
+       * on the host element but instead delegate it to an element in light DOM.
+       * @protected
+       * @override
+       */
+      get _shouldUseHostTabIndex() {
+        return false;
+      }
+
       /** @protected */
       ready() {
         super.ready();
@@ -77,6 +87,17 @@ export const DelegateFocusMixin = dedupeMixin(
         }
       }
 
+      /** @protected */
+      updated(props) {
+        super.updated(props);
+
+        // Use `updated` instead of individual observer to make sure
+        // we forward `tabIndex` after `focusElement` is initialized.
+        if (props.has('tabIndex')) {
+          this.__forwardTabIndex(this.tabIndex);
+        }
+      }
+
       /**
        * @param {FocusOptions=} options
        * @protected
@@ -205,26 +226,14 @@ export const DelegateFocusMixin = dedupeMixin(
         }
       }
 
-      /**
-       * Override an observer from `TabindexMixin`.
-       * Do not call super to remove tabindex attribute
-       * from the host after it has been forwarded.
-       * @param {string} tabindex
-       * @protected
-       * @override
-       */
-      _tabindexChanged(tabindex) {
-        this.__forwardTabIndex(tabindex);
-      }
-
       /** @private */
       __forwardTabIndex(tabindex) {
-        if (tabindex !== undefined && this.focusElement) {
+        if (tabindex != null && this.focusElement) {
           this.focusElement.tabIndex = tabindex;
 
           // Preserve tabindex="-1" on the host element
           if (tabindex !== -1) {
-            this.tabindex = undefined;
+            this.removeAttribute('tabindex');
           }
         }
 
@@ -233,11 +242,6 @@ export const DelegateFocusMixin = dedupeMixin(
           if (tabindex !== -1) {
             this._lastTabIndex = tabindex;
           }
-          this.tabindex = undefined;
-        }
-
-        // Lit does not remove attribute when setting property to undefined
-        if (tabindex === undefined && this.hasAttribute('tabindex')) {
           this.removeAttribute('tabindex');
         }
       }

packages/a11y-base/src/tabindex-mixin.d.ts

@@ -20,7 +20,7 @@ export declare class TabindexMixinClass {
   /**
    * Indicates whether the element can be focused and where it participates in sequential keyboard navigation.
    */
-  tabindex: number | null | undefined;
+  tabIndex: number;
 
   /**
    * Stores the last known tabindex since the element has been disabled.

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

@@ -23,9 +23,10 @@ export const TabindexMixin = (superclass) =>
          *
          * @protected
          */
-        tabindex: {
+        tabIndex: {
           type: Number,
           reflectToAttribute: true,
+          attribute: 'tabindex',
           observer: '_tabindexChanged',
           sync: true,
         },
@@ -41,6 +42,15 @@ export const TabindexMixin = (superclass) =>
       };
     }
 
+    /**
+     * Override to indicate that the component should not set tabindex attribute
+     * on the host element but instead delegate it to an element in light DOM.
+     * @protected
+     */
+    get _shouldUseHostTabIndex() {
+      return true;
+    }
+
     /**
      * When the element gets disabled, the observer saves the last known tabindex
      * and makes the element not focusable by setting tabindex to -1.
@@ -58,16 +68,12 @@ export const TabindexMixin = (superclass) =>
       }
 
       if (disabled) {
-        if (this.tabindex !== undefined) {
-          this._lastTabIndex = this.tabindex;
+        if (this.tabIndex !== undefined) {
+          this._lastTabIndex = this.tabIndex;
         }
         this.setAttribute('tabindex', '-1');
       } else if (oldDisabled) {
-        if (this._lastTabIndex !== undefined) {
-          this.setAttribute('tabindex', this._lastTabIndex);
-        } else {
-          this.tabindex = undefined;
-        }
+        this.tabIndex = this._lastTabIndex;
       }
     }
 
@@ -83,6 +89,10 @@ export const TabindexMixin = (superclass) =>
         return;
       }
 
+      if (!this._shouldUseHostTabIndex) {
+        return;
+      }
+
       if (this.disabled && tabindex !== -1) {
         this._lastTabIndex = tabindex;
         this.setAttribute('tabindex', '-1');

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

@@ -21,19 +21,14 @@ describe('TabindexMixin', () => {
       expect(element.hasAttribute('tabindex')).to.be.false;
     });
 
-    it('should reflect tabindex property to the attribute', () => {
-      element.tabindex = 1;
-      expect(element.getAttribute('tabindex')).to.be.equal('1');
-    });
-
     it('should reflect native tabIndex property to the attribute', () => {
       element.tabIndex = 1;
       expect(element.getAttribute('tabindex')).to.be.equal('1');
     });
 
     it('should reflect tabindex attribute to the property', () => {
       element.setAttribute('tabindex', '1');
-      expect(element.tabindex).to.be.equal(1);
+      expect(element.tabIndex).to.be.equal(1);
     });
 
     it('should set tabindex attribute to -1 when disabled', () => {
@@ -85,7 +80,7 @@ describe('TabindexMixin', () => {
     });
 
     it('should set tabindex property to the custom value', () => {
-      expect(element.tabindex).to.equal(1);
+      expect(element.tabIndex).to.equal(1);
     });
   });
 });

packages/button/src/vaadin-button-mixin.js

@@ -29,7 +29,7 @@ export const ButtonMixin = (superClass) =>
       });
 
       // Set tabindex to 0 by default
-      this.tabindex = 0;
+      this.tabIndex = 0;
     }
 
     /**

packages/checkbox/src/vaadin-checkbox-mixin.js

@@ -94,7 +94,7 @@ export const CheckboxMixin = (superclass) =>
 
       // Set tabindex to 0 by default to not lose focus on click in Safari
       // See https://github.com/vaadin/web-components/pull/6780
-      this.tabindex = 0;
+      this.tabIndex = 0;
     }
 
     /** @protected */

packages/radio-group/src/vaadin-radio-button-mixin.js

@@ -55,7 +55,7 @@ export const RadioButtonMixin = (superclass) =>
 
       // Set tabindex to 0 by default to not lose focus on click in Safari
       // See https://github.com/vaadin/web-components/pull/6780
-      this.tabindex = 0;
+      this.tabIndex = 0;
     }
 
     /** @protected */