-
-
Notifications
You must be signed in to change notification settings - Fork 8.9k
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
feat(theme-classic): use lang attribute in navbar locale dropdown items #7942
Conversation
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
Size Change: +75 B (0%) Total Size: 812 kB
ℹ️ View Unchanged
|
* `<link ... hreflang="...">` | ||
* BCP 47 language tag to use in: | ||
* - `<html lang="...">` | ||
* - `<li lang="...">` |
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.
<li>
or <a>
, or doesn't really matter? Also need to put this into the actual docs since they are in sync.
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.
Replaced by
`<html lang="...">` (or any other DOM tag name)
Somehow HTML, li, a are all the case case 🤪
Motivation
Apparently, this improves accessibility a bit, and also helps SEO crawlers
https://twitter.com/mgechev/status/1518447256034238464
https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/lang
https://www.w3schools.com/tags/att_global_lang.asp
Note: wonder if we shouldn't also use it for localized sites where some docs are not translated yet 🤷♂️ I guess the lang of a doc in such case is not the lang of the i18n site (as the doc is actually not translated) but the default locale instead