Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade axe and address accessibility issues #1276

Merged
merged 5 commits into from
Nov 18, 2021
Merged

Upgrade axe and address accessibility issues #1276

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1d66f08
Upgrade jest-axe
Nov 17, 2021
ccabb73
Add resolution for axe-core to ^4.3.5
Nov 17, 2021
3cbae64
Visually hide the PregressBar in the Carousel component
Nov 17, 2021
35b013e
Add role=progressbar to the time-based ProgressBar
Nov 17, 2021
16fcbe8
Add changesets
Nov 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,8 @@
"storybook-addon-performance": "^0.16.1",
"svgo": "^2.0.3",
"typescript": "^4.4.3"
},
"resolutions": {
"axe-core": "^4.3.5"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ensures that our underlying axe-core version (dependency of jest-axe et. al.) is above 4.3.3.

Why?

Starting in [email protected]), role="presentation" was flagged as invalid on <svg> elements, probably as a side-effect of dequelabs/axe-core#2989 (see our failing CI run).

This bug was reported in dequelabs/axe-core#3082, and solved in dequelabs/axe-core#3124 (patch released in [email protected]).

Resolving to a version above 4.3.3 is one way to address the issue—another would have been to pin axe-core to a version below 4.3.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also note: there was another issue with axe-core affecting the SideNavigation specs, similar to dequelabs/axe-core#2674.

The problem was apparently a bug in an isVisible matcher that would not account for zero width in some cases. In our case, it affected the SideNavigation's mobile specs and ended up timing out (see our failing CI run)

