Skip to content
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: placeholders #9114

Merged
merged 77 commits into from
Jul 24, 2019
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
77 commits
Select commit Hold shift + click to select a range
656acce
basic format
exterkamp Jun 3, 2019
fc762a5
Basic placeholder syntax.
exterkamp Jun 4, 2019
d1cdeab
minor corrections
exterkamp Jun 4, 2019
74e11c1
iterating
exterkamp Jun 4, 2019
981e028
Added ICU plural and ICU select examples.
exterkamp Jun 4, 2019
6d2dd5d
Gender example has replacement AND ICU.
exterkamp Jun 4, 2019
f22ee1b
less stutter syntax
exterkamp Jun 4, 2019
c66c0c6
docs
exterkamp Jun 4, 2019
70a5a07
Updated README.
exterkamp Jun 12, 2019
a7fc83a
correct-strings
exterkamp Jul 3, 2019
0de5485
updated i18n.js
exterkamp Jul 3, 2019
f8650eb
Merge branch 'master' into placeholders
exterkamp Jul 3, 2019
37dc2ae
remove unused from correct-strings
exterkamp Jul 3, 2019
58c7ded
exported correct-strings to be reused to import. ts first pass on co…
exterkamp Jul 3, 2019
ad01bdd
first pass at new UIStrings format.
exterkamp Jul 3, 2019
4182889
new jsdoc format for comments + _very_ basic parsing.
exterkamp Jul 4, 2019
8b0f8ea
Better parsing, filled out lh-error descriptions.
exterkamp Jul 5, 2019
ddf8171
Fixed tests, made unminified-css and doc-title back to normal.
exterkamp Jul 7, 2019
705ffb6
Updating readme for i18n.
exterkamp Jul 7, 2019
902c2f7
More readme updates.
exterkamp Jul 8, 2019
4507138
Move collect code into seperate util file for testing, and the tests …
exterkamp Jul 8, 2019
29e29df
Add placeholder baking tests.
exterkamp Jul 8, 2019
393beb6
readme updates
exterkamp Jul 8, 2019
799084c
More readme details.
exterkamp Jul 8, 2019
86e1593
Added test for ICU with examples, fixed audits that violated this.
exterkamp Jul 8, 2019
6160402
Patrick foodback
exterkamp Jul 9, 2019
02e8977
ICU select & ordinal readme examples. made runtime example more accu…
exterkamp Jul 9, 2019
f5083b8
collection-util docstrings
exterkamp Jul 9, 2019
5aca05a
remove old console log
exterkamp Jul 9, 2019
ee3e235
update assert on strings to new en-US
exterkamp Jul 9, 2019
6b8297d
polish
exterkamp Jul 9, 2019
77fcd92
refactoring conversion method, testing conversion to follow.
exterkamp Jul 9, 2019
7252f3d
brendan readme foodback.
exterkamp Jul 10, 2019
d4dd3ac
check for empty descriptions
exterkamp Jul 10, 2019
996ee5c
Use UIString as primary fallback, since it is just en. Update fallba…
exterkamp Jul 11, 2019
4ff414a
Untrack pre-locales.
exterkamp Jul 11, 2019
588f574
reorganized collect and correct
exterkamp Jul 11, 2019
2b17130
fixup collect-string output
paulirish Jul 11, 2019
570a6cc
Paul foodback. Readme updates, updated yarn cmds, updated ctc naming …
exterkamp Jul 12, 2019
dcc7cc7
Wording of CTC and LHL in readme and docstrings. Removed mkdirp. Ad…
exterkamp Jul 12, 2019
7743cda
rip my hopes and dreams, thanks linter.
exterkamp Jul 12, 2019
ce01c75
polish to package and to collect-strings
exterkamp Jul 12, 2019
dea280a
better examples!
exterkamp Jul 12, 2019
7f89ccf
correct some complex ICU, add backticks.
exterkamp Jul 15, 2019
cf42496
Merge branch 'master' into placeholders
exterkamp Jul 15, 2019
d156737
Update 'yarn update:sample-json' and json itself
exterkamp Jul 15, 2019
89410f8
Handle the TODOs in readme.
exterkamp Jul 15, 2019
ea3616b
Added @meanings for all duplicated strings.
exterkamp Jul 15, 2019
3d40437
update sample json
exterkamp Jul 15, 2019
bc2b8f3
remove from UIStrings, add autofilled meanings + warning log.
exterkamp Jul 16, 2019
92cafeb
Some comments
exterkamp Jul 16, 2019
3d746d6
added TODO to collect-strings
exterkamp Jul 16, 2019
5ad0dff
Merge branch 'master' into placeholders
exterkamp Jul 17, 2019
c2e4ce5
Paul foodback.
exterkamp Jul 17, 2019
5dd3e07
Brendan baking foodback part 1.
exterkamp Jul 18, 2019
6eef366
update bakery, and tests. Remove space from yarn clean.
exterkamp Jul 18, 2019
a35a45e
lint
exterkamp Jul 18, 2019
13d0f02
foodback round 2, renaming party.
exterkamp Jul 18, 2019
b34ef95
Last part of feedback
exterkamp Jul 18, 2019
48283f4
windows compat collect-strings handled in appropriate PR.
exterkamp Jul 18, 2019
95527d5
Brendan foodback part 2.
exterkamp Jul 19, 2019
327c7db
How'd that var get there.
exterkamp Jul 19, 2019
b006cf7
Brendan feedback v3
exterkamp Jul 21, 2019
51cbb78
added comment to example fallback.
exterkamp Jul 22, 2019
37be64c
Merge branch 'master' into placeholders
exterkamp Jul 22, 2019
d2a8326
complex ICU -> custom-formatted ICU
exterkamp Jul 22, 2019
be426b8
reorg example logic to directIcu replacement.
exterkamp Jul 22, 2019
125cb1d
stop empty plcaeholders from being added to every message. Stricter c…
exterkamp Jul 22, 2019
b95b96b
updated columnName description
exterkamp Jul 22, 2019
90efcff
actually can't assert that in the tests. The undefined part is done …
exterkamp Jul 22, 2019
6e6ef24
@example {example} varName
exterkamp Jul 23, 2019
6a9ef28
whoops. console log left in
exterkamp Jul 23, 2019
7762381
brendan feedback
exterkamp Jul 23, 2019
6c19a17
Merge branch 'master' into placeholders
exterkamp Jul 24, 2019
be34f7e
Add sanity check for ctc files
exterkamp Jul 24, 2019
23a68d5
I feel like my linter is turned off
exterkamp Jul 24, 2019
3685dd6
Merge branch 'master' into placeholders
exterkamp Jul 24, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions lighthouse-core/audits/accessibility/document-title.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,16 @@ const AxeAudit = require('./axe-audit.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
/** Title of an accesibility audit that evaluates if the page has a <title> element that describes the page. This title is descriptive of the successful state and is shown to users when no user action is required. */
title: 'Document has a `<title>` element',
title: 'placeholder',
// title: {
// message: 'Document has a $html_title$ element',
// description: 'Title of an accesibility audit that evaluates if the page has a <title> element that describes the page. This title is descriptive of the successful state and is shown to users when no user action is required.',
// placeholders: {
// html_title: {
// content: '`<title>`',
// },
// },
// },
/** Title of an accesibility audit that evaluates if the page has a <title> element that describes the page. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed. */
failureTitle: 'Document doesn\'t have a `<title>` element',
/** Description of a Lighthouse audit that tells the user *why* they should try to pass. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */
Expand Down
52 changes: 46 additions & 6 deletions lighthouse-core/audits/byte-efficiency/unminified-css.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,50 @@ const computeTokenLength = require('../../lib/minification-estimator.js').comput

const UIStrings = {
/** Imperative title of a Lighthouse audit that tells the user to minify (remove whitespace) the page's CSS code. This is displayed in a list of audit titles that Lighthouse generates. */
title: 'Minify CSS',
/** Description of a Lighthouse audit that tells the user *why* they should minify (remove whitespace) the page's CSS code. 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: 'Minifying CSS files can reduce network payload sizes. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/minify-css).',
title: {
message: 'Minify CSS like {css}',
placeholders: {
css: '`<link rel=stylesheet>`',
},
},
/** (Message Description goes here) Description of a Lighthouse audit that tells the user *why* they should minify (remove whitespace) the page's CSS code. 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: {
message: 'Minifying CSS files can reduce network payload sizes. {link_start}Learn More!!!{link_end}. This audit took {milliseconds} ms.',
placeholders: {
link_start: '[->',
link_end: '](https://developers.google.com/web/tools/lighthouse/audits/minify-css)',
/** 520 (Placeholder examples go here) */
milliseconds: '{timeInMs, number, milliseconds}',
},
},
/** [ICU Syntax] Some plural warning... */
warningPlural: '{itemCount, plural, ' +
'=1 {# error found}' +
'other {# errors found}}',
/** [ICU Syntax] Some gendered (ICU select) explanation... */
explanationGender: {
message: 'Someone minified this, {direct_replace_name}. {static_replacement} {person, select, ' +
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This contains a 1. direct replacement in {ICU string replace} a static_replacement that is covered by a placeholder, and "Google valid" ICU select syntax.

'female {She minified this CSS.} ' +
'male {He minified this CSS.} ' +
'other {They minified this CSS.}}',
placeholders: {
/** Some static replacement. */
static_replacement: '`<link rel=>`',
},
},
/** [ICU Syntax] Some gendered (ICU select) explanation... */
explanationGender2: {
message: 'Someone minified this, {name}. {static_replacement} {person, select, ' +
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example uses placeholders to allow us to make more perfectly formed translation strings, but stutters with {name} in the message, and the content being the same {name} since that is the replacement we want to fill it with. The advantage here is that we can give the translators an example value of the replacement, e.g. here it is a name, so the example like "Karen" would give more context to the translators.

'female {She minified this CSS.} ' +
'male {He minified this CSS.} ' +
'other {They minified this CSS.}}',
placeholders: {
/** Some static replacement. */
static_replacement: '`<link rel=>`',
/** This stutters, BUT we have the opportunity to tell translators an example, and give context? Example text: Karen. */
name: '{name}',
},
},
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
Expand All @@ -34,7 +74,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
return {
id: 'unminified-css',
title: str_(UIStrings.title),
description: str_(UIStrings.description),
description: str_(UIStrings.description, {timeInMs: 10}),
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.NUMERIC,
requiredArtifacts: ['CSSUsage', 'devtoolsLogs', 'traces', 'URL'],
};
Expand Down Expand Up @@ -109,7 +149,7 @@ class UnminifiedCSS extends ByteEfficiencyAudit {
{key: 'wastedBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnWastedBytes)},
];

return {items, headings};
return {items, headings, explanation: str_(UIStrings.explanationGender, {person: 'female', direct_replace_name: 'Kim'}), warnings: [str_(UIStrings.warningPlural, {itemCount: 2})]};
}
}

