From d83aff030afb75098168eb8a3d945091e7bd4a4b Mon Sep 17 00:00:00 2001 From: Andrey Prigogin Date: Mon, 12 Mar 2018 16:15:03 -0700 Subject: [PATCH 1/3] fix(checkbox): added aria-checked=mixed to indeterminate state --- packages/mdc-checkbox/adapter.js | 13 +++++++++++++ packages/mdc-checkbox/constants.js | 2 ++ packages/mdc-checkbox/foundation.js | 12 ++++++++++++ test/unit/mdc-checkbox/foundation.test.js | 10 ++++++---- 4 files changed, 33 insertions(+), 4 deletions(-) diff --git a/packages/mdc-checkbox/adapter.js b/packages/mdc-checkbox/adapter.js index 48e583cd6f6..a68ff35a331 100644 --- a/packages/mdc-checkbox/adapter.js +++ b/packages/mdc-checkbox/adapter.js @@ -43,6 +43,19 @@ class MDCCheckboxAdapter { /** @param {string} className */ removeClass(className) {} + /** + * Sets an attribute with a given value on the input element. + * @param {string} attr + * @param {string} value + */ + setNativeControlAttr(attr, value) {} + + /** + * Removes an attribute from the input element. + * @param {string} attr + */ + removeNativeControlAttr(attr) {} + /** @param {!EventListener} handler */ registerAnimationEndHandler(handler) {} diff --git a/packages/mdc-checkbox/constants.js b/packages/mdc-checkbox/constants.js index d27ac878865..da115429c5d 100644 --- a/packages/mdc-checkbox/constants.js +++ b/packages/mdc-checkbox/constants.js @@ -39,6 +39,8 @@ const strings = { TRANSITION_STATE_CHECKED: 'checked', TRANSITION_STATE_UNCHECKED: 'unchecked', TRANSITION_STATE_INDETERMINATE: 'indeterminate', + ARIA_CHECKED_ATTR: 'aria-checked', + ARIA_CHECKED_INDETERMINATE_VALUE: 'mixed', }; /** @enum {number} */ diff --git a/packages/mdc-checkbox/foundation.js b/packages/mdc-checkbox/foundation.js index 5487116e2c4..256b85907e8 100644 --- a/packages/mdc-checkbox/foundation.js +++ b/packages/mdc-checkbox/foundation.js @@ -49,6 +49,8 @@ class MDCCheckboxFoundation extends MDCFoundation { return /** @type {!MDCCheckboxAdapter} */ ({ addClass: (/* className: string */) => {}, removeClass: (/* className: string */) => {}, + setNativeControlAttr: () => {}, + removeNativeControlAttr: () => {}, registerAnimationEndHandler: (/* handler: EventListener */) => {}, deregisterAnimationEndHandler: (/* handler: EventListener */) => {}, registerChangeHandler: (/* handler: EventListener */) => {}, @@ -108,6 +110,12 @@ class MDCCheckboxFoundation extends MDCFoundation { /** @param {boolean} indeterminate */ setIndeterminate(indeterminate) { this.getNativeControl_().indeterminate = indeterminate; + if (indeterminate) { + this.adapter_.setNativeControlAttr( + strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE); + } else { + this.adapter_.removeNativeControlAttr(strings.ARIA_CHECKED_ATTR); + } } /** @return {boolean} */ @@ -203,6 +211,10 @@ class MDCCheckboxFoundation extends MDCFoundation { return; } + // Remove aria-checked (needed for initial inditerminate state) - screen + // readers will pick the right state from input. + this.adapter_.removeNativeControlAttr(strings.ARIA_CHECKED_ATTR); + // Check to ensure that there isn't a previously existing animation class, in case for example // the user interacted with the checkbox before the animation was finished. if (this.currentAnimationClass_.length > 0) { diff --git a/test/unit/mdc-checkbox/foundation.test.js b/test/unit/mdc-checkbox/foundation.test.js index ed0af5b4eba..281a4493e0c 100644 --- a/test/unit/mdc-checkbox/foundation.test.js +++ b/test/unit/mdc-checkbox/foundation.test.js @@ -110,9 +110,9 @@ test('exports numbers', () => { test('defaultAdapter returns a complete adapter implementation', () => { verifyDefaultAdapter(MDCCheckboxFoundation, [ - 'addClass', 'removeClass', 'registerAnimationEndHandler', 'deregisterAnimationEndHandler', - 'registerChangeHandler', 'deregisterChangeHandler', 'getNativeControl', 'forceLayout', - 'isAttachedToDOM', + 'addClass', 'removeClass', 'setNativeControlAttr', 'removeNativeControlAttr', 'registerAnimationEndHandler', + 'deregisterAnimationEndHandler', 'registerChangeHandler', 'deregisterChangeHandler', 'getNativeControl', + 'forceLayout', 'isAttachedToDOM', ]); }); @@ -207,13 +207,15 @@ test('#isChecked returns false when no native control is returned', () => { }); test('#setIndeterminate updates the value of nativeControl.indeterminate', () => { - const {foundation, nativeControl} = setupTest(); + const {foundation, nativeControl, mockAdapter} = setupTest(); foundation.setIndeterminate(true); assert.isOk(foundation.isIndeterminate()); assert.isOk(nativeControl.indeterminate); + td.verify(mockAdapter.setNativeControlAttr('aria-checked', 'mixed')); foundation.setIndeterminate(false); assert.isNotOk(foundation.isIndeterminate()); assert.isNotOk(nativeControl.indeterminate); + td.verify(mockAdapter.removeNativeControlAttr('aria-checked')); }); test('#setIndeterminate works when no native control is returned', () => { From 7ba7562832b9c76dc1cb49b53949cd67927e9ee5 Mon Sep 17 00:00:00 2001 From: Andrey Prigogin Date: Tue, 13 Mar 2018 10:50:09 -0700 Subject: [PATCH 2/3] fix(checkbox): moved attribute update logic into transitionCheckState_. --- packages/mdc-checkbox/foundation.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/mdc-checkbox/foundation.js b/packages/mdc-checkbox/foundation.js index 256b85907e8..7e58a3a4949 100644 --- a/packages/mdc-checkbox/foundation.js +++ b/packages/mdc-checkbox/foundation.js @@ -110,12 +110,6 @@ class MDCCheckboxFoundation extends MDCFoundation { /** @param {boolean} indeterminate */ setIndeterminate(indeterminate) { this.getNativeControl_().indeterminate = indeterminate; - if (indeterminate) { - this.adapter_.setNativeControlAttr( - strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE); - } else { - this.adapter_.removeNativeControlAttr(strings.ARIA_CHECKED_ATTR); - } } /** @return {boolean} */ @@ -211,9 +205,13 @@ class MDCCheckboxFoundation extends MDCFoundation { return; } - // Remove aria-checked (needed for initial inditerminate state) - screen - // readers will pick the right state from input. - this.adapter_.removeNativeControlAttr(strings.ARIA_CHECKED_ATTR); + // Ensure aria-checked is set to mixed if checkbox is in indeterminate state. + if (this.isIndeterminate()) { + this.adapter_.setNativeControlAttr( + strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE); + } else { + this.adapter_.removeNativeControlAttr(strings.ARIA_CHECKED_ATTR); + } // Check to ensure that there isn't a previously existing animation class, in case for example // the user interacted with the checkbox before the animation was finished. From 5bfba7f9867960aabe80b0990ec234666c46cb19 Mon Sep 17 00:00:00 2001 From: Andrey Prigogin Date: Tue, 13 Mar 2018 10:54:12 -0700 Subject: [PATCH 3/3] fix(checkbox): moved attribute update logic into transitionCheckState_2. --- packages/mdc-checkbox/foundation.js | 2 +- test/unit/mdc-checkbox/foundation.test.js | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/packages/mdc-checkbox/foundation.js b/packages/mdc-checkbox/foundation.js index 7e58a3a4949..f300b3126c1 100644 --- a/packages/mdc-checkbox/foundation.js +++ b/packages/mdc-checkbox/foundation.js @@ -208,7 +208,7 @@ class MDCCheckboxFoundation extends MDCFoundation { // Ensure aria-checked is set to mixed if checkbox is in indeterminate state. if (this.isIndeterminate()) { this.adapter_.setNativeControlAttr( - strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE); + strings.ARIA_CHECKED_ATTR, strings.ARIA_CHECKED_INDETERMINATE_VALUE); } else { this.adapter_.removeNativeControlAttr(strings.ARIA_CHECKED_ATTR); } diff --git a/test/unit/mdc-checkbox/foundation.test.js b/test/unit/mdc-checkbox/foundation.test.js index 281a4493e0c..0629553a6d2 100644 --- a/test/unit/mdc-checkbox/foundation.test.js +++ b/test/unit/mdc-checkbox/foundation.test.js @@ -207,15 +207,13 @@ test('#isChecked returns false when no native control is returned', () => { }); test('#setIndeterminate updates the value of nativeControl.indeterminate', () => { - const {foundation, nativeControl, mockAdapter} = setupTest(); + const {foundation, nativeControl} = setupTest(); foundation.setIndeterminate(true); assert.isOk(foundation.isIndeterminate()); assert.isOk(nativeControl.indeterminate); - td.verify(mockAdapter.setNativeControlAttr('aria-checked', 'mixed')); foundation.setIndeterminate(false); assert.isNotOk(foundation.isIndeterminate()); assert.isNotOk(nativeControl.indeterminate); - td.verify(mockAdapter.removeNativeControlAttr('aria-checked')); }); test('#setIndeterminate works when no native control is returned', () => { @@ -426,6 +424,16 @@ test('change handler triggers layout for changes within the same frame to correc td.verify(mockAdapter.forceLayout()); }); +test('change handler updates aria-checked attribute correctly.', () => { + const {mockAdapter, change} = setupChangeHandlerTest(); + + change({checked: true, indeterminate: true}); + td.verify(mockAdapter.setNativeControlAttr('aria-checked', 'mixed')); + + change({checked: true, indeterminate: false}); + td.verify(mockAdapter.removeNativeControlAttr('aria-checked')); +}); + test('change handler does not add animation classes when isAttachedToDOM() is falsy', () => { const {mockAdapter, change} = setupChangeHandlerTest(); const animClassArg = td.matchers.argThat((cls) => cls.indexOf('mdc-checkbox--anim') >= 0);