It was fixed in dequelabs/axe-core#3172 and released in [email protected], so resolving to ^4.3.5 is fine for us.

}
}
1 change: 1 addition & 0 deletions packages/circuit-ui/components/Carousel/Carousel.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ const Carousel = ({
<StyledStatus step={state.step} total={slidesTotal} />

<StyledProgressBar
aria-hidden
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only place where we're using the ProgressBar's time-based variant inside Circuit.

It's also the reason I hesitated to add role="progressbar" to the ProgressBar—as I wrote back then:

The only place I can see this time-based variant used is in the carousel, where it's mostly a visual indicator that the next image is about to be shown.

Also: I added the role just to try it out, and ran into another error because the Carousel isn't exposing a label prop for its progress bar (so the aria-labelledby points to an empty span). Here again, I'm not sure what would be the expected label, if any. Something like "In progress" doesn't sound quite right.

On second thought, it simply seems to me like the progress indicator should only be visual here. It's not actually loading, it's only an indicator for when the next carousel slide will come up.

Therefore: aria-hidden.

⚠️ the Carousel component itself has many accessibility issues, e.g. it's not using live regions to announce new content, it's not handling focus properly, etc. See this guide for reference. However, that's a different topic altogether, and should probably be tackled when the Carousel gets redesigned (it's on the backlog).

key={state.step}
size="byte"
variant="secondary"
Expand Down
8 changes: 8 additions & 0 deletions packages/circuit-ui/components/Carousel/__snapshots__/Carousel.spec.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,12 +537,14 @@ exports[`Carousel styles should render with children as a function 1`] = `
3
</strong>
<div
aria-hidden="true"
class="circuit-42 circuit-43"
>
<span
aria-labelledby="progress-bar_6"
class="circuit-44"
loop=""
role="progressbar"
/>
<span
class="circuit-45"
Expand Down Expand Up @@ -1203,12 +1205,14 @@ exports[`Carousel styles should render with children as a node 1`] = `
3
</strong>
<div
aria-hidden="true"
class="circuit-42 circuit-43"
>
<span
aria-labelledby="progress-bar_8"
class="circuit-44"
loop=""
role="progressbar"
/>
<span
class="circuit-45"
Expand Down Expand Up @@ -1868,12 +1872,14 @@ exports[`Carousel styles should render with default paused styles 1`] = `
3
</strong>
<div
aria-hidden="true"
class="circuit-42 circuit-43"
>
<span
aria-labelledby="progress-bar_4"
class="circuit-44"
loop=""
role="progressbar"
/>
<span
class="circuit-45"
Expand Down Expand Up @@ -2504,12 +2510,14 @@ exports[`Carousel styles should render with default styles 1`] = `
3
</strong>
<div
aria-hidden="true"
class="circuit-42 circuit-43"
>
<span
aria-labelledby="progress-bar_2"
class="circuit-44"
loop=""
role="progressbar"
/>
<span
class="circuit-45"
Expand Down
1 change: 1 addition & 0 deletions packages/circuit-ui/components/ProgressBar/ProgressBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,7 @@ export function ProgressBar({
/>
) : (
<TimeProgress
role="progressbar"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ProgressBar's time-based variant (story) started breaking in CI back when dependabot tried to upgrade jest-axe to v5 (see our failing CI run). As I wrote in a comment back then:

So the new jest-axe version is unhappy because the ProgressBar's time-based variant (when there is no given max or value props) doesn't have the proper aria-attributes.

Looking at the difference between it and the step-based variant, I think it might be because it's missing a progressbar role (?).

However, I'm not sure it should have this role. The only place I can see this time-based variant used is in the carousel, where it's mostly a visual indicator that the next image is about to be shown.

To answer my own questions:

I think it might be because it's missing a progressbar role (?).

Not quite: it was failing because aria-labelledby cannot be used on a <span> without a role.

However, I'm not sure it should have this role.

I now think it should. Leaving aside the Carousel case (that's for another comment), and looking at MDN for role="progressbar":

If the actual value of the progressbar can be determined, the developer has to indicate this progress using the aria-valuenow, aria-valuemin, and aria-valuemax attributes1. If the progress value is indeterminate, the developer should omit the aria-valuenow attribute.

I think it's safe to add role="progressbar" here in this case. An alternative would be to remove aria-labelledby, meaning that the label would only be loosely associated with the progress bar.

We will also revisit accessibility for this component as part of the a11y audit fixes we'll apply this month.

Footnotes

  1. That's what we're already doing for the ProgressBar's step-based variant ↩

title={title}
aria-labelledby={ariaId}
duration={duration}
Expand Down
4 changes: 4 additions & 0 deletions packages/circuit-ui/components/ProgressBar/__snapshots__/ProgressBar.spec.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ exports[`ProgressBar time-based should render with a hidden label 1`] = `
<span
aria-labelledby="progress-bar_5"
class="circuit-1"
role="progressbar"
title="A progress bar"
/>
<span
Expand Down Expand Up @@ -322,6 +323,7 @@ exports[`ProgressBar time-based should render with default styles 1`] = `
<span
aria-labelledby="progress-bar_4"
class="circuit-1"
role="progressbar"
/>
<span
class="circuit-2"
Expand Down Expand Up @@ -424,6 +426,7 @@ exports[`ProgressBar time-based should render with loop styles 1`] = `
aria-labelledby="progress-bar_7"
class="circuit-1"
loop=""
role="progressbar"
/>
<span
class="circuit-2"
Expand Down Expand Up @@ -519,6 +522,7 @@ exports[`ProgressBar time-based should render with paused styles 1`] = `
<span
aria-labelledby="progress-bar_6"
class="circuit-1"
role="progressbar"
/>
<span
class="circuit-2"
Expand Down
4 changes: 2 additions & 2 deletions packages/circuit-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@
"@testing-library/react-hooks": "^7.0.0",
"@testing-library/user-event": "^13.1.3",
"@types/cross-spawn": "^6.0.2",
"@types/jest-axe": "^3.5.1",
"@types/jest-axe": "^3.5.3",
"@types/jscodeshift": "^0.11.0",
"@types/lodash": "^4.14.165",
"@types/react": "^17.0.0",
Expand All @@ -79,7 +79,7 @@
"cross-env": "^7.0.2",
"css-loader": "^5.0.1",
"fork-ts-checker-webpack-plugin": "^6.0.3",
"jest-axe": "^4.1.0",
"jest-axe": "^5.0.1",
"react": "^17.0.1",
"react-docgen": "^5.3.1",
"react-docgen-annotation-resolver": "^2.0.0",
Expand Down
98 changes: 42 additions & 56 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4322,14 +4322,22 @@
dependencies:
"@types/istanbul-lib-report" "*"

"@types/jest-axe@^3.0.0", "@types/jest-axe@^3.5.1":
"@types/jest-axe@^3.0.0":
version "3.5.2"
resolved "https://registry.yarnpkg.com/@types/jest-axe/-/jest-axe-3.5.2.tgz#00924237586f49a1a8660ab849d1a6f34dcae1ce"
integrity sha512-5esefQ7hTfZ7i5H2ra8n4oiHeI0Uo7ZUI+71R4qosm+ZLabmlHj6sf20KPcM8bgz9xJV4oUwzA+koE/1lkupQQ==
dependencies:
"@types/jest" "*"
axe-core "^3.5.5"

"@types/jest-axe@^3.5.3":
version "3.5.3"
resolved "https://registry.yarnpkg.com/@types/jest-axe/-/jest-axe-3.5.3.tgz#5af918553388aa0a448af75603b44093985778c6"
integrity sha512-ad9qI9f+00N8IlOuGh6dnZ6o0BDdV9VhGfTUr1zCejsPvOfZd6eohffe4JYxUoUuRYEftyMcaJ6Ux4+MsOpGHg==
dependencies:
"@types/jest" "*"
axe-core "^3.5.5"

"@types/jest@*", "@types/jest@^27.0.1":
version "27.0.1"
resolved "https://registry.yarnpkg.com/@types/jest/-/jest-27.0.1.tgz#fafcc997da0135865311bb1215ba16dba6bdf4ca"
Expand Down Expand Up @@ -5486,20 +5494,10 @@ aws4@^1.8.0:
resolved "https://registry.yarnpkg.com/aws4/-/aws4-1.11.0.tgz#d61f46d83b2519250e2784daf5b09479a8b41c59"
integrity sha512-xh1Rl34h6Fi1DC2WWKfxUTVqRsNnr6LsKz2+hfwDxQJWmrx8+c7ylaqBMcHfl1U1r2dsifOvKX3LQuLNZ+XSvA==

axe-core@^3.5.5:
version "3.5.5"
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-3.5.5.tgz#84315073b53fa3c0c51676c588d59da09a192227"
integrity sha512-5P0QZ6J5xGikH780pghEdbEKijCTrruK9KxtPZCFWUpef0f6GipO+xEZ5GKCb020mmqgbiNO6TcA55CriL784Q==

axe-core@^4.0.1, axe-core@^4.0.2:
version "4.1.4"
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-4.1.4.tgz#f19cd99a84ee32a318b9c5b5bb8ed373ad94f143"
integrity sha512-Pdgfv6iP0gNx9ejRGa3zE7Xgkj/iclXqLfe7BnatdZz0QnLZ3jrRHUVH8wNSdN68w05Sk3ShGTb3ydktMTooig==

axe-core@^4.2.0:
version "4.3.2"
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-4.3.2.tgz#fcf8777b82c62cfc69c7e9f32c0d2226287680e7"
integrity sha512-5LMaDRWm8ZFPAEdzTYmgjjEdj1YnQcpfrVajO/sn/LhbpGp0Y0H64c2hLZI1gRMxfA+w1S71Uc/nHaOXgcCvGg==
[email protected], axe-core@^3.5.5, axe-core@^4.0.2, axe-core@^4.2.0, axe-core@^4.3.5:
version "4.3.5"
resolved "https://registry.yarnpkg.com/axe-core/-/axe-core-4.3.5.tgz#78d6911ba317a8262bfee292aeafcc1e04b49cc5"
integrity sha512-WKTW1+xAzhMS5dJsxWkliixlO/PqC4VhmO9T4juNYcaTg9jzWiJsou6m5pxWYGfigWbwzJWeFY6z47a+4neRXA==

[email protected]:
version "0.21.4"
Expand Down Expand Up @@ -6370,6 +6368,14 @@ [email protected], chalk@^2.0.0, chalk@^2.0.1, chalk@^2.1.0, chalk@^2.3.2, chalk@^2.4.
escape-string-regexp "^1.0.5"
supports-color "^5.3.0"

[email protected]:
version "4.1.0"
resolved "https://registry.yarnpkg.com/chalk/-/chalk-4.1.0.tgz#4e14870a618d9e2edd97dd8345fd9d9dc315646a"
integrity sha512-qwx12AxXe2Q5xQ43Ac//I6v5aXTipYrSESdOgzrN+9XjgEpyjpKuvSGaN4qE93f7TQTlerQQ8S+EQ0EyDoVL1A==
dependencies:
ansi-styles "^4.1.0"
supports-color "^7.1.0"

chalk@^1.0.0, chalk@^1.1.3:
version "1.1.3"
resolved "https://registry.yarnpkg.com/chalk/-/chalk-1.1.3.tgz#a8115c55e4a702fe4d150abd3872822a7e09fc98"
Expand Down Expand Up @@ -7782,11 +7788,6 @@ diff-sequences@^24.9.0:
resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-24.9.0.tgz#5715d6244e2aa65f48bba0bc972db0b0b11e95b5"
integrity sha512-Dj6Wk3tWyTE+Fo1rW8v0Xhwk80um6yFYKbuAxc9c3EZxIHFDYwbi34Uk42u1CdnIiVorvt4RmlSDjIPyzGC2ew==

diff-sequences@^26.6.2:
version "26.6.2"
resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-26.6.2.tgz#48ba99157de1923412eed41db6b6d4aa9ca7c0b1"
integrity sha512-Mv/TDa3nZ9sbc5soK+OoA74BsS3mL37yixCvUAQkiuA4Wz6YtwP/K47n2rv2ovzHZvoiQeA5FTQOschKkEwB0Q==

diff-sequences@^27.0.6:
version "27.0.6"
resolved "https://registry.yarnpkg.com/diff-sequences/-/diff-sequences-27.0.6.tgz#3305cb2e55a033924054695cc66019fd7f8e5723"
Expand Down Expand Up @@ -11237,15 +11238,15 @@ java-properties@^1.0.0:
resolved "https://registry.yarnpkg.com/java-properties/-/java-properties-1.0.2.tgz#ccd1fa73907438a5b5c38982269d0e771fe78211"
integrity sha512-qjdpeo2yKlYTH7nFdK0vbZWuTCesk4o63v5iVOlhMQPfuIZQfW/HI35SjfhA+4qpg36rnFSvUK5b1m+ckIblQQ==

jest-axe@^4.1.0:
version "4.1.0"
resolved "https://registry.yarnpkg.com/jest-axe/-/jest-axe-4.1.0.tgz#5e73069c6517ffa106f31bb6a589a09250e4a04c"
integrity sha512-TPWRbwQYwdt1jtvM0DRi//0e51omWirMkyf99paeY7kYS5XsUZ9IcyP4omrqPKJ46xHDxs1AYqIwVz/minBoFw==
jest-axe@^5.0.1:
version "5.0.1"
resolved "https://registry.yarnpkg.com/jest-axe/-/jest-axe-5.0.1.tgz#26c43643b2e5f2bd4900c1ab36f8283635957a6e"
integrity sha512-MMOWA6gT4pcZGbTLS8ZEqABH08Lnj5bInfLPpn9ADWX2wFF++odbbh8csmSfkwKjHaioVPzlCtIypAtxFDx/rw==
dependencies:
axe-core "^4.0.1"
chalk "^4.0.0"
jest-matcher-utils "^26.0.1"
lodash.merge "^4.6.2"
axe-core "4.2.1"
chalk "4.1.0"
jest-matcher-utils "27.0.2"
lodash.merge "4.6.2"

jest-changed-files@^27.3.0:
version "27.3.0"
Expand Down Expand Up @@ -11336,16 +11337,6 @@ jest-diff@^24.9.0:
jest-get-type "^24.9.0"
pretty-format "^24.9.0"

jest-diff@^26.6.2:
version "26.6.2"
resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-26.6.2.tgz#1aa7468b52c3a68d7d5c5fdcdfcd5e49bd164394"
integrity sha512-6m+9Z3Gv9wN0WFVasqjCL/06+EFCMTqDEUl/b87HYK2rAPTyfz4ZIuSlPhY51PIQRWx5TaxeF1qmXKe9gfN3sA==
dependencies:
chalk "^4.0.0"
diff-sequences "^26.6.2"
jest-get-type "^26.3.0"
pretty-format "^26.6.2"

jest-diff@^27.0.0:
version "27.2.0"
resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-27.2.0.tgz#bda761c360f751bab1e7a2fe2fc2b0a35ce8518c"
Expand All @@ -11356,7 +11347,7 @@ jest-diff@^27.0.0:
jest-get-type "^27.0.6"
pretty-format "^27.2.0"

jest-diff@^27.3.1:
jest-diff@^27.0.2, jest-diff@^27.3.1:
version "27.3.1"
resolved "https://registry.yarnpkg.com/jest-diff/-/jest-diff-27.3.1.tgz#d2775fea15411f5f5aeda2a5e02c2f36440f6d55"
integrity sha512-PCeuAH4AWUo2O5+ksW4pL9v5xJAcIKPUPfIhZBcG1RKv/0+dvaWTQK1Nrau8d67dp65fOqbeMdoil+6PedyEPQ==
Expand Down Expand Up @@ -11428,12 +11419,7 @@ jest-get-type@^24.9.0:
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-24.9.0.tgz#1684a0c8a50f2e4901b6644ae861f579eed2ef0e"
integrity sha512-lUseMzAley4LhIcpSP9Jf+fTrQ4a1yHQwLNeeVa2cEmbCGeoZAtYPOIv8JaxLD/sUpKxetKGP+gsHl8f8TSj8Q==

jest-get-type@^26.3.0:
version "26.3.0"
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-26.3.0.tgz#e97dc3c3f53c2b406ca7afaed4493b1d099199e0"
integrity sha512-TpfaviN1R2pQWkIihlfEanwOXK0zcxrKEE4MlU6Tn7keoXdN6/3gK/xl0yEh8DOunn5pOVGKf8hB4R9gVh04ig==

jest-get-type@^27.0.6, jest-get-type@^27.3.1:
jest-get-type@^27.0.1, jest-get-type@^27.0.6, jest-get-type@^27.3.1:
version "27.3.1"
resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-27.3.1.tgz#a8a2b0a12b50169773099eee60a0e6dd11423eff"
integrity sha512-+Ilqi8hgHSAdhlQ3s12CAVNd8H96ZkQBfYoXmArzZnOfAtVAJEiPDBirjByEblvG/4LPJmkL+nBqPO3A1YJAEg==
Expand Down Expand Up @@ -11511,6 +11497,16 @@ jest-leak-detector@^27.3.1:
jest-get-type "^27.3.1"
pretty-format "^27.3.1"

[email protected]:
version "27.0.2"
resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-27.0.2.tgz#f14c060605a95a466cdc759acc546c6f4cbfc4f0"
integrity sha512-Qczi5xnTNjkhcIB0Yy75Txt+Ez51xdhOxsukN7awzq2auZQGPHcQrJ623PZj0ECDEMOk2soxWx05EXdXGd1CbA==
dependencies:
chalk "^4.0.0"
jest-diff "^27.0.2"
jest-get-type "^27.0.1"
pretty-format "^27.0.2"

jest-matcher-utils@^22.0.0:
version "22.4.3"
resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-22.4.3.tgz#4632fe428ebc73ebc194d3c7b65d37b161f710ff"
Expand All @@ -11530,16 +11526,6 @@ jest-matcher-utils@^24.9.0:
jest-get-type "^24.9.0"
pretty-format "^24.9.0"

jest-matcher-utils@^26.0.1:
version "26.6.2"
resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-26.6.2.tgz#8e6fd6e863c8b2d31ac6472eeb237bc595e53e7a"
integrity sha512-llnc8vQgYcNqDrqRDXWwMr9i7rS5XFiCwvh6DTP7Jqa2mqpcCBBlpCbn+trkG0KNhPu/h8rzyBkriOtBstvWhw==
dependencies:
chalk "^4.0.0"
jest-diff "^26.6.2"
jest-get-type "^26.3.0"
pretty-format "^26.6.2"

jest-matcher-utils@^27.3.1:
version "27.3.1"
resolved "https://registry.yarnpkg.com/jest-matcher-utils/-/jest-matcher-utils-27.3.1.tgz#257ad61e54a6d4044e080d85dbdc4a08811e9c1c"
Expand Down Expand Up @@ -12582,7 +12568,7 @@ lodash.isstring@^4.0.1:
resolved "https://registry.yarnpkg.com/lodash.isstring/-/lodash.isstring-4.0.1.tgz#d527dfb5456eca7cc9bb95d5daeaf88ba54a5451"
integrity sha1-1SfftUVuynzJu5XV2ur4i6VKVFE=

lodash.merge@^4.6.2:
lodash.merge@4.6.2, lodash.merge@^4.6.2:
version "4.6.2"
resolved "https://registry.yarnpkg.com/lodash.merge/-/lodash.merge-4.6.2.tgz#558aa53b43b661e1925a0afdfa36a9a1085fe57a"
integrity sha512-0KpjqXRVvrYyCsX1swR/XTK0va6VQkQM6MNo7PqW77ByjAhoARA8EfrP1N4+KlKj8YS0ZUCtRT/YUuhyYDujIQ==
Expand Down