test: Bring accessibility up to 100% in Lighthouse check #606
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fix the errors reported by Lighthouse.
Added the Lighthouse CI config file. I had to tweak the info message server/index.js emits, because Lighthouse needs to see something like "Listening on..." to detect that the server has started.
Lighthouse CI should now run in pre-flight. The scores for accessibility and best practices need to be 100 for CI to pass.
Adding a title for accessibility to Header changed its test snapshot, so it had to be updated.
Pull the latest version of @lhci/cli at major version 11.
Some of the dev dependencies are needed for the webpack build, so let's just install everything.
Lighthouse needs chromium, but trying to test its installation in a Github action is exceedingly difficult even with nektos/act. Let's see if this works.
Pulls in browserless/chrome docker image to have chrome pre-installed. I'm having a lot of trouble with the PROTOCOL_TIMEOUT error during lhci autorun, which is still unresolved by Lighthouse (it's the #1 issue on the GoogleChrome/lighthouse repo).
I do not want to deal with git auth in docker images.
Added the Lighthouse token so we can see status checks. The lighthouse github action I pulled in has some issue with CHROME_PATH, so I'm going back to running lhci autorun by itself.
![reviewify-code-review[bot]](https://avatars.githubusercontent.com/in/269822?s=60&v=4){kind=small}
reviewify recommended I remove it, so I did. The machines tell us what to do now.
zoobot
reviewed
Apr 3, 2023
zoobot
reviewed
Apr 3, 2023
zoobot
added a commit
that referenced
this pull request
Apr 5, 2023
* fix: resolve accessibility issues Fix the errors reported by Lighthouse. * test: Add Lighthouse CI locally Added the Lighthouse CI config file. I had to tweak the info message server/index.js emits, because Lighthouse needs to see something like "Listening on..." to detect that the server has started. * ci: Added Lighthouse CI github action Lighthouse CI should now run in pre-flight. The scores for accessibility and best practices need to be 100 for CI to pass. * test: Update snapshots Adding a title for accessibility to Header changed its test snapshot, so it had to be updated. * fix: pull the right @lhci/cli version Pull the latest version of @lhci/cli at major version 11. * fix: install all npm dependencies Some of the dev dependencies are needed for the webpack build, so let's just install everything. * fix: install chromium in github action Lighthouse needs chromium, but trying to test its installation in a Github action is exceedingly difficult even with nektos/act. Let's see if this works. * test: Use a docker image for chrome Pulls in browserless/chrome docker image to have chrome pre-installed. I'm having a lot of trouble with the PROTOCOL_TIMEOUT error during lhci autorun, which is still unresolved by Lighthouse (it's the #1 issue on the GoogleChrome/lighthouse repo). * test: just download chrome manually I do not want to deal with git auth in docker images. * test: Add Lighthouse token Added the Lighthouse token so we can see status checks. The lighthouse github action I pulled in has some issue with CHROME_PATH, so I'm going back to running lhci autorun by itself. * fix: remove disable-storage-reset reviewify recommended I remove it, so I did. The machines tell us what to do now. * test: Use latest github actions --------- Co-authored-by: Rose <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #438.
Abstract
There are two parts to this: fixing the outstanding accessibility issues that Lighthouse reports, and then adding Lighthouse to our CI to ensure that there are no regressions to accessibility in the future.
Fixing accessibility issues
There were two types of errors. The first one has to do with some buttons not having a name which can be seen by screen readers. There are multiple ways to fix this, and my solution was to add a
titleattribute to the buttons. The side effect of this is that these buttons now also show tooltip text matching the title. @ri0nardo for reference, these are the buttons changed:The Menu button:
Screen recording 2023-03-30 16.55.30.webm
the Geolocate button:
Screen recording 2023-03-30 16.54.58.webm
and the
NewTreeButton:Screen recording 2023-03-30 16.56.13.webm
Adding the title required updating the
Header.test.jssnapshot.The other type of error was that zoom should be enabled on the map.
Adding Lighthouse to CI
It's great to be able to fix these issues, but the only way you can make the fix last is to add regression testing. I accomplish this by adding Lighthouse to our CI pre-flight. The Lighthouse test will fail if the scores for accessibility or best practices are under 100, so future developers submitting PRs will need to make sure that their UI features meet the standards of Lighthouse. The best, recommended practice for Lighthouse CI is to require 100 from the SEO and PWA scores as well, as performance can give variable results, but fixing those issues is a whole separate issue of work.
To see the results of our Lighthouse runs in pre-flight, I installed the Lighthouse Github app and run the action with the
LHCI_GITHUB_APP_TOKENenv variable. You should be seeing a status in the pre-flight with the Lighthouse scores and the icon below.Actually getting the github action to work was quite difficult, mainly because Lighthouse needs to have chrome installed and the different solutions I tried to install Chrome didn't seem to work (using a docker image, using a github action) etc. In the end, just manually downloading the chrome .deb debian file worked best, although I fear pulling the .deb by wget could be flaky if it 404s.
I used nektos/act to test my github action locally, so I could get faster turn-around and not submit a hundred github commits just to test my github action. That approach has its limits too though, as the action would fail due to a PROTOCOL_TIMEOUT error that is apparently the #1 most common issue with Lighthouse.
Overall, I like that we can do automated testing with Lighthouse CI, but the tool seems a lot buggier than I would expect from Google devs. It's good enough.