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 27 commits
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
8 changes: 4 additions & 4 deletions lighthouse-core/audits/apple-touch-icon.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,10 @@ const UIStrings = {
title: 'Provides a valid `apple-touch-icon`',
/** Title of a Lighthouse audit that tells the user that their site contains a vaild touch icon. This descriptive title is shown when the page does not contain a valid iOS touch icon. "apple-touch-icon" is an HTML attribute value and should not be translated. */
failureTitle: 'Does not provide a valid `apple-touch-icon`',
/** Description of a Lighthouse audit that tells the user what having a valid apple-touch-icon does. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. "apple-touch-icon" is an HTML attribute value and should not be translated. */
description: 'For ideal appearance on iOS when users add to the home screen, define an ' +
'apple-touch-icon. It must point to a non-transparent 192px (or 180px) square PNG. ' +
'[Learn More](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/).',
/** Description of a Lighthouse audit that tells the user that having an apple-touch-icon allows websites to include an app icon to their installed progressive web apps, similar to a native app. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. "apple-touch-icon" is an HTML attribute value and should not be translated. */
description: 'For ideal appearance on iOS when users add a progressive web app to the home ' +
'screen, define an `apple-touch-icon`. It must point to a non-transparent 192px (or 180px) ' +
exterkamp marked this conversation as resolved.
Show resolved Hide resolved
'square PNG. [Learn More](https://developers.google.com/web/fundamentals/design-and-ux/browser-customization/).',
/** Warning that HTML attribute `apple-touch-icon-precomposed` should not be used in favor of `apple-touch-icon`. "apple-touch-icon-precomposed" and "apple-touch-icon" are HTML attribute values and should not be translated. */
precomposedWarning: '`apple-touch-icon-precomposed` is out of date; ' +
'`apple-touch-icon` is preferred.',
Expand Down
57 changes: 35 additions & 22 deletions lighthouse-core/audits/content-width.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,27 @@
'use strict';

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

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on the content size of a web site compared to the viewport, which is the size of the screen the site is displayed on. This descriptive title is shown to users when the site's content is sized appropriately. */
title: 'Content is sized correctly for the viewport',
/** Title of a Lighthouse audit that provides detail on the content size of a web site compared to the viewport, which is the size of the screen the site is displayed on. This descriptive title is shown to users when the site's content is not sized appropriately. */
failureTitle: 'Content is not sized correctly for the viewport',
/** Description of a Lighthouse audit that tells the user why they should care that a site's content size should match its viewport size, which is the size of the screen the site is displayed on. 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: '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).',
/**
* @description Explanatory message stating that the viewport size and window size differ.
* @example {100} innerWidth
* @example {101} outerWidth
* */
explanation: 'The viewport size of {innerWidth}px does not match the window ' +
'size of {outerWidth}px.',
};

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

class ContentWidth extends Audit {
/**
Expand All @@ -14,11 +35,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 +52,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;
23 changes: 18 additions & 5 deletions lighthouse-core/audits/installable-manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,20 @@

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

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on if a website is installable as an application. This descriptive title is shown to users when a webapp is installable. */
title: 'Web app manifest meets the installability requirements',
/** Title of a Lighthouse audit that provides detail on if a website is installable as an application. This descriptive title is shown to users when a webapp is not installable. */
failureTitle: 'Web app manifest does not meet the installability requirements',
/** Description of a Lighthouse audit that tells the user why installability is important for webapps. 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: '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 +43,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 +104,4 @@ class InstallableManifest extends MultiCheckAudit {
}

module.exports = InstallableManifest;
module.exports.UIStrings = UIStrings;
8 changes: 5 additions & 3 deletions lighthouse-core/audits/load-fast-enough-for-pwa.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ const UIStrings = {
/** Label for the audit identifying the time it took for the page to become interactive on a mobile network. */
displayValueTextWithOverride: 'Interactive on simulated mobile network at ' +
'{timeInMs, number, seconds}\xa0s',
/** Explanatory message displayed when a web page loads too slowly to be considered quickly interactive. This references another Lighthouse auditing category, "Performance", that can give additional details on performance debugging. */
explanationLoadSlow: 'Your page loads too slowly and is not interactive within 10 seconds. ' +
'Look at the opportunities and diagnostics in the "Performance" section to learn how to ' +
'improve.',
};

