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.

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

Add pa11y-ci for accesibility linting #1046

Draft
wants to merge 53 commits into
base: main
Choose a base branch
from

Conversation

TrialDragon
Copy link
Member

This adds a new CI job for pa11y-ci in order to automate checking if the site, or PR to the site, satisfies basic accessibility guidelines.

Closes #396

@TrialDragon TrialDragon added C-Feature A new feature, making something new possible C-Accessibility A-Build-System labels Feb 22, 2024
@TrialDragon TrialDragon force-pushed the 396_add_accesibility_lints_to_ci branch from 5fc13ed to 64a60ba Compare February 22, 2024 05:54
@TrialDragon TrialDragon force-pushed the 396_add_accesibility_lints_to_ci branch from 5de128b to 6afe97a Compare February 22, 2024 06:12
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@TrialDragon
Copy link
Member Author

TrialDragon commented Feb 22, 2024

Mm, switching to echo broke it further. Edit: Fixed it with the -e argument. Also there is probably still the puppeteer issue regarding the chrome executable. I'll continue work on this tomorrow. (Open to suggestions regarding causes and fixes for this.)

@TrialDragon
Copy link
Member Author

TrialDragon commented Feb 22, 2024

It's working finally! Now to actually solve the accessibility issues—after #1026 is merged since it already solves some of the issues with image height width attributes and alt text.

@BD103
Copy link
Member

BD103 commented Feb 22, 2024

There's a chance a lot of the accessibility errors are duplicated, since we use templates. I know there are also a few dead pages with the Zola welcome page. If those can be removed, CI times should decrease.

Does pa11y-ci run Chrome for accessibility checking? Is there an alternative tool that doesn't take as much time?

@TrialDragon
Copy link
Member Author

TrialDragon commented Feb 22, 2024

There's a chance a lot of the accessibility errors are duplicated, since we use templates. I know there are also a few dead pages with the Zola welcome page. If those can be removed, CI times should decrease.

Does pa11y-ci run Chrome for accessibility checking? Is there an alternative tool that doesn't take as much time?

Yes, it does run Chrome via Puppeteer. There are alternative tools; they're paid / cost money. (Axe by Deque, and AccessLint). The latter can be technically free for personal accounts, but not for our use case afaik. To my knowledge pa11y is the one free tool in this sector. (Maybe ESLint has somethings, but it's not a dedicated accessibility lint and I couldn't find good info on it).

The dead pages should be gotten rid of with ... o, damn, the PR that fixed those things was closed. (#601). I guess I can open up a new one to get rid of the whole improper use of pages for community stuff that plagues us.

Some of the issues are being duplicated due to templates, yes. Not sure if there is a good way to solve it.

@BD103
Copy link
Member

BD103 commented Feb 22, 2024

The dead pages should be gotten rid of with ... o, damn, the PR that fixed those things was closed. (#601). I guess I can open up a new one to get rid of the whole improper use of pages for community stuff that plagues us.

Yes, that would be awesome! There's quite a few: I recommend building the website and grep-ing Welcome to Zola! to find all of the pages. Ones being built for each Bevy Asset, error, people page, donation page, and probably more.

Some of the issues are being duplicated due to templates, yes. Not sure if there is a good way to solve it.

Unfortunately I don't think it's something that can be solved. My point was just that some errors may be overly-inflated due to duplicated HTML.

@TrialDragon
Copy link
Member Author

Next step is to actually add the dummy files meant to represent the generic assets section, examples section, example page, community things, et cetera. Just removing the dependencies now to gauge how fast this will be approximately.

@TrialDragon TrialDragon force-pushed the 396_add_accesibility_lints_to_ci branch from c4d2831 to e5aaed0 Compare February 26, 2024 22:00
@TrialDragon
Copy link
Member Author

Okay, it should now cover all the areas without having to wait for things to be generated. Will fix the actual accessibility issues it catches later, after #1026 is merged, or closed, in the indeterminate future.

@TrialDragon
Copy link
Member Author

Okay, looking thru the complaints the CI is now officially complaining about the invalid pages. So, uh, I guess this is blocked from merging until that's dealt with.

@TrialDragon TrialDragon force-pushed the 396_add_accesibility_lints_to_ci branch from 851d9ea to 202afb1 Compare March 1, 2024 21:36
@TrialDragon
Copy link
Member Author

Okay last two issues is getting rid of invalid pages and adding aria labels to the image comparing widget. I'll wait until the shortcode in #710 is merged so that stuff isn't being unnecessarily repeated creating merge conflicts that could be avoided.

To try and prevent starting jobs when the CI will fail anyway.
@SIGSTACKFAULT
Copy link
Contributor

SIGSTACKFAULT commented May 5, 2024

@TrialDragon I adopted #710 and shortcodes were merged in #1144
If I'd known about this I would have added labels >.<

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add accessibility lints to CI
4 participants