-
Notifications
You must be signed in to change notification settings - Fork 311
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 scanner #3949
Add pa11y scanner #3949
Conversation
Previously, we did accessibility scanning for every file in every push / build / PR, which took > 25 mins each run. Now, we more carefully curate the files that need to be scanned, to limit the scan time. This commit: - Adds `npm run pa11y:list-files` command which runs a site build and outputs the files to be scanned for the current changeset - Adds a per-page pa11y scan, which lists pages that have changed or whose layouts have changed - Adds a site-wide pa11y scan: if files that affect the whole site change, take a sample of files from across the site to check - Incidentally deletes an unused layout - Moves _plugins/ to lib/ and ignores lib/ in the 11ty build - Pins the glob package to 8, because 9 and higher breaks something with the SVG Sprite package The code reads a little weird, and one more refactor could help.
39fb4f9
to
7f5c57d
Compare
I added some non blocking suggestions related to readability, this feels like a great start Matt! |
lib/pa11y.js
Outdated
} | ||
|
||
function didSitewideFilesChange() { | ||
const result = doesIntersect(changedFileSet(), sitewideFileSet()) |
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.
Non-blocking suggestion here, I might rename result
to filesChanged
.
lib/pa11y.js
Outdated
return filesToWrite | ||
} | ||
|
||
async function pa11yScan ({ dir, results }) { |
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.
Non blocking question/suggestion: results
sounds like it's an array of pages/files/documents sent from eleventy.after
, if that's correct then maybe renaming results
to have a little more context could help readability? If so I think the same renaming should occur where results
is passed to the pa11yScan
function.
Previously, we did accessibility scanning for every file in every push / build / PR, which took > 25 mins each run.
Now, we more carefully curate the files that need to be scanned, to limit the scan time.
This PR:
Commentary:
This commit gets us to the "Make it work" stage, not yet addressing "Make it right" or "Make it fast".
The code also reads a little weird, and I'm not sure a conditional "eleventy.after" event is the right design for this. I think one more refactor here would be wise.
Closes #3875