fix(Tag): adds various fixes for screen reader accessibility; adds AAT #4863

dakahn · 2019-12-11T22:37:42Z

Closes #4427

Changelog

New

added button reset to styling
added AAT tests for DAP and Axe
added AAT tests for aria-label

Changed

changed <span role="button"> to simply <button>
title prop is now used in aria-label
updated Storybook story to be more clear

Removed

removed aria-label on close icon
removed title attribute on filtered Tag

Testing / Reviewing

Tester should verify:
A) component speaks properly in JAWS latest on Windows 10 in Chrome, Edge, and Firefox latest
B) component speaks properly in VoiceOver on Safari in macOS and iOS
C) #4427 is satisfied

…filtertags

netlify · 2019-12-11T22:47:13Z

Deploy preview for the-carbon-components ready!

Built with commit 846bd21

https://deploy-preview-4863--the-carbon-components.netlify.com

packages/react/src/components/Tag/Tag-test.js

joshblack · 2019-12-11T22:47:45Z

packages/react/src/components/Tag/Tag.js

  ) : (
-    <span className={tagClasses} {...other}>
+    <button className={tagClasses} {...other}>


Do the other tag types need to be a button?

Nah, I don't think so. They're non-interactable generally.

joshblack · 2019-12-11T22:49:32Z

packages/react/src/components/Tag/Tag.js

+      aria-label={
+        title !== undefined
+          ? `${title} ${children}`
+          : `Clear filter ${children}`


Is this so that we don't have a mismatch with the visual and accessible labels? Would there a preferred way to separate these or is this the ideal? Would we ever want something like:

<span class="tag"> <button>Clear</button> Text for tag </span>

Well, since we don't technically have a visual label for the X icon remove button this was just so it spoke clearly (as defined in the issue) in VO etc

title is -- not what we want here, but removing or not incorporating the prop is a breaking change.

packages/react/src/components/Tag/Tag-test.js

netlify · 2019-12-11T22:57:36Z

Deploy preview for carbon-elements ready!

Built with commit 4f8795a

https://deploy-preview-4863--carbon-elements.netlify.com

netlify · 2019-12-11T22:57:38Z

Deploy preview for the-carbon-components ready!

Built with commit 4f8795a

https://deploy-preview-4863--the-carbon-components.netlify.com

netlify · 2019-12-11T22:59:00Z

Deploy preview for carbon-components-react ready!

Built with commit 4f8795a

https://deploy-preview-4863--carbon-components-react.netlify.com

Co-Authored-By: Josh Black <[email protected]>

packages/react/src/components/Tag/Tag-test.js

Co-Authored-By: Josh Black <[email protected]>

packages/react/src/components/Tag/Tag-test.js

abbeyhrt

I was only able to test on Safari and Chrome with VO since I don't have access to a Windows VM but it was good in those browsers!

Co-Authored-By: Josh Black <[email protected]>

joshblack · 2019-12-18T16:36:22Z

@dakahn looks like it hit the following test failure:

dakahn · 2019-12-18T20:10:23Z

Yeah, looks like if the tested element contains text it needs to be wrapped in a <main> tag moving forward 👍

asudoh · 2019-12-18T23:17:05Z

@dakahn Is this PR ready for re-review? Thanks!

carbon-design-system#4863) * fix(filter): add role button and update story for clarity * fix(tag): change span to button, remove role and add button-reset helper * fix(tag): add button-reset to tag styling * test(tag): add Axe test * fix(tag): remove unused aria, title, tab-index; add DAP * test(tag): add busted unit tests for screenreader HALP * test(tag): add aria-label test * Update packages/react/src/components/Tag/Tag-test.js Co-Authored-By: Josh Black <[email protected]> * Update packages/react/src/components/Tag/Tag-test.js Co-Authored-By: Josh Black <[email protected]> * Update packages/react/src/components/Tag/Tag-test.js Co-Authored-By: Josh Black <[email protected]> * Update packages/react/src/components/Tag/Tag-test.js Co-Authored-By: Josh Black <[email protected]> * test(tags): add less specific check for children for aria-label * test(tag): add <main> wrapper for DAP test element Co-authored-by: Josh Black <[email protected]>

dakahn added 9 commits December 5, 2019 11:36

fix(filter): add role button and update story for clarity

3bdb91a

Merge branch '4427-filtertags' of github.com:dakahn/carbon into 4427-…

0304691

…filtertags

fix(tag): change span to button, remove role and add button-reset helper

49a2e0d

fix(tag): add button-reset to tag styling

5ad6d50

chore(tag): sync with upstream master

cd5e8a0

test(tag): add Axe test

180be7f

fix(tag): remove unused aria, title, tab-index; add DAP

ba059f0

test(tag): add busted unit tests for screenreader HALP

1c7d3b0

test(tag): add aria-label test

846bd21

dakahn requested a review from a team as a code owner December 11, 2019 22:37

ghost requested review from asudoh and joshblack December 11, 2019 22:37

dakahn requested a review from abbeyhrt December 11, 2019 22:38

Merge branch 'master' into 4427-filtertags

aa9728f

joshblack suggested changes Dec 11, 2019

View reviewed changes

dakahn and others added 2 commits December 12, 2019 00:08

Update packages/react/src/components/Tag/Tag-test.js

fc275f6

Co-Authored-By: Josh Black <[email protected]>

Update packages/react/src/components/Tag/Tag-test.js

6beb042

Co-Authored-By: Josh Black <[email protected]>

joshblack reviewed Dec 12, 2019

View reviewed changes

packages/react/src/components/Tag/Tag-test.js Outdated Show resolved Hide resolved

Update packages/react/src/components/Tag/Tag-test.js

3f41457

Co-Authored-By: Josh Black <[email protected]>

joshblack suggested changes Dec 12, 2019

View reviewed changes

packages/react/src/components/Tag/Tag-test.js Show resolved Hide resolved

packages/react/src/components/Tag/Tag-test.js Outdated Show resolved Hide resolved

abbeyhrt approved these changes Dec 13, 2019

View reviewed changes

dakahn and others added 3 commits December 17, 2019 13:13

Update packages/react/src/components/Tag/Tag-test.js

eaebfd0

Co-Authored-By: Josh Black <[email protected]>

test(tags): add less specific check for children for aria-label

cf69e2f

test(tag): resolve merge conflict

e965060

dakahn requested a review from joshblack December 17, 2019 19:51

joshblack approved these changes Dec 17, 2019

View reviewed changes

asudoh added the status: failing CI 🙅‍♂️ label Dec 17, 2019

Merge branch 'master' into 4427-filtertags

8a8dff1

dakahn added 2 commits December 18, 2019 11:05

Merge branch 'master' into 4427-filtertags

177b1c0

test(tag): add <main> wrapper for DAP test element

582848e

dakahn added status: ready to merge 🎉 and removed status: failing CI 🙅‍♂️ status: ready to merge 🎉 labels Dec 18, 2019

Merge branch 'master' into 4427-filtertags

5e25686

joshblack added the status: ready to merge 🎉 label Jan 6, 2020

Merge branch 'master' into 4427-filtertags

4f8795a

joshblack merged commit 0d5c573 into carbon-design-system:master Jan 6, 2020

joshblack mentioned this pull request Jan 6, 2020

chore(project): v10.9.1 release #4957

Closed

emyarod mentioned this pull request Jan 7, 2020

fix(Tag): ignore click events when disabled #4792

Merged

joshblack mentioned this pull request Jan 31, 2020

chore(project): update carbon to v10.10.0-rc.0 #5236

Closed

joshblack mentioned this pull request Feb 25, 2020

chore(release): v10.10.0 #5432

Closed

emyarod mentioned this pull request Mar 5, 2020

feat(Tag): support click handler for filter tag close icon #5470

Merged

emyarod mentioned this pull request May 21, 2020

fix(tag): add title to tags #6112

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(Tag): adds various fixes for screen reader accessibility; adds AAT #4863

fix(Tag): adds various fixes for screen reader accessibility; adds AAT #4863

dakahn commented Dec 11, 2019 •

edited

Loading

netlify bot commented Dec 11, 2019

joshblack Dec 11, 2019

dakahn Dec 12, 2019

joshblack Dec 11, 2019

dakahn Dec 12, 2019

dakahn Dec 12, 2019

netlify bot commented Dec 11, 2019 •

edited

Loading

netlify bot commented Dec 11, 2019 •

edited

Loading

netlify bot commented Dec 11, 2019 •

edited

Loading

abbeyhrt left a comment

joshblack commented Dec 18, 2019

dakahn commented Dec 18, 2019

asudoh commented Dec 18, 2019

fix(Tag): adds various fixes for screen reader accessibility; adds AAT #4863

fix(Tag): adds various fixes for screen reader accessibility; adds AAT #4863

Conversation

dakahn commented Dec 11, 2019 • edited Loading

Changelog

Testing / Reviewing

netlify bot commented Dec 11, 2019

joshblack Dec 11, 2019

Choose a reason for hiding this comment

dakahn Dec 12, 2019

Choose a reason for hiding this comment

joshblack Dec 11, 2019

Choose a reason for hiding this comment

dakahn Dec 12, 2019

Choose a reason for hiding this comment

dakahn Dec 12, 2019

Choose a reason for hiding this comment

netlify bot commented Dec 11, 2019 • edited Loading

netlify bot commented Dec 11, 2019 • edited Loading

netlify bot commented Dec 11, 2019 • edited Loading

abbeyhrt left a comment

Choose a reason for hiding this comment

joshblack commented Dec 18, 2019

dakahn commented Dec 18, 2019

asudoh commented Dec 18, 2019

dakahn commented Dec 11, 2019 •

edited

Loading

netlify bot commented Dec 11, 2019 •

edited

Loading

netlify bot commented Dec 11, 2019 •

edited

Loading

netlify bot commented Dec 11, 2019 •

edited

Loading