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: pwa #9105

Merged
merged 28 commits into from
Aug 9, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
92fc46b
initial strings and tests
exterkamp Jun 2, 2019
91d442d
String feedback
exterkamp Jun 3, 2019
56d0ca3
Changed wording and updated tests. Is-on-https use items.length inst…
exterkamp Jun 3, 2019
c9fb5d1
Added descriptions
exterkamp Jun 3, 2019
7dc454c
missed one
exterkamp Jun 3, 2019
3828cd8
Removed comma.
exterkamp Jun 4, 2019
45e496c
themed omnibox more clear
exterkamp Jun 4, 2019
8f0b99c
Merge branch 'master' into pwa_i18n
exterkamp Jun 25, 2019
ee512e9
revert nulls in proto sample
exterkamp Jun 25, 2019
92261a6
Merge branch 'master' into pwa_i18n
exterkamp Jun 26, 2019
61c0234
collect pwa en-XL, fix untranslated explanation. Thanks en-XL!
exterkamp Jun 26, 2019
29e9468
Merge branch 'master' into pwa_i18n
exterkamp Jul 10, 2019
da92533
Merge branch 'master' into pwa_i18n
exterkamp Jul 25, 2019
e802f1a
update descriptions with examples
exterkamp Jul 25, 2019
64ae1b7
Patrick string feedback.
exterkamp Jul 26, 2019
8ffcaee
fixed test
exterkamp Jul 26, 2019
f96c92c
brendan string foodback 1/few
exterkamp Aug 1, 2019
ba1ba98
loosen test regex
exterkamp Aug 1, 2019
4ab1967
point TODO to gh issue
exterkamp Aug 1, 2019
1fd1385
manual audit desc rewording
exterkamp Aug 1, 2019
0e470cd
Apply Brendan's suggestions from code review
exterkamp Aug 7, 2019
da299d9
updating en and en-xl with new strings
exterkamp Aug 7, 2019
227c97c
update tests, update sample json, fix linting.
exterkamp Aug 7, 2019
f2bb698
fixed offline-start-url test
exterkamp Aug 7, 2019
bf15438
TODO to issue from username
exterkamp Aug 7, 2019
27984af
add viewport code spans!
exterkamp Aug 7, 2019
f49735f
better apple-touch-icon desc
exterkamp Aug 8, 2019
517efd7
brendan last nits
exterkamp Aug 9, 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
48 changes: 26 additions & 22 deletions lighthouse-core/audits/content-width.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@
'use strict';