const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);
Expand Down Expand Up @@ -86,9 +90,7 @@ class LoadFastEnough4Pwa extends Audit {
let explanation;
if (!score) {
displayValue = str_(displayValueTemplate, {timeInMs: tti.timing});
explanation = 'Your page loads too slowly and is not interactive within 10 seconds. ' +
'Look at the opportunities and diagnostics in the "Performance" section to learn how to ' +
'improve.';
explanation = str_(UIStrings.explanationLoadSlow);
}

return {
Expand Down
18 changes: 15 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,17 @@
'use strict';

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

const UIStrings = {
/** Title of a Lighthouse audit that prompts the user to manually check that their site works across different web browsers. */
title: 'Site works cross-browser',
/** Description of a Lighthouse audit that tells the user why they should make sites work across different browsers. 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: '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 +30,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;

17 changes: 14 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,17 @@
'use strict';

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

const UIStrings = {
/** Title of a Lighthouse audit that prompts the user to manually check that each page on their website uses a unique URL. */
title: 'Each page has a URL',
/** Description of a Lighthouse audit that tells the user why they should use unique URLs for each web page. 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: 'Ensure individual pages are deep linkable via URL 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).',
};

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

/**
* @fileoverview Manual PWA audit to ensure every page has a deep link.
Expand All @@ -18,11 +29,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;
17 changes: 14 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,17 @@
'use strict';

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

const UIStrings = {
/** Title of a Lighthouse audit that prompts the user to manually check that page transitions (navigating to other pages on a website) shouldn't feel like they are waiting for the network to load. */
title: 'Page transitions don\'t feel like they block on the network',
/** Description of a Lighthouse audit that tells the user why they should make transitions in their web app feel fast. 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: '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 +29,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(#7238): make this i18n-able.
Copy link
Member

Choose a reason for hiding this comment

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

👍

explanation: `Failures: ${result.failures.join(',\n')}.`,
details,
};
Expand Down
29 changes: 24 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,25 @@
'use strict';

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

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on the a website's offline capabilities. "200" refers to the HTTP status code when a site responds successfully. This descriptive title is shown to users when the test page responds successfully while offline. */
title: '`start_url` responds with a 200 when offline',
/** Title of a Lighthouse audit that provides detail on the a website's offline capabilities. "200" refers to the HTTP status code when a site responds successfully. This descriptive title is shown to users when the test page does not respond successfully while offline. */
failureTitle: '`start_url` does not respond with a 200 when offline',
/** Description of a Lighthouse audit that tells the user why a website should respond to requests when offline. 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: '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).',
/**
* @description Warning that the audit couldn't find the start_url and used the page's URL instead.
* @example {No Manifest Fetched.} manifestWarning
* */
warningCantStart: 'Lighthouse 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}\'.',
};

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

class OfflineStartUrl extends Audit {
/**
Expand All @@ -14,9 +33,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 +51,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 +65,4 @@ class OfflineStartUrl extends Audit {
}

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

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

const UIStrings = {
/** Title of a Lighthouse audit that provides detail on HTTP to HTTPS redirects. This descriptive title is shown to users when HTTP traffic is redirected to HTTPS. */
title: 'Redirects HTTP traffic to HTTPS',
/** Title of a Lighthouse audit that provides detail on HTTP to HTTPS redirects. This descriptive title is shown to users when HTTP traffic is not redirected to HTTPS. */
failureTitle: 'Does not redirect HTTP traffic to HTTPS',
/** Description of a Lighthouse audit that tells the user why they should direct HTTP traffic to HTTPS. 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: 'If you\'ve already set up HTTPS, make sure that you redirect all HTTP ' +
'traffic to HTTPS in order enable secure web features for all your users. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-redirects-to-https).',
Copy link
Member

Choose a reason for hiding this comment

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

was this edit from the original from an earlier discussion? It seems like a less universal thing to use as the incentive vs something like "If you\'ve already set up HTTPS, make sure that you redirect all HTTP traffic to HTTPS in order to keep acess to your site secure" or whatever.

(If you want to keep this version, it should at least be "in order to enable secure web features...")

Copy link
Collaborator

Choose a reason for hiding this comment

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

I seem to remember saying something like this. It seems to me that in the context of PWA the primary reason you care about this is for using a service worker for all your visitors. That's a very concrete and specific carrot compared to "make it secure so that it will be secure" but 🤷‍♂

Copy link
Member

Choose a reason for hiding this comment

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

I seem to remember saying something like this. It seems to me that in the context of PWA the primary reason you care about this is for using a service worker for all your visitors. That's a very concrete and specific carrot compared to "make it secure so that it will be secure" but 🤷‍♂

well the most important reason to make it secure is so it will be secure :D but very good point about the context and the whole point is to be checking something a dev might forgot to check that will undermine other things they're trying to do. So change SGTM, but still needs the typo fix

Suggested change
'traffic to HTTPS in order enable secure web features for all your users. [Learn more](https://developers.google.com/web/tools/lighthouse/audits/http-redirects-to-https).',
'traffic to HTTPS in order to enable secure web features for all your users. [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 +27,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 +46,4 @@ class RedirectsHTTP extends Audit {
}

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