-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
MCR-3080: bring back a11y tests in ci #2715
Changes from 24 commits
10fe077
36ceb71
ca34107
41333e9
1014ee0
eebe3cc
3f9a1e2
64f664f
2167a6c
0bc8e79
ce879d7
7861efd
cfae62b
e2e2df3
585d269
f6ff9a9
58fdd78
14c2fe6
edd8960
833c0ae
e60db8f
144ae5f
3402246
09dea90
75a1d3a
6fe5157
ea421f5
eb4b34d
321d164
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
# Define Frontend Toolset for Accessibility Standards | ||
|
||
Decide on a toolset to help ensure MCRRS frontend development follows accessibility standards (specifically WCAG AA guidelines). | ||
Decide on a toolset to help ensure MCRRS frontend development follows accessibility standards (specifically WCAG 2.2 AA guidelines). | ||
|
||
Tooling is not a replacement for engineers/designers with strong expertise in HTML or manual testing with assistive technology. Thus, the toolset will focus on the types of accessibility issues where tooling is useful. This includes validating HTML attributes, checking basic page structure, ensuring media content has descriptive tags, ensuring that buttons are used appropriately, and checking for basic color contrast. | ||
|
||
|
@@ -19,7 +19,7 @@ Together these tools include linting, testing, and auditing loaded web pages for | |
|
||
[eslint jsx-a11y][eslint-a11y] Provides React-friendly code linting which allows developers to find accessibility issues during local development. | ||
|
||
[pa11y][pa11y] and [pa11y-ci][pa11y-ci] - Accessibility test runner for end to end testing in the command line or Node.js. End to end testing can catch concerns that are not noticeable in linting or unit tests since tests run on browser. | ||
[cypress-axe][cypress-axe] and [axe-core][axe-copre] - Accessibility test runner for end to end testing in the command line or Node.js. End to end testing can catch concerns that are not noticeable in linting or unit tests since tests run on browser. Our previous choice [pa11y][pa11y] seems to not be maintained anymore and no longer works with our end to end testing. | ||
|
||
[jest-axe][jest-axe] - This tool provides additional jest matchers, like `.toHaveNoViolations`, to use in unit tests. This may be redundant when end to end accessibility testing is present. | ||
|
||
|
@@ -43,12 +43,27 @@ Together these tools include linting, testing, and auditing loaded web pages for | |
- `+` Allows the team to get a Lighthouse report alongside every PR. | ||
- `-` Seemed too robust for this stage of the project (pilot only), provides a lot of information about performance that is not relevant to us now. May be useful later on. | ||
|
||
#### [cypress-axe][cypress-axe] and [axe-core][axe-core] | ||
- `+` `axe-core` is a well known and maintained a11y testing engine | ||
- `+` `axe-core` has great documentation | ||
- `+` `cypress-axe` can run tests with specific a11y standards like `WCAG 2.2 AA`. | ||
- `+` `cypress-axe` is a Cypress plugin for axe-core and most of the configuration for this plugin follows `axe-core` which has great documentation. | ||
- `-` `cypress-axe` itself has sparse documentation | ||
- `-` `cypress-axe` is very minimal in features and if we wanted specific things like reporting, we would have to implement that ourselves. | ||
|
||
#### [Cypress Accessibility][cypress-accessibility] | ||
- `+` Built in accessibility testing by Cypress. | ||
- `-` Still in early access and not much is known about implementation or documentation. | ||
- `-` As of now it looks like it would cost money in addition to what we already pay for Cypress. | ||
- `-` To get access we have to sign up for early access as a trial. | ||
|
||
#### [pa11y][pa11y] and [pa11y-ci][pa11y-ci] | ||
|
||
- `+` pa11y-ci has clear patterns for use in ci | ||
- `+` Unlike other options explored, includes ‘actions’ to be used within a test to interact with the page under test. | ||
- `+` Has significant documentation. | ||
- `-` Could slow development processes if its set too strict. However, many examples of how to optimize configuration as needed in documentation and tutorial | ||
- `-` No longer works with our end to end testing tool Cypress | ||
|
||
#### [eslint-jsx-a11y][eslint-a11y] & [addon-a11y][storybook-addon-a11y] | ||
|
||
|
@@ -63,3 +78,6 @@ Together these tools include linting, testing, and auditing loaded web pages for | |
[axe-core-cli]: https://www.npmjs.com/package/@axe-core/cli | ||
[cypress-audit]: https://github.com/mfrachet/cypress-audit | ||
[lighthouse-ci]: https://github.com/GoogleChrome/lighthouse-ci | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ADRs aren't supposed to be edited in hindsight. They are meant to be a point in time record. So what you do here - this would be a new ADR to update frontend a11y toolset to use |
||
[cypress-axe]: https://github.com/component-driven/cypress-axe | ||
[axe-core]: https://github.com/dequelabs/axe-core | ||
[cypress-accessibility]: https://www.cypress.io/blog/introducing-cypress-accessibility |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -71,6 +71,8 @@ | |
"devDependencies": { | ||
"c8": "^10.1.2", | ||
"cypress": "^13.13.2", | ||
"axe-core": "^4.10.0", | ||
"cypress-axe": "^1.5.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mojotalantikite I added these two dependencies here too. Correct me if things changed but, I did this because Cypress in CI runs from root and on local is runs from I could do some configuration in each test to point to the location of the install based on deployment, but if installing the deps in both doesn't cost or complicate much would this be ok? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mojotalantikite @JasonLin0991 imo this is also a good time to consider if cypress should be moved considering the new world we are. Throwing it out there, not trying to suggest moving the entire folder is needed ...just want us to think expansively since we are also doing a lot around modularizing and simplifying our top level architecture right now so maybe we are seeing another opportunity here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is totally fine. I personally think it's clearer to declare the dependencies in each service's Plus with pnpm everything is symlinked around, so it's not actually going to install it in two places, it's just going to throw in a symlink from the global package store to each service's |
||
"danger": "^11.2.6", | ||
"husky": "^9.1.5", | ||
"lint-staged": "^15.2.7", | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also a request - could you please also clarify that
jest-axe
is removed from our tool set? Reason is we aren't using it and it was removed from package.json I think awhile ago so its not in use.