Added Cypress Axe. #103
Added Cypress Axe. #103
Conversation
PR ReportBundle SizesView bundle sizes for 'client'
Overall bundle size change: 0%View bundle sizes for 'server'
Overall bundle size change: 0.26%Test CoverageView test coverage
Triggered by commit: a4c0f81 |
| }); | ||
|
|
||
| Then(`the welcome message appears`, () => { | ||
| cy.get('#root').contains(`Welcome to the Strimzi UI`); | ||
| cy.checkA11y(); |
There was a problem hiding this comment.
https://www.npmjs.com/package/cypress-axe#using-the-violationcallback-argument - I'd like if we had support for this to log out to console.
Otherwise, when test fails - while we can watch the video that GHA uploads as an artifact, it would be good to have failure in the workflow log.
There was a problem hiding this comment.
Also - Carbon is not always 100% accessible - how do you flag particular rules as "ignore for this element"?
There was a problem hiding this comment.
Finally - we will need to discuss "what rules should we run" and "what impact level fails". Can you propose a default set?
There was a problem hiding this comment.
We can disable rules that the frameworks don't support. On patternfly we try to not disable rules.
| @@ -5,9 +5,10 @@ | |||
| import { Given, Then } from 'cypress-cucumber-preprocessor/steps'; | |||
|
|
|||
| Given('I am on the strimzi-ui homepage', () => { | |||
| cy.visit('localhost:3000/index.html'); | |||
| cy.visit('localhost:8080/index.html'); | |||
There was a problem hiding this comment.
Why the port change? When your run npm run e2e it should be starting up express on 3000
There was a problem hiding this comment.
Was a mistake I will fix it
| ); | ||
|
|
||
| Cypress.Commands.add('testA11y', (target: string) => { | ||
| cy.injectAxe(); |
There was a problem hiding this comment.
Can the inject and configure be run multiple times inside a test (expect there will be multiple calls to testA11y - or does it have to be ran only once per test?
There was a problem hiding this comment.
It only has to be run once.
There was a problem hiding this comment.
cy.testally would be called multiple times - so should cy.injextAxe() and configureAxe() instead be in a before all?
There was a problem hiding this comment.
yeah that makes sense I'll modify it.
| cy.configureAxe({ | ||
| rules: [ | ||
| { id: 'color-contrast', enabled: false }, // seem to be somewhat inaccurate and has difficulty always picking up the correct colors, tons of open issues for it on axe-core | ||
| { id: 'focusable-content', enabled: false }, // recently updated and need to give the PF team time to fix issues before enabling |
There was a problem hiding this comment.
Are the issues being fixed in Axe by PF, or are rules disabled until new version of PF is released?
There was a problem hiding this comment.
Currently they are being fixed by the patternfly team
package.json
Outdated
| @@ -63,9 +63,8 @@ | |||
| "pino-filter": "^1.0.0", | |||
| "pino-http": "^5.3.0", | |||
| "pino-pretty": "^4.3.0", | |||
| "sass": "^1.29.0", | |||
| "react-i18next": "^11.7.3", | |||
| "sass": "^1.26.10", | |||
There was a problem hiding this comment.
When you merge package.json please ensure only one sass entry and version 1.29.0
There was a problem hiding this comment.
Thought I fixed that thanks for the catch.
|
@dlabaj few outstanding questions that are blocking this being merged (post-rebase) |
dlabaj
force-pushed
the
cypress-a11y
branch
3 times, most recently
from
f58a9f9
to
1c55d6a
Compare
Fixes issue strimzi#20 using cypress axe. Signed-off-by: Donald Labaj <[email protected]>
| ); | ||
|
|
||
| Cypress.Commands.add('testA11y', (target: string) => { | ||
| cy.injectAxe(); |
There was a problem hiding this comment.
cy.testally would be called multiple times - so should cy.injextAxe() and configureAxe() instead be in a before all?
Fixes issue #20 using cypress axe.
Signed-off-by: Donald Labaj [email protected]