diff --git a/packages/a11y-base/src/delegate-focus-mixin.js b/packages/a11y-base/src/delegate-focus-mixin.js index c73f37b8f8..a8512fb861 100644 --- a/packages/a11y-base/src/delegate-focus-mixin.js +++ b/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'); } } diff --git a/packages/a11y-base/src/tabindex-mixin.d.ts b/packages/a11y-base/src/tabindex-mixin.d.ts index 43451b78fb..40e227992c 100644 --- a/packages/a11y-base/src/tabindex-mixin.d.ts +++ b/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. diff --git a/packages/a11y-base/src/tabindex-mixin.js b/packages/a11y-base/src/tabindex-mixin.js index 2c84ff4d03..dac1d3514a 100644 --- a/packages/a11y-base/src/tabindex-mixin.js +++ b/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'); diff --git a/packages/a11y-base/test/tabindex-mixin.test.js b/packages/a11y-base/test/tabindex-mixin.test.js index d8e9e19d82..4b2884b32d 100644 --- a/packages/a11y-base/test/tabindex-mixin.test.js +++ b/packages/a11y-base/test/tabindex-mixin.test.js @@ -21,11 +21,6 @@ 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'); @@ -33,7 +28,7 @@ describe('TabindexMixin', () => { 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); }); }); }); diff --git a/packages/button/src/vaadin-button-mixin.js b/packages/button/src/vaadin-button-mixin.js index 068616064e..844208e417 100644 --- a/packages/button/src/vaadin-button-mixin.js +++ b/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; } /** diff --git a/packages/checkbox/src/vaadin-checkbox-mixin.js b/packages/checkbox/src/vaadin-checkbox-mixin.js index a61c835565..9f7f3e0687 100644 --- a/packages/checkbox/src/vaadin-checkbox-mixin.js +++ b/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 */ diff --git a/packages/radio-group/src/vaadin-radio-button-mixin.js b/packages/radio-group/src/vaadin-radio-button-mixin.js index e4ffd7600c..0340e93edc 100644 --- a/packages/radio-group/src/vaadin-radio-button-mixin.js +++ b/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 */