[BUG-7463] Response example tabs a11y #7464

3ptmonster · 2021-08-16T19:46:37Z

Update tabs to use <button> elements instead of <a>
Add aria roles for tablist, tabs, and tabpanel to assist with screen-reader a11y
Replace ids with data-name attribute for tabpanels
Create test/e2e-cypress/tests/a11y directory and add cypress test 7463
Fix broken test swos-63

Description

The tabs implementation for Example Value and Schema/Model is not keyboard navigable.

Motivation and Context

This change is required because the implementation as it stands is not keyboard navigable, and is therefore broken for keyboard-only users.

Fixes #7463 well as a high-priority item on this larger a11y open issue list.

How Has This Been Tested?

Environment
OS: macOS 11.5 Big Sur
Browser(s):

Chrome v92.0.4515.131
Safari v14.1.2
Node version: 14.15.0
npm version: 6.14.8

I forked the master branch and ran the petstore in my local environment to test the changes. The testing consisted of keyboard-only navigation through the petstore with the macOS VoiceOver utility in both Chrome and Safari. I pressed the Tab key until I was focused on the button labeled Example Value, then pressed Tab a second time to ensure the focus changed to the button labeled Model. Once those were satisfied, the third Tab press was done to test that focus changed to the tabpanel.

Screenshots (if appropriate):

Checklist

My PR contains...

No code changes (src/ is unmodified: changes to documentation, CI, metadata, etc.)
Dependency changes (any modification to dependencies in package.json)
Bug fixes (non-breaking change which fixes an issue)
Improvements (misc. changes to existing features)
Features (non-breaking change which adds functionality)

My changes...

are breaking changes to a public API (config options, System API, major UI change, etc).
are breaking changes to a private API (Redux, component props, utility functions, etc.).
are breaking changes to a developer API (npm script behavior changes, new dev system dependencies, etc).
are not breaking changes.

Documentation

My changes do not require a change to the project documentation.
My changes require a change to the project documentation.
If yes to above: I have updated the documentation accordingly.

Automated tests

My changes can not or do not need to be tested.
My changes can and should be tested by unit and/or integration tests.
If yes to above: I have added tests to cover my changes.
If yes to above: I have taken care to cover edge cases in my tests.
All new and existing tests passed.

Update tabs to use <button> elements instead of <a> Add aria roles for tablist, tabs, and tabpanel Add aria attributes for additional a11y compliance and screen reader accessibility

- Remove aria-controls and aria-labelledby attributes along with the removal of ids - Replace ids with data-name attribute for tabpanels - Add cypress test 7463 and update swos-63

char0n · 2021-08-17T09:05:01Z

Thanks @3ptmonster;

We'll get into review ASAP. I was wandering - how it is that you're familiar with swos-63 ?

3ptmonster · 2021-08-17T12:53:28Z

@char0n I was not familiar with it at first, but before creating the PR I ran all the tests and it was the only one to fail, so I looked into it before making the appropriate updates.

char0n

Great work!

Please have a look at my review comments and let me know if they make sense.

This PR changes package-lock.json file; there's not reason why it should. Can you please remove changes to packe-lock.json from this PR? Thanks!

src/core/components/model-example.jsx

test/e2e-cypress/tests/bugs/7463.js

Cgfor3 · 2021-08-19T13:32:46Z

This PR changes package-lock.json file; there's not reason why it should. Can you please remove changes to packe-lock.json from this PR? Thanks!

Restore package-lock file from master branch

- Move tabs test file to tests/a11y directory

test/e2e-cypress/tests/a11y/7463.js

Remove unnecessary issue # from describe statement

…s and aria-labelledby attributes

char0n · 2021-09-09T15:03:04Z

@Cgfor3 thanks for the additional effort and work!

I'll re-review during tomorrow.

char0n

The functionality and test looks good! Thanks for your work on this. Before we move to merging I'll like to simplify this code if we can.

Consider my further comments and nitpicks

src/core/components/model-example.jsx

…entiate between "schema" and "model" Updated affected tests accordingly

char0n · 2021-09-16T13:40:33Z

@3ptmonster this has been a great experience collaborating with! Hope you'll keep those a11n PR comming ;]

Waiting for tests to pass and going for a merge

src/core/components/model-example.jsx

3ptmonster · 2021-09-16T13:45:19Z

@3ptmonster this has been a great experience collaborating with! Hope you'll keep those a11n PR comming ;]

Waiting for tests to pass and going for a merge

I appreciate all the help and feedback! Definitely looking forward to collaborating again.

char0n · 2021-09-20T16:02:51Z

@3ptmonster we have a visual regression after this PR described here: #7517

Cgfor3 · 2021-09-20T16:24:57Z

@3ptmonster we have a visual regression after this PR described here: #7517

Are there documented design requirements for that call-to-action element? What would be the best way for me to assist with this regression?

char0n · 2021-09-21T10:19:51Z

Are there documented design requirements for that call-to-action element?

After the changes we made, the ideal outcome would be if the UI looked the same. I did tested it locally, but unfortunately I forgot to open the changes in Incognito Window; so the cache kicked in.

