fix: forward tabindex attribute to focusElement (#3416)

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

@@ -56,7 +56,9 @@ export const TabindexMixin = (superclass) =>
       super._disabledChanged(disabled, oldDisabled);
 
       if (disabled) {
-        this.__lastTabIndex = this.tabindex;
+        if (this.tabindex !== undefined) {
+          this.__lastTabIndex = this.tabindex;
+        }
         this.tabindex = -1;
       } else if (oldDisabled) {
         this.tabindex = this.__lastTabIndex;

packages/field-base/src/delegate-focus-mixin.d.ts

@@ -6,13 +6,18 @@
 import { Constructor } from '@open-wc/dedupe-mixin';
 import { DisabledMixinClass } from '@vaadin/component-base/src/disabled-mixin.js';
 import { FocusMixinClass } from '@vaadin/component-base/src/focus-mixin.js';
+import { TabindexMixinClass } from '@vaadin/component-base/src/tabindex-mixin.js';
 
 /**
  * A mixin to forward focus to an element in the light DOM.
  */
 export declare function DelegateFocusMixin<T extends Constructor<HTMLElement>>(
   base: T
-): T & Constructor<DelegateFocusMixinClass> & Constructor<DisabledMixinClass> & Constructor<FocusMixinClass>;
+): T &
+  Constructor<DelegateFocusMixinClass> &
+  Constructor<DisabledMixinClass> &
+  Constructor<FocusMixinClass> &
+  Constructor<TabindexMixinClass>;
 
 export declare class DelegateFocusMixinClass {
   /**

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

@@ -4,19 +4,19 @@
  * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
  */
 import { dedupingMixin } from '@polymer/polymer/lib/utils/mixin.js';
-import { DisabledMixin } from '@vaadin/component-base/src/disabled-mixin.js';
 import { FocusMixin } from '@vaadin/component-base/src/focus-mixin.js';
+import { TabindexMixin } from '@vaadin/component-base/src/tabindex-mixin.js';
 
 /**
  * A mixin to forward focus to an element in the light DOM.
  *
  * @polymerMixin
- * @mixes DisabledMixin
  * @mixes FocusMixin
+ * @mixes TabindexMixin
  */
 export const DelegateFocusMixin = dedupingMixin(
   (superclass) =>
-    class DelegateFocusMixinClass extends FocusMixin(DisabledMixin(superclass)) {
+    class DelegateFocusMixinClass extends FocusMixin(TabindexMixin(superclass)) {
       static get properties() {
         return {
           /**
@@ -40,6 +40,19 @@ export const DelegateFocusMixin = dedupingMixin(
             type: Object,
             readOnly: true,
             observer: '_focusElementChanged'
+          },
+
+          /**
+           * Indicates whether the element can be focused and where it participates in sequential keyboard navigation.
+           *
+           * By default, the host element does not have tabindex attribute. Instead, `focusElement` should have it.
+           * Toggling `tabindex` attribute on the host element propagates its value to `focusElement`.
+           *
+           * @protected
+           */
+          tabindex: {
+            type: Number,
+            value: undefined
           }
         };
       }
@@ -103,6 +116,7 @@ export const DelegateFocusMixin = dedupingMixin(
         if (element) {
           element.disabled = this.disabled;
           this._addFocusListeners(element);
+          this.__forwardTabIndex(this.tabindex);
         } else if (oldElement) {
           this._removeFocusListeners(oldElement);
         }
@@ -162,10 +176,12 @@ export const DelegateFocusMixin = dedupingMixin(
 
       /**
        * @param {boolean} disabled
+       * @param {boolean} oldDisabled
        * @protected
+       * @override
        */
-      _disabledChanged(disabled) {
-        super._disabledChanged(disabled);
+      _disabledChanged(disabled, oldDisabled) {
+        super._disabledChanged(disabled, oldDisabled);
 
         if (this.focusElement) {
           this.focusElement.disabled = disabled;
@@ -175,5 +191,37 @@ export const DelegateFocusMixin = dedupingMixin(
           this.blur();
         }
       }
+
+      /**
+       * 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) {
+          this.focusElement.tabIndex = tabindex;
+
+          // Preserve tabindex="-1" on the host element
+          if (tabindex !== -1) {
+            this.tabindex = undefined;
+          }
+        }
+
+        if (this.disabled && tabindex) {
+          // If tabindex attribute was changed while component was disabled
+          if (tabindex !== -1) {
+            this.__lastTabIndex = tabindex;
+          }
+          this.tabindex = undefined;
+        }
+      }
     }
 );

packages/field-base/src/shadow-focus-mixin.js

@@ -4,7 +4,6 @@
  * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
  */
 import { KeyboardMixin } from '@vaadin/component-base/src/keyboard-mixin.js';
-import { TabindexMixin } from '@vaadin/component-base/src/tabindex-mixin.js';
 import { DelegateFocusMixin } from './delegate-focus-mixin.js';
 
 /**
@@ -13,10 +12,23 @@ import { DelegateFocusMixin } from './delegate-focus-mixin.js';
  * @polymerMixin
  * @mixes DelegateFocusMixin
  * @mixes KeyboardMixin
- * @mixes TabindexMixin
  */
 export const ShadowFocusMixin = (superClass) =>
-  class ShadowFocusMixinClass extends TabindexMixin(DelegateFocusMixin(KeyboardMixin(superClass))) {
+  class ShadowFocusMixinClass extends DelegateFocusMixin(KeyboardMixin(superClass)) {
+    static get properties() {
+      return {
+        /**
+         * Indicates whether the element can be focused and where it participates in sequential keyboard navigation.
+         *
+         * @protected
+         */
+        tabindex: {
+          type: Number,
+          value: 0
+        }
+      };
+    }
+
     /**
      * Override an event listener from `KeyboardMixin`
      * to prevent setting `focused` on Shift Tab.

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

@@ -210,4 +210,62 @@ describe('delegate-focus-mixin', () => {
       expect(element.hasAttribute('focus-ring')).to.be.false;
     });
   });
+
+  describe('tabindex', () => {
+    describe('default', () => {
+      beforeEach(() => {
+        element = fixtureSync(`<delegate-focus-mixin-element></delegate-focus-mixin-element>`);
+        input = element.querySelector('input');
+      });
+
+      it('should forward tabindex set using property to the input', () => {
+        element.tabIndex = -1;
+        expect(input.getAttribute('tabindex')).to.equal('-1');
+      });
+
+      it('should forward tabindex set using attribute to the input', () => {
+        element.setAttribute('tabindex', '-1');
+        expect(input.getAttribute('tabindex')).to.equal('-1');
+      });
+
+      it('should set input tabindex to -1 when host element is disabled', () => {
+        element.disabled = true;
+        expect(input.getAttribute('tabindex')).to.equal('-1');
+      });
+
+      it('should restore input tabindex when host element is re-enabled', () => {
+        element.disabled = true;
+        element.disabled = false;
+        expect(input.tabIndex).to.equal(0);
+      });
+
+      it('should keep tabindex value changed while element is disabled', () => {
+        element.disabled = true;
+        element.setAttribute('tabindex', '1');
+        element.disabled = false;
+        expect(input.getAttribute('tabindex')).to.equal('1');
+      });
+    });
+
+    describe('attribute', () => {
+      beforeEach(() => {
+        element = fixtureSync(`<delegate-focus-mixin-element tabindex="-1"></delegate-focus-mixin-element>`);
+        input = element.querySelector('input');
+      });
+
+      it('should forward tabindex attribute value to the input', () => {
+        expect(input.getAttribute('tabindex')).to.equal('-1');
+      });
+
+      it('should update input tabindex when host attribute changed', () => {
+        element.setAttribute('tabindex', '0');
+        expect(input.getAttribute('tabindex')).to.equal('0');
+      });
+
+      it('should remove tabindex attribute from the host when changed', () => {
+        element.setAttribute('tabindex', '0');
+        expect(element.getAttribute('tabindex')).to.equal(null);
+      });
+    });
+  });
 });

packages/message-input/test/dom/__snapshots__/message-input.test.snap.js

@@ -42,6 +42,7 @@ snapshots["vaadin-message-input disabled"] =
     rows="1"
     slot="textarea"
     style="min-height: 0px;"
+    tabindex="-1"
   >
   </textarea>
   <label slot="label">