From 81c7d75d4c3b9cd3bddd81a6a28660d251d0f737 Mon Sep 17 00:00:00 2001 From: Robert Concepcion III Date: Tue, 25 May 2021 11:08:29 -0400 Subject: [PATCH 1/7] ids-button: changes for #139 accessibility (allow role=presentation or tabindex=-1) --- src/ids-button/ids-button.js | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/ids-button/ids-button.js b/src/ids-button/ids-button.js index b8615c40b2..b8c942d8db 100644 --- a/src/ids-button/ids-button.js +++ b/src/ids-button/ids-button.js @@ -12,6 +12,8 @@ import { IdsRenderLoopMixin, IdsRenderLoopItem } from '../ids-mixins/ids-render- import styles from './ids-button.scss'; +const { stringToBool } = stringUtils; + // Button Styles const BUTTON_TYPES = [ 'default', @@ -106,7 +108,10 @@ class IdsButton extends mix(IdsElement).with( this.setIconAlignment(); this.shouldUpdate = true; super.connectedCallback(); - this.setAttribute('role', 'button'); + + if (!this.hasAttribute('role')) { + this.setAttribute('role', 'button'); + } } /** @@ -286,16 +291,19 @@ class IdsButton extends mix(IdsElement).with( * @param {boolean|string} val true if the button will be disabled */ set disabled(val) { + const isValueTruthy = stringToBool(val); this.shouldUpdate = false; - this.removeAttribute(props.DISABLED); - this.shouldUpdate = true; + if (isValueTruthy) { + this.setAttribute(props.DISABLED, ''); + } else { + this.removeAttribute(props.DISABLED); + } - const trueVal = stringUtils.stringToBool(val); - this.state.disabled = trueVal; + this.state.disabled = isValueTruthy; /* istanbul ignore next */ if (this.button) { - this.button.disabled = trueVal; + this.button.disabled = isValueTruthy; } } @@ -309,9 +317,6 @@ class IdsButton extends mix(IdsElement).with( * @returns {void} */ set tabIndex(val) { - // Remove the webcomponent tabIndex - this.shouldUpdate = false; - this.removeAttribute(props.TABINDEX); this.shouldUpdate = true; const trueVal = Number(val); From 3377e7e7a4d5b5f79ad6c74b8fce161a72ea528c Mon Sep 17 00:00:00 2001 From: Robert Concepcion III Date: Tue, 25 May 2021 11:28:06 -0400 Subject: [PATCH 2/7] ids-button: remove tabindex attrib on NaN values --- src/ids-button/ids-button.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ids-button/ids-button.js b/src/ids-button/ids-button.js index b8c942d8db..34d8c34b86 100644 --- a/src/ids-button/ids-button.js +++ b/src/ids-button/ids-button.js @@ -323,8 +323,10 @@ class IdsButton extends mix(IdsElement).with( if (Number.isNaN(trueVal) || trueVal < -1) { this.state.tabIndex = 0; this.button.setAttribute(props.TABINDEX, '0'); + this.removeAttribute(props.TABINDEX); return; } + this.state.tabIndex = trueVal; this.button.setAttribute(props.TABINDEX, `${trueVal}`); } From 226bbc2baa3828ea91d4dc291d336c106dc67547 Mon Sep 17 00:00:00 2001 From: Robert Concepcion III Date: Tue, 25 May 2021 11:29:05 -0400 Subject: [PATCH 3/7] ids-button: update tests now that tabindex in certain scenarios and disabled is reflected as well on WC --- test/ids-button/ids-button-func-test.js | 4 ++-- test/ids-button/ids-button-func-test.js.snap | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/test/ids-button/ids-button-func-test.js b/test/ids-button/ids-button-func-test.js index cfff8fe592..3943623cfb 100644 --- a/test/ids-button/ids-button-func-test.js +++ b/test/ids-button/ids-button-func-test.js @@ -68,7 +68,7 @@ describe('IdsButton Component', () => { it('can be disabled/enabled', () => { btn.disabled = true; - expect(btn.hasAttribute('disabled')).toBeFalsy(); + expect(btn.hasAttribute('disabled')).toBeTruthy(); expect(btn.disabled).toBeTruthy(); expect(btn.button.hasAttribute('disabled')).toBeTruthy(); expect(btn.state.disabled).toBeTruthy(); @@ -105,7 +105,7 @@ describe('IdsButton Component', () => { btn.setAttribute('tabindex', '0'); - expect(btn.hasAttribute('tabindex')).toBeFalsy(); + expect(btn.hasAttribute('tabindex')).toBeTruthy(); expect(btn.tabIndex).toEqual(0); expect(btn.button.getAttribute('tabindex')).toEqual('0'); expect(btn.state.tabIndex).toEqual(0); diff --git a/test/ids-button/ids-button-func-test.js.snap b/test/ids-button/ids-button-func-test.js.snap index d6867768fe..6d0caaba47 100644 --- a/test/ids-button/ids-button-func-test.js.snap +++ b/test/ids-button/ids-button-func-test.js.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`IdsButton Component renders correctly 1`] = `"test"`; +exports[`IdsButton Component renders correctly 1`] = `"test"`; exports[`IdsButton Component renders icons on the opposite side correctly 1`] = `"Settings"`; From c15f2010ba7a875325d7e4e5fc74eb3e68bd064d Mon Sep 17 00:00:00 2001 From: Robert Concepcion III Date: Wed, 26 May 2021 16:21:57 -0400 Subject: [PATCH 4/7] ids-button: remove "role" setter on lightDOM to fix axe tools issues popping up --- src/ids-button/ids-button.js | 4 ---- test/ids-button/ids-button-func-test.js.snap | 4 ++-- test/ids-menu-button/ids-menu-button-func-test.js.snap | 2 +- test/ids-theme-switcher/ids-theme-switcher-func-test.js.snap | 2 +- test/ids-trigger-button/ids-trigger-button-func-test.js.snap | 2 +- 5 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/ids-button/ids-button.js b/src/ids-button/ids-button.js index 34d8c34b86..be53c39f35 100644 --- a/src/ids-button/ids-button.js +++ b/src/ids-button/ids-button.js @@ -108,10 +108,6 @@ class IdsButton extends mix(IdsElement).with( this.setIconAlignment(); this.shouldUpdate = true; super.connectedCallback(); - - if (!this.hasAttribute('role')) { - this.setAttribute('role', 'button'); - } } /** diff --git a/test/ids-button/ids-button-func-test.js.snap b/test/ids-button/ids-button-func-test.js.snap index 6d0caaba47..9655f126d5 100644 --- a/test/ids-button/ids-button-func-test.js.snap +++ b/test/ids-button/ids-button-func-test.js.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`IdsButton Component renders correctly 1`] = `"test"`; +exports[`IdsButton Component renders correctly 1`] = `"test"`; -exports[`IdsButton Component renders icons on the opposite side correctly 1`] = `"Settings"`; +exports[`IdsButton Component renders icons on the opposite side correctly 1`] = `"Settings"`; diff --git a/test/ids-menu-button/ids-menu-button-func-test.js.snap b/test/ids-menu-button/ids-menu-button-func-test.js.snap index c2ae5bd7fd..fa2058d51e 100644 --- a/test/ids-menu-button/ids-menu-button-func-test.js.snap +++ b/test/ids-menu-button/ids-menu-button-func-test.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`IdsMenuButton Component renders correctly 1`] = `""`; +exports[`IdsMenuButton Component renders correctly 1`] = `""`; diff --git a/test/ids-theme-switcher/ids-theme-switcher-func-test.js.snap b/test/ids-theme-switcher/ids-theme-switcher-func-test.js.snap index bd1053ef47..aa89613c60 100644 --- a/test/ids-theme-switcher/ids-theme-switcher-func-test.js.snap +++ b/test/ids-theme-switcher/ids-theme-switcher-func-test.js.snap @@ -1,7 +1,7 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP exports[`IdsThemeSwitcher Component renders correctly 1`] = ` -" +" Theme Switcher diff --git a/test/ids-trigger-button/ids-trigger-button-func-test.js.snap b/test/ids-trigger-button/ids-trigger-button-func-test.js.snap index 3310a14187..ff72d98c5b 100644 --- a/test/ids-trigger-button/ids-trigger-button-func-test.js.snap +++ b/test/ids-trigger-button/ids-trigger-button-func-test.js.snap @@ -1,3 +1,3 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`IdsTriggerButton Component renders correctly 1`] = `""`; +exports[`IdsTriggerButton Component renders correctly 1`] = `""`; From e038e26e3625af7869fbd32acacd9aed4aeb8a75 Mon Sep 17 00:00:00 2001 From: Robert Concepcion III Date: Thu, 27 May 2021 17:01:04 -0400 Subject: [PATCH 5/7] re: PR -- push button suggestions + fix an issue removing tabIndex from button if needed --- src/ids-button/ids-button.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/ids-button/ids-button.js b/src/ids-button/ids-button.js index be53c39f35..e50c99ef1b 100644 --- a/src/ids-button/ids-button.js +++ b/src/ids-button/ids-button.js @@ -90,10 +90,13 @@ class IdsButton extends mix(IdsElement).with( switch (name) { // Convert "tabindex" to "tabIndex" case 'tabindex': + if (Number.isNaN(Number.parseInt(newValue))) { + this.tabIndex = null; + } this.tabIndex = Number(newValue); break; default: - IdsElement.prototype.attributeChangedCallback.apply(this, [name, oldValue, newValue]); + super.attributeChangedCallback.apply(this, [name, oldValue, newValue]); break; } } @@ -295,6 +298,7 @@ class IdsButton extends mix(IdsElement).with( this.removeAttribute(props.DISABLED); } + this.shouldUpdate = true; this.state.disabled = isValueTruthy; /* istanbul ignore next */ @@ -316,6 +320,7 @@ class IdsButton extends mix(IdsElement).with( this.shouldUpdate = true; const trueVal = Number(val); + if (Number.isNaN(trueVal) || trueVal < -1) { this.state.tabIndex = 0; this.button.setAttribute(props.TABINDEX, '0'); From 8692d42e5e18ac32edad2cd4b3c998e7e3836a40 Mon Sep 17 00:00:00 2001 From: Robert Concepcion III Date: Fri, 28 May 2021 11:37:19 -0400 Subject: [PATCH 6/7] update trigger field snap after rebase/merge with main --- test/ids-trigger-field/ids-trigger-field-func-test.js.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ids-trigger-field/ids-trigger-field-func-test.js.snap b/test/ids-trigger-field/ids-trigger-field-func-test.js.snap index 263ae205fa..df7fc9bb74 100644 --- a/test/ids-trigger-field/ids-trigger-field-func-test.js.snap +++ b/test/ids-trigger-field/ids-trigger-field-func-test.js.snap @@ -1,5 +1,5 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`IdsTriggerField Component renders correctly 1`] = `""`; +exports[`IdsTriggerField Component renders correctly 1`] = `""`; -exports[`IdsTriggerField Component renders correctly 2`] = `""`; +exports[`IdsTriggerField Component renders correctly 2`] = `""`; From 1a8d49026aa2d2a6073097df50fdcbc376c317fc Mon Sep 17 00:00:00 2001 From: Ed Coyle Date: Tue, 1 Jun 2021 09:22:46 -0400 Subject: [PATCH 7/7] Remove extraneous change to 'shouldUpdate' flag in tabindex setter --- src/ids-button/ids-button.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ids-button/ids-button.js b/src/ids-button/ids-button.js index e50c99ef1b..bc5b9f7a9e 100644 --- a/src/ids-button/ids-button.js +++ b/src/ids-button/ids-button.js @@ -317,8 +317,6 @@ class IdsButton extends mix(IdsElement).with( * @returns {void} */ set tabIndex(val) { - this.shouldUpdate = true; - const trueVal = Number(val); if (Number.isNaN(trueVal) || trueVal < -1) {