From 76ccb9443524ecfcdf38292a5258aa189ba0e0a3 Mon Sep 17 00:00:00 2001 From: Thomas Allmer Date: Thu, 16 May 2019 08:19:49 +0200 Subject: [PATCH] fix(button): do not override user provided tabindex --- packages/button/src/LionButton.js | 31 +++++---- packages/button/test/lion-button.test.js | 82 ++++++++++++++++++------ 2 files changed, 83 insertions(+), 30 deletions(-) diff --git a/packages/button/src/LionButton.js b/packages/button/src/LionButton.js index 320d967df..4622dbb65 100644 --- a/packages/button/src/LionButton.js +++ b/packages/button/src/LionButton.js @@ -8,6 +8,14 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { type: Boolean, reflect: true, }, + role: { + type: String, + reflect: true, + }, + tabindex: { + type: Number, + reflect: true, + }, }; } @@ -93,10 +101,10 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { ]; } - update(changedProperties) { - super.update(changedProperties); - if (changedProperties.has('disabled')) { - this.__onDisabledChanged(); + _requestUpdate(name, oldValue) { + super._requestUpdate(name, oldValue); + if (name === 'disabled') { + this.__onDisabledChanged(oldValue); } } @@ -125,12 +133,13 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { constructor() { super(); this.disabled = false; + this.role = 'button'; + this.tabindex = 0; this.__keydownDelegationHandler = this.__keydownDelegationHandler.bind(this); } connectedCallback() { super.connectedCallback(); - this.__setupA11y(); this.__setupKeydownDelegation(); } @@ -144,11 +153,6 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { this.$$slot('_button').click(); } - __setupA11y() { - this.setAttribute('role', 'button'); - this.setAttribute('tabindex', this.disabled ? -1 : 0); - } - __setupKeydownDelegation() { this.addEventListener('keydown', this.__keydownDelegationHandler); } @@ -167,6 +171,11 @@ export class LionButton extends DelegateMixin(SlotMixin(LionLitElement)) { } __onDisabledChanged() { - this.setAttribute('tabindex', this.disabled ? -1 : 0); + if (this.disabled) { + this.__originalTabIndex = this.tabindex; + this.tabindex = -1; + } else { + this.tabindex = this.__originalTabIndex; + } } } diff --git a/packages/button/test/lion-button.test.js b/packages/button/test/lion-button.test.js index aa5c82534..8583aefa5 100644 --- a/packages/button/test/lion-button.test.js +++ b/packages/button/test/lion-button.test.js @@ -6,43 +6,87 @@ import '../lion-button.js'; describe('lion-button', () => { it('behaves like native `button` in terms of a11y', async () => { - const lionButton = await fixture(`foo`); - expect(lionButton.getAttribute('role')).to.equal('button'); - expect(lionButton.getAttribute('tabindex')).to.equal('0'); + const el = await fixture(`foo`); + expect(el.getAttribute('role')).to.equal('button'); + expect(el.getAttribute('tabindex')).to.equal('0'); }); it('has no type by default on the native button', async () => { - const lionButton = await fixture(`foo`); - const nativeButton = lionButton.$$slot('_button'); + const el = await fixture(`foo`); + const nativeButton = el.$$slot('_button'); expect(nativeButton.getAttribute('type')).to.be.null; }); it('has type="submit" on the native button when set', async () => { - const lionButton = await fixture(`foo`); - const nativeButton = lionButton.$$slot('_button'); + const el = await fixture(`foo`); + const nativeButton = el.$$slot('_button'); expect(nativeButton.getAttribute('type')).to.equal('submit'); }); it('hides the native button in the UI', async () => { - const lionButton = await fixture(`foo`); - const nativeButton = lionButton.$$slot('_button'); + const el = await fixture(`foo`); + const nativeButton = el.$$slot('_button'); expect(nativeButton.getAttribute('tabindex')).to.equal('-1'); expect(window.getComputedStyle(nativeButton).visibility).to.equal('hidden'); }); it('can be disabled imperatively', async () => { - const lionButton = await fixture(`foo`); - expect(lionButton.getAttribute('tabindex')).to.equal('-1'); + const el = await fixture(`foo`); + expect(el.getAttribute('tabindex')).to.equal('-1'); - lionButton.disabled = false; - await lionButton.updateComplete; - expect(lionButton.getAttribute('tabindex')).to.equal('0'); - expect(lionButton.hasAttribute('disabled')).to.equal(false); + el.disabled = false; + await el.updateComplete; + expect(el.getAttribute('tabindex')).to.equal('0'); + expect(el.hasAttribute('disabled')).to.equal(false); - lionButton.disabled = true; - await lionButton.updateComplete; - expect(lionButton.getAttribute('tabindex')).to.equal('-1'); - expect(lionButton.hasAttribute('disabled')).to.equal(true); + el.disabled = true; + await el.updateComplete; + expect(el.getAttribute('tabindex')).to.equal('-1'); + expect(el.hasAttribute('disabled')).to.equal(true); + }); + + describe('a11y', () => { + it('has a role="button" by default', async () => { + const el = await fixture(`foo`); + expect(el.getAttribute('role')).to.equal('button'); + el.role = 'foo'; + await el.updateComplete; + expect(el.getAttribute('role')).to.equal('foo'); + }); + + it('does not override user provided role', async () => { + const el = await fixture(`foo`); + expect(el.getAttribute('role')).to.equal('foo'); + }); + + it('has a tabindex="0" by default', async () => { + const el = await fixture(`foo`); + expect(el.getAttribute('tabindex')).to.equal('0'); + }); + + it('has a tabindex="-1" when disabled', async () => { + const el = await fixture(`foo`); + expect(el.getAttribute('tabindex')).to.equal('-1'); + el.disabled = false; + await el.updateComplete; + expect(el.getAttribute('tabindex')).to.equal('0'); + el.disabled = true; + await el.updateComplete; + expect(el.getAttribute('tabindex')).to.equal('-1'); + }); + + it('does not override user provided tabindex', async () => { + const el = await fixture(`foo`); + expect(el.getAttribute('tabindex')).to.equal('5'); + }); + + it('disabled does not override user provided tabindex', async () => { + const el = await fixture(`foo`); + expect(el.getAttribute('tabindex')).to.equal('-1'); + el.disabled = false; + await el.updateComplete; + expect(el.getAttribute('tabindex')).to.equal('5'); + }); }); describe('form integration', () => {