Expand Down
59 changes: 52 additions & 7 deletions lighthouse-core/lib/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -177,11 +177,11 @@
},
"lighthouse-core/audits/accessibility/document-title.js | failureTitle": {
"message": "Document doesn't have a `<title>` element",
"description": "Title of an accesibility audit that evaluates if the page has a <title> element that describes the page. This title is descriptive of the failing state and is shown to users when there is a failure that needs to be addressed."
"description": "title: {"
},
"lighthouse-core/audits/accessibility/document-title.js | title": {
"message": "Document has a `<title>` element",
"description": "Title of an accesibility audit that evaluates if the page has a <title> element that describes the page. This title is descriptive of the successful state and is shown to users when no user action is required."
"message": "placeholder",
"description": ""
},
"lighthouse-core/audits/accessibility/duplicate-id.js | description": {
"message": "The value of an id attribute must be unique to prevent other instances from being overlooked by assistive technologies. [Learn more](https://dequeuniversity.com/rules/axe/3.1/duplicate-id?application=lighthouse).",
Expand Down Expand Up @@ -508,12 +508,57 @@
"description": "Title of a diagnostic audit that provides detail on large network resources required during page load. 'Payloads' is roughly equivalent to 'resources'. This descriptive title is shown to users when the amount is acceptable and no user action is required."
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | description": {
"message": "Minifying CSS files can reduce network payload sizes. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/minify-css).",
"description": "Description of a Lighthouse audit that tells the user *why* they should minify (remove whitespace) the page's CSS code. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation."
"message": "Minifying CSS files can reduce network payload sizes. $link_start$Learn More!!!$link_end$. This audit took $milliseconds$ ms.",
"description": "(Message Description goes here) Description of a Lighthouse audit that tells the user *why* they should minify (remove whitespace) the page's CSS code. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation.",
"placeholders": {
"link_start": {
"content": "[->"
},
"link_end": {
"content": "](https://developers.google.com/web/tools/lighthouse/audits/minify-css)"
},
"milliseconds": {
"content": "{timeInMs, number, milliseconds}",
"example": "520 (Placeholder examples go here)"
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | explanationGender": {
"message": "Someone minified this, {direct_replace_name}. $static_replacement$ {person, select, female {She minified this CSS.} male {He minified this CSS.} other {They minified this CSS.}}",
"description": "[ICU Syntax] Some gendered (ICU select) explanation...",
"placeholders": {
"static_replacement": {
"content": "`<link rel=>`",
"example": "Some static replacement."
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | explanationGender2": {
"message": "Someone minified this, $name$. $static_replacement$ {person, select, female {She minified this CSS.} male {He minified this CSS.} other {They minified this CSS.}}",
"description": "[ICU Syntax] Some gendered (ICU select) explanation...",
"placeholders": {
"static_replacement": {
"content": "`<link rel=>`",
"example": "Some static replacement."
},
"name": {
"content": "{name}",
"example": "This stutters, BUT we have the opportunity to tell translators an example, and give context? Example text: Karen."
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | title": {
"message": "Minify CSS",
"description": "Imperative title of a Lighthouse audit that tells the user to minify (remove whitespace) the page's CSS code. This is displayed in a list of audit titles that Lighthouse generates."
"message": "Minify CSS like $css$",
"description": "Imperative title of a Lighthouse audit that tells the user to minify (remove whitespace) the page's CSS code. This is displayed in a list of audit titles that Lighthouse generates.",
"placeholders": {
"css": {
"content": "`<link rel=stylesheet>`"
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | warningPlural": {
"message": "{itemCount, plural, =1 {# error found}other {# errors found}}",
"description": "[ICU Syntax] Some plural warning..."
},
"lighthouse-core/audits/byte-efficiency/unminified-javascript.js | description": {
"message": "Minifying JavaScript files can reduce payload sizes and script parse time. [Learn more](https://developers.google.com/speed/docs/insights/MinifyResources).",
Expand Down
20 changes: 18 additions & 2 deletions lighthouse-core/lib/i18n/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function lookupLocale(locale) {
*/
function _preprocessMessageValues(icuMessage, values) {
if (!values) return;

// console.log(icuMessage, values);
const clonedValues = JSON.parse(JSON.stringify(values));
const parsed = MessageParser.parse(icuMessage);
// Throw an error if a message's value isn't provided
Expand Down Expand Up @@ -174,7 +174,23 @@ const _icuMessageInstanceMap = new Map();
*/
function _formatIcuMessage(locale, icuMessageId, icuMessage, values) {
const localeMessages = LOCALES[locale];
const localeMessage = localeMessages[icuMessageId] && localeMessages[icuMessageId].message;
let localeMessage = localeMessages[icuMessageId] && localeMessages[icuMessageId].message;
// lets try to replace some things :o
const placeholders = localeMessages[icuMessageId] && localeMessages[icuMessageId].placeholders;
if (placeholders) {
// do some regex
Object.entries(placeholders).forEach(entry => {
const key = entry[0];
const value = entry[1];
//use key and value here
let regexStr = `\$${key}\$`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we'll definitely want a collect-strings test that verifies that all the placeholders are in the string (would we also want the stricter check that there aren't any escaped values that don't have placeholders?)

// console.log(key, value, regexStr);
localeMessage = localeMessage.replace(regexStr, value.content);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe to avoid the issue with positional replacements ($1 etc), we could use use the named replacements in values if value.content is \$\d+ or whatever? We'll just have to figure out how to do the mapping (just use key?)

// console.log(localeMessage);
});
icuMessage = localeMessage;
}

// fallback to the original english message if we couldn't find a message in the specified locale
// better to have an english message than no message at all, in some number cases it won't even matter
const messageForMessageFormat = localeMessage || icuMessage;
Expand Down
46 changes: 44 additions & 2 deletions lighthouse-core/lib/i18n/locales/de.json
Original file line number Diff line number Diff line change
Expand Up @@ -369,10 +369,52 @@
"message": "Vermeidet sehr große Netzwerknutzlasten"
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | description": {
"message": "Durch die Komprimierung von CSS-Dateien kann die Größe von Netzwerknutzlasten reduziert werden. [Weitere Informationen](https://developers.google.com/web/tools/lighthouse/audits/minify-css)."
"message": "Durch die Komprimierung von CSS-Dateien kann die Größe von Netzwerknutzlasten reduziert werden. $link_start$Weitere Informationen$link_end$. This audit took $milliseconds$ ms.",
"placeholders": {
"link_start": {
"content": "[->"
},
"link_end": {
"content": "](https://developers.google.com/web/tools/lighthouse/audits/minify-css)"
},
"milliseconds": {
"content": "{timeInMs, number, milliseconds}",
"example": "520"
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | title": {
"message": "CSS komprimieren"
"message": "CSS komprimieren $css$",
"placeholders": {
"css": {
"content": "`<link rel=stylesheet>`"
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | explanationGender": {
"message": "<GERMAN> Someone minified this, {direct_replace_name}. $static_replacement$ {person, select, female {<GERMAN> She minified this CSS.} male {<GERMAN> He minified this CSS.} other {<GERMAN> They minified this CSS.}}",
"placeholders": {
"static_replacement": {
"content": "`<link rel=>`",
"example": "Some static replacement."
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | explanationGender2": {
"message": "Someone minified this, $name$. $static_replacement$ {person, select, female {She minified this CSS.} male {He minified this CSS.} other {They minified this CSS.}}",
"placeholders": {
"static_replacement": {
"content": "`<link rel=>`",
"example": "Some static replacement."
},
"name": {
"content": "{name}",
"example": "This stutters, BUT we have the opportunity to tell translators an example, and give context? Example text: Karen."
}
}
},
"lighthouse-core/audits/byte-efficiency/unminified-css.js | warningPlural": {
"message": "{itemCount, plural, =1 {<GERMAN> # error found}other {<GERMAN> # errors found}}"
},
"lighthouse-core/audits/byte-efficiency/unminified-javascript.js | description": {
"message": "Durch die Komprimierung von JavaScript-Dateien können Nutzlastgrößen und die Zeit zum Parsen von Skripts reduziert werden. [Weitere Informationen](https://developers.google.com/speed/docs/insights/MinifyResources)."
Expand Down
58 changes: 54 additions & 4 deletions lighthouse-core/scripts/i18n/collect-strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,60 @@ function collectAllStringsInDir(dir, strings = {}) {
let lastPropertyEndIndex = 0;
for (const property of stmt.declarations[0].init.properties) {
const key = property.key.name;
const message = exportVars.UIStrings[key];
const description = computeDescription(ast, property, lastPropertyEndIndex);
strings[`${relativePath} | ${key}`] = {message, description};
lastPropertyEndIndex = property.range[1];
const val = exportVars.UIStrings[key];
if (typeof val === 'string') {
const description = computeDescription(ast, property, lastPropertyEndIndex);
strings[`${relativePath} | ${key}`] = {message: val, description};
lastPropertyEndIndex = property.range[1];
} else {
// console.log(property.value.properties[0].range[1]);
// console.log(property.value.properties[1].value.properties);
let message = val.message;
// const prevProp = property.value.properties[1].value.properties[1];
// const thisProp = property.value.properties[1].value.properties[2];

const description = computeDescription(ast, property, lastPropertyEndIndex);
/**
* Transform:
* placeholders: {
* /** example val *\/
* key: value,
* ...
* },
* Into:
* placeholders: {
* key: {
* content: value,
* example: example val,
* },
* ...
* }
*/
// init last prop to the 'messages' end range
let lastPropEndIndex = property.value.properties[0].range[1];
let idx = 0;
const placeholdersMini = val.placeholders;
const placeholders = {};
Object.entries(placeholdersMini).forEach(entry => {
const key = entry[0];
const value = entry[1];
const thisProp = property.value.properties[1].value.properties[idx];
const thisDesc = computeDescription(ast, thisProp, lastPropEndIndex);
placeholders[key] = {
content: value,
};
if (thisDesc) {
placeholders[key].example = thisDesc;
}

// replace {.*} with $.*$
message = message.replace(`{${key}}`, `\$${key}\$`);
idx++;
lastPropEndIndex = thisProp.range[1];
});
strings[`${relativePath} | ${key}`] = {message, description, placeholders};
lastPropertyEndIndex = property.range[1];
}
}
}
}
Expand Down