const Audit = require('./audit.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
title: 'Content is sized correctly for the viewport',
failureTitle: 'Content is not sized correctly for the viewport',
description: 'If the width of your app\'s content doesn\'t match the width ' +
brendankenny marked this conversation as resolved.
Show resolved Hide resolved
'of the viewport, your app might not be optimized for mobile screens. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/content-sized-correctly-for-viewport).',
explanation: 'The viewport size is {innerWidth}px, whereas the window size is {outerWidth}px.',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class ContentWidth extends Audit {
/**
Expand All @@ -14,11 +26,9 @@ class ContentWidth extends Audit {
static get meta() {
return {
id: 'content-width',
title: 'Content is sized correctly for the viewport',
failureTitle: 'Content is not sized correctly for the viewport',
description: 'If the width of your app\'s content doesn\'t match the width ' +
'of the viewport, your app might not be optimized for mobile screens. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/content-sized-correctly-for-viewport).',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['ViewportDimensions', 'TestedAsMobileDevice'],
};
}
Expand All @@ -33,32 +43,26 @@ class ContentWidth extends Audit {
const windowWidth = artifacts.ViewportDimensions.outerWidth;
const widthsMatch = viewportWidth === windowWidth;

if (IsMobile) {
return {
score: Number(widthsMatch),
explanation: this.createExplanation(widthsMatch, artifacts.ViewportDimensions),
};
} else {
if (!IsMobile) {
brendankenny marked this conversation as resolved.
Show resolved Hide resolved
return {
score: 1,
notApplicable: true,
};
}
}

/**
* @param {boolean} match
* @param {LH.Artifacts.ViewportDimensions} artifact
* @return {string}
*/
static createExplanation(match, artifact) {
if (match) {
return '';
let explanation = '';
if (!widthsMatch) {
explanation = str_(UIStrings.explanation,
{innerWidth: artifacts.ViewportDimensions.innerWidth,
outerWidth: artifacts.ViewportDimensions.outerWidth});
}

return 'The viewport size is ' + artifact.innerWidth + 'px, ' +
'whereas the window size is ' + artifact.outerWidth + 'px.';
return {
score: Number(widthsMatch),
explanation,
};
}
}

module.exports = ContentWidth;
module.exports.UIStrings = UIStrings;
20 changes: 15 additions & 5 deletions lighthouse-core/audits/installable-manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,17 @@

const MultiCheckAudit = require('./multi-check-audit.js');
const ManifestValues = require('../computed/manifest-values.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
title: 'Web app manifest meets the installability requirements',
failureTitle: 'Web app manifest does not meet the installability requirements',
description: 'Browsers can proactively prompt users to add your app to their homescreen, ' +
'which can lead to higher engagement. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/**
* @fileoverview
Expand All @@ -29,11 +40,9 @@ class InstallableManifest extends MultiCheckAudit {
static get meta() {
return {
id: 'installable-manifest',
title: 'Web app manifest meets the installability requirements',
failureTitle: 'Web app manifest does not meet the installability requirements',
description: 'Browsers can proactively prompt users to add your app to their homescreen, ' +
'which can lead to higher engagement. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/install-prompt).',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['URL', 'WebAppManifest'],
};
}
Expand Down Expand Up @@ -92,3 +101,4 @@ class InstallableManifest extends MultiCheckAudit {
}

module.exports = InstallableManifest;
module.exports.UIStrings = UIStrings;
37 changes: 24 additions & 13 deletions lighthouse-core/audits/is-on-https.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,25 @@

const Audit = require('./audit.js');
const URL = require('../lib/url-shim.js');
const Util = require('../report/html/renderer/util.js');
const NetworkRecords = require('../computed/network-records.js');
const i18n = require('../lib/i18n/i18n.js');

exterkamp marked this conversation as resolved.
Show resolved Hide resolved
const UIStrings = {
title: 'Uses HTTPS',
failureTitle: 'Does not use HTTPS',
description: 'All sites should be protected with HTTPS, even ones that don\'t handle ' +
'sensitive data. HTTPS prevents intruders from tampering with or passively listening ' +
'in on the communications between your app and your users, and is a prerequisite for ' +
'HTTP/2 and many new web platform APIs. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/https).',
displayValue: `{itemCount, plural,
Copy link
Collaborator

Choose a reason for hiding this comment

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

just for my personal clarity, this is one that would be preserved in the new proposal as "real" ICU syntax for translators?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, this is sent to the translators as "real" ICU. Plurals, and ordinals are the only* ICU syntax passed on.

*afaik

=1 {1 insecure request found}
other {# insecure requests found}
}`,
columnInsecure: 'Insecure URL',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
const SECURE_SCHEMES = ['data', 'https', 'wss', 'blob', 'chrome', 'chrome-extension', 'about'];
const SECURE_DOMAINS = ['localhost', '127.0.0.1'];

Expand All @@ -20,13 +36,9 @@ class HTTPS extends Audit {
static get meta() {
return {
id: 'is-on-https',
title: 'Uses HTTPS',
failureTitle: 'Does not use HTTPS',
description: 'All sites should be protected with HTTPS, even ones that don\'t handle ' +
'sensitive data. HTTPS prevents intruders from tampering with or passively listening ' +
'in on the communications between your app and your users, and is a prerequisite for ' +
'HTTP/2 and many new web platform APIs. ' +
'[Learn more](https://developers.google.com/web/tools/lighthouse/audits/https).',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs'],
};
}
Expand Down Expand Up @@ -54,17 +66,15 @@ class HTTPS extends Audit {
.map(record => URL.elideDataURI(record.url));

let displayValue = '';
if (insecureURLs.length > 1) {
displayValue = `${Util.formatNumber(insecureURLs.length)} insecure requests found`;
} else if (insecureURLs.length === 1) {
displayValue = `${insecureURLs.length} insecure request found`;
if (insecureURLs.length > 0) {
displayValue = str_(UIStrings.displayValue, {itemCount: insecureURLs.length});
}

const items = Array.from(new Set(insecureURLs)).map(url => ({url}));

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
{key: 'url', itemType: 'url', text: 'Insecure URL'},
{key: 'url', itemType: 'url', text: str_(UIStrings.columnInsecure)},
];

return {
Expand All @@ -80,3 +90,4 @@ class HTTPS extends Audit {
}

module.exports = HTTPS;
module.exports.UIStrings = UIStrings;
16 changes: 13 additions & 3 deletions lighthouse-core/audits/manual/pwa-cross-browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,15 @@
'use strict';

const ManualAudit = require('./manual-audit.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
title: 'Site works cross-browser',
description: 'To reach the most number of users, sites should work across ' +
'every major browser. [Learn more](https://developers.google.com/web/progressive-web-apps/checklist#site-works-cross-browser).',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/**
* @fileoverview Manual PWA audit for cross browser support.
Expand All @@ -19,11 +28,12 @@ class PWACrossBrowser extends ManualAudit {
static get meta() {
return Object.assign({
id: 'pwa-cross-browser',
description: 'To reach the most number of users, sites should work across ' +
'every major browser. [Learn more](https://developers.google.com/web/progressive-web-apps/checklist#site-works-cross-browser).',
title: 'Site works cross-browser',
title: str_(UIStrings.title),
description: str_(UIStrings.description),
}, super.partialMeta);
}
}

module.exports = PWACrossBrowser;
module.exports.UIStrings = UIStrings;

15 changes: 12 additions & 3 deletions lighthouse-core/audits/manual/pwa-each-page-has-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
'use strict';

const ManualAudit = require('./manual-audit.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
title: 'Each page has a URL',
description: 'Ensure individual pages are deep linkable via the URLs and that URLs are ' +
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
'unique for the purpose of shareability on social media. [Learn more](https://developers.google.com/web/progressive-web-apps/checklist#each-page-has-a-url).',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/**
* @fileoverview Manual PWA audit to ensure every page has a deep link.
Expand All @@ -18,11 +27,11 @@ class PWAEachPageHasURL extends ManualAudit {
static get meta() {
return Object.assign({
id: 'pwa-each-page-has-url',
description: 'Ensure individual pages are deep linkable via the URLs and that URLs are ' +
'unique for the purpose of shareability on social media. [Learn more](https://developers.google.com/web/progressive-web-apps/checklist#each-page-has-a-url).',
title: 'Each page has a URL',
title: str_(UIStrings.title),
description: str_(UIStrings.description),
}, super.partialMeta);
}
}

module.exports = PWAEachPageHasURL;
module.exports.UIStrings = UIStrings;
15 changes: 12 additions & 3 deletions lighthouse-core/audits/manual/pwa-page-transitions.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,15 @@
'use strict';

const ManualAudit = require('./manual-audit.js');
const i18n = require('../../lib/i18n/i18n.js');

const UIStrings = {
title: 'Page transitions don\'t feel like they block on the network',
description: 'Transitions should feel snappy as you tap around, even on a slow network, a ' +
'key to perceived performance. [Learn more](https://developers.google.com/web/progressive-web-apps/checklist#page-transitions-dont-feel-like-they-block-on-the-network).',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

/**
* @fileoverview Manual PWA audit for janky-free page transitions.
Expand All @@ -18,11 +27,11 @@ class PWAPageTransitions extends ManualAudit {
static get meta() {
return Object.assign({
id: 'pwa-page-transitions',
description: 'Transitions should feel snappy as you tap around, even on a slow network, a ' +
'key to perceived performance. [Learn more](https://developers.google.com/web/progressive-web-apps/checklist#page-transitions-dont-feel-like-they-block-on-the-network).',
title: 'Page transitions don\'t feel like they block on the network',
title: str_(UIStrings.title),
description: str_(UIStrings.description),
}, super.partialMeta);
}
}

module.exports = PWAPageTransitions;
module.exports.UIStrings = UIStrings;
1 change: 1 addition & 0 deletions lighthouse-core/audits/multi-check-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ class MultiCheckAudit extends Audit {
if (result.failures.length > 0) {
return {
score: 0,
// TODO(exterkamp): make this i18n-able.
Copy link
Collaborator

Choose a reason for hiding this comment

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

haven't we sorta cheated similar to this in the past like in error messages? seems like we could do bullets or something

Copy link
Member Author

Choose a reason for hiding this comment

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

Afaict we don't ever really use "failures" because it is just an auto combination of a list of strings into an explanation. I think we just use explanations manually in my experience. We might want to just migrate away from failures totally since this is all they do.

I added this to keep the PR scoped, but I think there is opportunity here for better i18n.

Copy link
Collaborator

Choose a reason for hiding this comment

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

are we tracking this in a mega i18n issue somewhere? if we intend to collect TODOs in the code, I wonder if it might be better to make them all TODO(i18n) or something easily findable when we need to make our next i18n impacting decisions

Copy link
Collaborator

Choose a reason for hiding this comment

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

ping :)

Copy link
Member

Choose a reason for hiding this comment

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

+1 we need a way to track this. I was also told that #7238 is super important for tracking this stuff, so let's actually track some of this stuff in there :P

explanation: `Failures: ${result.failures.join(',\n')}.`,
details,
};
Expand Down
22 changes: 17 additions & 5 deletions lighthouse-core/audits/offline-start-url.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,18 @@
'use strict';

const Audit = require('./audit.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
title: 'start_url responds with a 200 when offline',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
failureTitle: 'start_url does not respond with a 200 when offline',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
description: 'A service worker enables your web app to be reliable in unpredictable ' +
'network conditions. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-200-when-offline).',
warningCantStart: 'We couldn\'t read the start_url from the manifest. As a result, the ' +
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
'start_url was assumed to be the document\'s URL. Error message: \'{manifestWarning}\'.',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class OfflineStartUrl extends Audit {
/**
Expand All @@ -14,9 +26,9 @@ class OfflineStartUrl extends Audit {
static get meta() {
return {
id: 'offline-start-url',
title: 'start_url responds with a 200 when offline',
failureTitle: 'start_url does not respond with a 200 when offline',
description: 'A service worker enables your web app to be reliable in unpredictable network conditions. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-200-when-offline).',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['WebAppManifest', 'StartUrl'],
};
}
Expand All @@ -32,8 +44,7 @@ class OfflineStartUrl extends Audit {
const manifest = artifacts.WebAppManifest;
if (manifest && manifest.value && manifest.value.start_url.warning) {
const manifestWarning = manifest.value.start_url.warning;
warnings.push('We couldn\'t read the start_url from the manifest. As a result, the ' +
`start_url was assumed to be the document's URL. Error message: '${manifestWarning}'.`);
warnings.push(str_(UIStrings.warningCantStart, {manifestWarning}));
}

const hasOfflineStartUrl = artifacts.StartUrl.statusCode === 200;
Expand All @@ -47,3 +58,4 @@ class OfflineStartUrl extends Audit {
}

module.exports = OfflineStartUrl;
module.exports.UIStrings = UIStrings;
18 changes: 14 additions & 4 deletions lighthouse-core/audits/redirects-http.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,16 @@
'use strict';

const Audit = require('./audit.js');
const i18n = require('../lib/i18n/i18n.js');

const UIStrings = {
title: 'Redirects HTTP traffic to HTTPS',
failureTitle: 'Does not redirect HTTP traffic to HTTPS',
description: 'If you\'ve already set up HTTPS, make sure that you redirect all HTTP ' +
'traffic to HTTPS. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-redirects-to-https).',
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class RedirectsHTTP extends Audit {
/**
Expand All @@ -14,10 +24,9 @@ class RedirectsHTTP extends Audit {
static get meta() {
return {
id: 'redirects-http',
title: 'Redirects HTTP traffic to HTTPS',
failureTitle: 'Does not redirect HTTP traffic to HTTPS',
description: 'If you\'ve already set up HTTPS, make sure that you redirect all HTTP ' +
'traffic to HTTPS. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-redirects-to-https).',
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
requiredArtifacts: ['HTTPRedirect'],
};
}
Expand All @@ -34,3 +43,4 @@ class RedirectsHTTP extends Audit {
}

module.exports = RedirectsHTTP;
module.exports.UIStrings = UIStrings;
Loading