-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[lighthouse] migrate accessibility content from web.dev #3768
Conversation
✅ Deploy Preview for developer-chrome-com ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
|
Hello! This is an automated review by our custom reviewbot. It updates automatically when code or GitHub comments in this pull request are created or updated. Requested changesIf there are any common problems with the content files you created or modified, they will be listed here.
|
| @@ -1,13 +1,16 @@ | |||
| - url: /docs/lighthouse/overview/ | |||
| - url: /docs/lighthouse/seo/ | |||
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.
rearranged this so that the TOC is in the same order as the Lighthouse report (perf, a11y, best practices, seo, pwa)
| {% Glitch { | ||
| id: 'gorgeous-raven', | ||
| path: 'example.html:13:39', | ||
| height: 346 | ||
| } %} |
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.
unfortunately several of these rely on a hosted form of chromevox-lite (in this example at https://robdodson.github.io/chromevox-lite/dist/index.min.mjs) which doesn't exist in that repo anymore and doesn't seem to be hosted elsewhere now. If we can't find a replacement, I guess we'll just delete?
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.
I suppose if that's what we need to do until we can find an interim solution. Is there a way to comment out nunjucks syntax? Perhaps it could be fixed later.
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.
I suppose if that's what we need to do until we can find an interim solution. Is there a way to comment out nunjucks syntax? Perhaps it could be fixed later.
Commented it out and left an short explanation.
I was wrong and the other glitches are actually fine, it's just this one, so it's not a huge deal if we can't get a replacement.
| --- | ||
| layout: 'layouts/project-landing.njk' | ||
| title: 'Accessibility Audits' | ||
| description: 'These checks highlight opportunities to improve the accessibility of your web app. Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.' |
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.
since the TOC is already generated per locale, it would be nice if this could use i18n.docs.lighthouse-accessibility.overview so you wouldn't need an index.md per locale
malchata
left a comment
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.
Cursory look, but LGTM
Adds a Lighthouse accessibility docs section and migrates the last of the content from web.dev (the rest of the accessibility audits were redirected to axe docs in GoogleChrome/web.dev#8654).
layoutand updated links where neededscoring/index.md- I regenerated the accessibility weight table because it was super out of date. I don't know if there's a ton of value in seeing the audit weights (we don't have a similar page for best practices/SEO, for instance), but there are ~1500 visitors a month, so we can just keep the page.