What would be the best way for me to assist with this regression?

It's being handled in #7517 quite well. This was just FYI.

…nks (#7517) Fixes small visual regression introduced in #7464. Refs #7464

Cgfor3 added 2 commits August 16, 2021 11:02

[BUG-7463]: Response tabs not keyboard navigable

a54c40c

Update tabs to use <button> elements instead of <a> Add aria roles for tablist, tabs, and tabpanel Add aria attributes for additional a11y compliance and screen reader accessibility

- Remove ids from tabs and tabpanels to prevent id duplication

9add398

- Remove aria-controls and aria-labelledby attributes along with the removal of ids - Replace ids with data-name attribute for tabpanels - Add cypress test 7463 and update swos-63

char0n self-assigned this Aug 17, 2021

char0n added cat: a11y type: enhancement labels Aug 17, 2021

char0n requested changes Aug 19, 2021

View reviewed changes

- Restore package-lock.json

59a9e1f

- Move tabs test file to tests/a11y directory

char0n reviewed Aug 20, 2021

View reviewed changes

test/e2e-cypress/tests/a11y/7463.js Outdated Show resolved Hide resolved

char0n reviewed Aug 20, 2021

View reviewed changes

test/e2e-cypress/tests/a11y/7463.js Outdated Show resolved Hide resolved

Cgfor3 added 2 commits August 20, 2021 10:38

Rename test file to be more descriptive of what is being tested.

deb2243

Remove unnecessary issue # from describe statement

Add id attributes to both tabs and tabpanels to leverage aria-control…

bee67e0

…s and aria-labelledby attributes

3ptmonster requested a review from char0n August 30, 2021 20:02

char0n approved these changes Sep 15, 2021

View reviewed changes

char0n reviewed Sep 15, 2021

View reviewed changes

src/core/components/model-example.jsx Outdated Show resolved Hide resolved

char0n reviewed Sep 15, 2021

View reviewed changes

src/core/components/model-example.jsx Outdated Show resolved Hide resolved

char0n reviewed Sep 15, 2021

View reviewed changes

src/core/components/model-example.jsx Outdated Show resolved Hide resolved

3ptmonster and others added 4 commits September 15, 2021 08:41

Merge branch 'master' into bug/7463-keyboard-nav-response-tabs

40d89bc

Nit changes around the use of cx and removal of unnecessary quotes

8d7b868

Updated data-name attribute to use "model" and remove logic to differ…

4766dfc

…entiate between "schema" and "model" Updated affected tests accordingly

Merge branch 'master' into bug/7463-keyboard-nav-response-tabs

21bae3b

char0n reviewed Sep 16, 2021

View reviewed changes

src/core/components/model-example.jsx Outdated Show resolved Hide resolved

Update src/core/components/model-example.jsx

f67bc65

char0n added 2 commits September 16, 2021 16:56

Merge branch 'master' into bug/7463-keyboard-nav-response-tabs

b866ec3

Merge branch 'master' into bug/7463-keyboard-nav-response-tabs

dfe5ba5

char0n merged commit 8ffb1ae into swagger-api:master Sep 17, 2021

dhenneke mentioned this pull request Sep 20, 2021

fix(style): update the buttons of the example tabs to be displayed as links #7517

Merged

17 tasks

char0n pushed a commit that referenced this pull request Sep 21, 2021

fix(ux): update the buttons of the example tabs to be displayed as li…

ff5deb3

…nks (#7517) Fixes small visual regression introduced in #7464. Refs #7464

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG-7463] Response example tabs a11y #7464

[BUG-7463] Response example tabs a11y #7464

3ptmonster commented Aug 16, 2021 •

edited

char0n commented Aug 17, 2021

3ptmonster commented Aug 17, 2021

char0n left a comment

Cgfor3 commented Aug 19, 2021 •

edited

char0n commented Sep 9, 2021

char0n left a comment

char0n commented Sep 16, 2021

3ptmonster commented Sep 16, 2021

char0n commented Sep 20, 2021

Cgfor3 commented Sep 20, 2021

char0n commented Sep 21, 2021

[BUG-7463] Response example tabs a11y #7464

[BUG-7463] Response example tabs a11y #7464

Conversation

3ptmonster commented Aug 16, 2021 • edited

Description

Motivation and Context

How Has This Been Tested?

Screenshots (if appropriate):

Checklist

My PR contains...

My changes...

Documentation

Automated tests

char0n commented Aug 17, 2021

3ptmonster commented Aug 17, 2021

char0n left a comment

Choose a reason for hiding this comment

Cgfor3 commented Aug 19, 2021 • edited

char0n commented Sep 9, 2021

char0n left a comment

Choose a reason for hiding this comment

char0n commented Sep 16, 2021

3ptmonster commented Sep 16, 2021

char0n commented Sep 20, 2021

Cgfor3 commented Sep 20, 2021

char0n commented Sep 21, 2021

3ptmonster commented Aug 16, 2021 •

edited

Cgfor3 commented Aug 19, 2021 •

edited