From 7366906e43bed82350b57cd1333d9e61996d7a7e Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 28 Nov 2019 11:40:21 +0100 Subject: [PATCH 1/4] [test] Assert accessible name --- package.json | 1 + .../src/Autocomplete/Autocomplete.test.js | 8 +-- packages/material-ui/src/Chip/Chip.test.js | 3 +- .../material-ui/src/Select/Select.test.js | 1 + test/utils/createDOM.js | 9 ++- test/utils/init.d.ts | 7 ++ test/utils/initMatchers.js | 68 +++++++++++++++++++ yarn.lock | 5 ++ 8 files changed, 95 insertions(+), 7 deletions(-) diff --git a/package.json b/package.json index 0b2584b8298c1e..56b2e641ad59c9 100644 --- a/package.json +++ b/package.json @@ -83,6 +83,7 @@ "confusing-browser-globals": "^1.0.9", "cross-env": "^6.0.0", "danger": "^9.1.8", + "dom-accessibility-api": "^0.2.0", "dtslint": "^2.0.0", "enzyme": "^3.9.0", "enzyme-adapter-react-16": "^1.14.0", diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js index fee476af934dd8..84c6863e25fe01 100644 --- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js +++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js @@ -120,9 +120,9 @@ describe('', () => { const buttons = getAllByRole('button'); expect(buttons).to.have.length(2); - // TODO: computeAccessibleName + expect(buttons[0]).to.have.accessibleName('Clear'); expect(buttons[0]).to.have.attribute('title', 'Clear'); - // TODO: computeAccessibleName + expect(buttons[1]).to.have.accessibleName('Open'); expect(buttons[1]).to.have.attribute('title', 'Open'); buttons.forEach(button => { expect(button, 'button is not in tab order').to.have.property('tabIndex', -1); @@ -161,9 +161,9 @@ describe('', () => { const buttons = getAllByRole('button'); expect(buttons).to.have.length(2); - // TODO: computeAccessibleName + expect(buttons[0]).to.have.accessibleName('Clear'); expect(buttons[0]).to.have.attribute('title', 'Clear'); - // TODO: computeAccessibleName + expect(buttons[1]).to.have.accessibleName('Close'); expect(buttons[1]).to.have.attribute('title', 'Close'); buttons.forEach(button => { expect(button, 'button is not in tab order').to.have.property('tabIndex', -1); diff --git a/packages/material-ui/src/Chip/Chip.test.js b/packages/material-ui/src/Chip/Chip.test.js index 98600b09691a23..669213487dba1e 100644 --- a/packages/material-ui/src/Chip/Chip.test.js +++ b/packages/material-ui/src/Chip/Chip.test.js @@ -75,8 +75,7 @@ describe('', () => { const button = getByRole('button'); expect(button).to.have.property('tabIndex', 0); - // TODO: accessible name computation - expect(button).to.have.text('My Chip'); + expect(button).to.have.accessibleName('My Chip'); }); it('should apply user value of tabIndex', () => { diff --git a/packages/material-ui/src/Select/Select.test.js b/packages/material-ui/src/Select/Select.test.js index a898521cb92a80..ad79a6137de0d5 100644 --- a/packages/material-ui/src/Select/Select.test.js +++ b/packages/material-ui/src/Select/Select.test.js @@ -406,6 +406,7 @@ describe('); + // TODO what is the accessible name actually? expect(getByRole('button')).to.have.attribute('aria-labelledby', ' '); }); diff --git a/test/utils/createDOM.js b/test/utils/createDOM.js index 4c7710bc93ab92..3b9a8cedf71aa7 100644 --- a/test/utils/createDOM.js +++ b/test/utils/createDOM.js @@ -2,7 +2,14 @@ const { JSDOM } = require('jsdom'); const Node = require('jsdom/lib/jsdom/living/node-document-position'); // We can use jsdom-global at some point if maintaining these lists is a burden. -const whitelist = ['Element', 'HTMLElement', 'HTMLInputElement', 'Performance']; +const whitelist = [ + // required for fake getComputedStyle + 'CSSStyleDeclaration', + 'Element', + 'HTMLElement', + 'HTMLInputElement', + 'Performance', +]; const blacklist = ['sessionStorage', 'localStorage']; function createDOM() { diff --git a/test/utils/init.d.ts b/test/utils/init.d.ts index 9db9ea9827ef5f..a83ccd15434d3e 100644 --- a/test/utils/init.d.ts +++ b/test/utils/init.d.ts @@ -2,6 +2,13 @@ declare namespace Chai { interface Assertion { + /** + * checks if the accessible name computation (according to `accname` spec) + * matches the expectation. + * @see https://www.w3.org/TR/accname-1.2/ + * @param name + */ + accessibleName(name: string): Assertion; /** * checks if the element in question is considered aria-hidden * Does not replace accessibility check as that requires display/visibility/layout diff --git a/test/utils/initMatchers.js b/test/utils/initMatchers.js index 1ac2f5e8805a67..b83d55411732f9 100644 --- a/test/utils/initMatchers.js +++ b/test/utils/initMatchers.js @@ -2,6 +2,7 @@ import chai from 'chai'; import chaiDom from 'chai-dom'; import { isInaccessible } from '@testing-library/dom'; import { prettyDOM } from '@testing-library/react/pure'; +import { computeAccessibleName } from 'dom-accessibility-api'; chai.use(chaiDom); chai.use((chaiAPI, utils) => { @@ -62,4 +63,71 @@ chai.use((chaiAPI, utils) => { `expected ${utils.elToString(element)} to be accessible but it was inaccessible`, ); }); + + chai.Assertion.addMethod('accessibleName', function hasAccessibleName(expectedName) { + const root = utils.flag(this, 'object'); + // make sure it's an Element + new chai.Assertion(root.nodeType, `Expected an Element but got '${String(root)}'`).to.equal(1); + + const blockElements = new Set( + 'html', + 'address', + 'blockquote', + 'body', + 'dd', + 'div', + 'dl', + 'dt', + 'fieldset', + 'form', + 'frame', + 'frameset', + 'h1', + 'h2', + 'h3', + 'h4', + 'h5', + 'h6', + 'noframes', + 'ol', + 'p', + 'ul', + 'center', + 'dir', + 'hr', + 'menu', + 'pre', + ); + /** + * + * @param {Element} element + * @returns {CSSStyleDeclaration} + */ + function pretendVisibleGetComputedStyle(element) { + const declaration = new CSSStyleDeclaration(); + + // initial values + declaration.content = ''; + // technically it's `inline`. We partially apply the default user agent sheet (chrome) here + // we're only interested in elements that use block + declaration.display = blockElements.has(element.tagName) ? 'block' : 'inline'; + declaration.visibility = 'visible'; + + return declaration; + } + + const actualName = computeAccessibleName(root, { + // in local development we pretend to be visible. full getComputedStyle is + // expensive and reserved for CI + getComputedStyle: process.env.CI ? undefined : pretendVisibleGetComputedStyle, + }); + + this.assert( + actualName === expectedName, + `expected ${utils.elToString( + root, + )} to have accessible name '${expectedName}' but got '${actualName}' instead.`, + `expected ${utils.elToString(root)} not to have accessible name '${expectedName}'.`, + ); + }); }); diff --git a/yarn.lock b/yarn.lock index fd2a3e0b4b6f47..4953b33eff1e19 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5751,6 +5751,11 @@ doctrine@^3.0.0: dependencies: esutils "^2.0.2" +dom-accessibility-api@^0.2.0: + version "0.2.0" + resolved "https://registry.yarnpkg.com/dom-accessibility-api/-/dom-accessibility-api-0.2.0.tgz#2890ce677bd7b2172778ed979ab2ff4967c3085d" + integrity sha512-afrHGxXpS5C2jUC5hquPb3GWytNKHI+wJLKr/jvri95sZpLYpEJi3CtI/yBPEJ+/R9/CXaWXifadz94tsDcotg== + dom-helpers@^3.2.1, dom-helpers@^3.4.0: version "3.4.0" resolved "https://registry.yarnpkg.com/dom-helpers/-/dom-helpers-3.4.0.tgz#e9b369700f959f62ecde5a6babde4bccd9169af8" From 4a5a2fbe7f0adc020a4fbdf235f27058e9911b85 Mon Sep 17 00:00:00 2001 From: Sebastian Silbermann Date: Thu, 28 Nov 2019 11:54:38 +0100 Subject: [PATCH 2/4] Add accname test for Select --- .../test/integration/Select.test.js | 26 ++++++++++++++----- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/packages/material-ui/test/integration/Select.test.js b/packages/material-ui/test/integration/Select.test.js index c356ac6a3854a8..b8d4e52b8b5f3c 100644 --- a/packages/material-ui/test/integration/Select.test.js +++ b/packages/material-ui/test/integration/Select.test.js @@ -75,9 +75,7 @@ describe(' integration', () => { }); describe('with label', () => { + it('requires `id` and `labelId` for a proper accessible name', () => { + const { getByRole } = render( + + Age + + , + ); + + expect(getByRole('button')).to.have.accessibleName('Age Ten'); + }); + // we're somewhat abusing "focus" here. What we're actually interested in is // displaying it as "active". WAI-ARIA authoring practices do not consider the // the trigger part of the widget while a native + @@ -117,7 +129,7 @@ describe('