cal-itp / benefits

Transit benefits enrollment, minus the paperwork.
https://docs.calitp.org/benefits
GNU Affero General Public License v3.0
27 stars 9 forks source link

GitHub Actions: Run accessibility checks on additional pages #1278

Closed afeld closed 1 year ago

afeld commented 1 year ago

We currently run Lighthouse on a couple pages:

https://github.com/cal-itp/benefits/blob/f2503e5efb6bf4678f477045d964c64ecfd97c7f/.github/workflows/tests-ui.yml#L44-L49

It would be helpful to have it (or some other automated accessibility checker run against other pages as well.

Acceptance Criteria

Additional context

Some options:

indexing commented 1 year ago

@machikoyasuda The other day you mentioned we are running Lighthouse checks, so I believe we can close this issue?

machikoyasuda commented 1 year ago

@indexing This issue is referring to adding automation to run Lighthouse checks on every page on every PR. So far, we're only running it on the home page and the help page. Not sure if this Issue is a priority anymore though.

machikoyasuda commented 1 year ago

@indexing To me, this seems like something that could be a 1-week stretch ""fun"" ticket for a developer to work on during a slush week, if there aren't any other similar tickets. It's definitely not a must have.

indexing commented 1 year ago

@thekaveman What are you thoughts? Is it worth expanding the views we're checking via Lighthouse?

thekaveman commented 1 year ago

I agree with @machikoyasuda this seems like a good exercise to timebox for a Stretch week.

Generally it would be good to do more automated checking like this (I rarely if ever remember to run Lighthouse myself locally). Let's see what we can accomplish in a reasonable amount of time using just Cypress and one of the plugins above, but without worrying about mocking all the external services or getting through e.g. Login.gov just yet.

machikoyasuda commented 1 year ago

Beginning preliminary research on this. May continue it into next week/stretch week.

machikoyasuda commented 1 year ago

After extensive testing #1675 last week, I've come to the conclusion that, at this time, we shouldn't add the cypress-axe or cypress-audit tests to our continuous integration GitHub Actions. The tests worked perfectly well locally, but resulted in false positives when run on GitHub Actions.

Steps we can take now:

  1. Skip nav color contrast - Affects every page.
  2. Headline hierarchy bug (#1528 bug reported here) for pages that use a Media Item, but don't have a H2. - Affects 1 page.
  3. Aria labeling for Modals - Affects all pages with modals.

Either way, we have to remember that automated testing only provides so much reassurance - and we'll always need some manual testing. And also, that certain accessibility issues (like color contrast) should be detected earlier at the design level.

As an example, both of the aforementioned tools weren't able to catch that a button had insufficient color contrast - because aria was set to disabled for that button! When I took aria-disabled=true out from the button, both tools were able to notice the button's issues.

cc @thekaveman @angela-tran

thekaveman commented 1 year ago

@machikoyasuda what do you think about closing this issue now that you've done the research and made recommendations?