-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
i18n: localize audits in best-practices #9092
Conversation
By this do you mean move all best-practices audits into |
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.
Awesome!! 🇪🇸 🇨🇳 🇩🇪
displayValue = `${Util.formatNumber(insecureURLs.length)} insecure requests found`; | ||
} else if (insecureURLs.length === 1) { | ||
displayValue = `${insecureURLs.length} insecure request found`; | ||
if (items.length > 0) { |
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 moved this to be items.length
instead of insecureURLs.length b/c items de-dupes the list and so the displayValue would show "3" when the test for dedupe wanted "2". Before it was a loose match
but now it is exact, and made me hunt down why the displayValue differed from the extendedInfo.value and this is what I found.
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 moved this to be items.length instead of insecureURLs.length b/c items de-dupes the list
yes, very good call
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 reviewed from the top down through password-inputs...
.
Brendan did from the bottom up to uses-http2
.
@@ -5060,6 +5060,23 @@ | |||
"warningHeader": "Warnings: " | |||
}, | |||
"icuMessagePaths": { |
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.
icuMessagePaths
😱gets a quite a bit longer on each of these PRs :)
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.
retweet 🐦
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.
We could make ICUmsg paths optional, only add it when requested... 🤷♂
@@ -94,6 +94,8 @@ const UIStrings = { | |||
seoCrawlingGroupTitle: 'Crawling and Indexing', | |||
/* Description of the navigation section within the Search Engine Optimization (SEO) category. Within this section are audits with descriptive titles that highlight ways to make a website accessible to search engine crawlers. */ | |||
seoCrawlingGroupDescription: 'To appear in search results, crawlers need access to your app.', | |||
/** Title of the Best Practices category of audits. This is displayed at the top of a list of audits focused on topics related to following web development best practices and accepted guidelines. Also used as a label of a score gauge; try to limit to 20 characters. */ |
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 love how we write "Also used as a label of a score gauge" on the category titles as if anyone will know what that means :)
lighthouse-core/audits/dobetterweb/uses-passive-event-listeners.js
Outdated
Show resolved
Hide resolved
Co-Authored-By: Paul Irish <paul.irish@gmail.com> Co-Authored-By: Brendan Kenny <bckenny@gmail.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
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.
some more suggestions but otherwise LGTM
/** Description of a Lighthouse audit that tells the user why they should maintain the correct aspect ratios for all images. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | ||
description: 'Image display dimensions should match natural aspect ratio. ' + | ||
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/aspect-ratio).', | ||
/** Warning that the sizing information cannot be collected for an image. */ |
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.
happy if this uses a better description than my suggestion, but the current version doesn't match well with the message string :)
}`, | ||
/** Table column header for the version of the Javascript library found. */ | ||
columnVersion: 'Library Version', | ||
/** Table column header for the count of vulnerabilities found within a JavaSscript library. */ |
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.
/** Table column header for the count of vulnerabilities found within a JavaSscript library. */ | |
/** Label for a column in a data table; entries will be the counts of JavaScript-library vulnerabilities found. */ |
lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
Outdated
Show resolved
Hide resolved
lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
Outdated
Show resolved
Hide resolved
lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google. ℹ️ Googlers: Go here for more info. |
Summary
i18n best practices!
Follow ups:
move errant best-prac audits into dobetterweb?move all best-practices audits tobest-practices/
folderextended_info
inbest-practices
Part of: #7238