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

report(i18n): use LHR locale for toLocaleString #5734

Merged
merged 1 commit into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions lighthouse-core/report/html/renderer/report-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@ class ReportRenderer {
// If any mutations happen to the report within the renderers, we want the original object untouched
const clone = /** @type {LH.ReportResult} */ (JSON.parse(JSON.stringify(report)));
// Mutate the UIStrings if necessary (while saving originals)
const clonedStrings = JSON.parse(JSON.stringify(Util.UIStrings));
const originalUIStrings = JSON.parse(JSON.stringify(Util.UIStrings));
// If LHR is older (≤3.0.3), it has no locale setting. Set default.
if (!clone.configSettings.locale) {
clone.configSettings.locale = 'en-US';
}
Util.setNumberDateLocale(clone.configSettings.locale);
if (clone.i18n && clone.i18n.rendererFormattedStrings) {
ReportRenderer.updateAllUIStrings(clone.i18n.rendererFormattedStrings);
}
Expand All @@ -61,7 +66,7 @@ class ReportRenderer {
container.appendChild(this._renderReport(clone));

// put the UIStrings back into original state
ReportRenderer.updateAllUIStrings(clonedStrings);
ReportRenderer.updateAllUIStrings(originalUIStrings);
Copy link
Member

Choose a reason for hiding this comment

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

updated because it was a little confusing with clone being the cloned LHR, while clonedStrings was the clone of the original Util.UIStrings, not the translated strings found in clone :)


return /** @type {Element} **/ (container);
}
Expand Down
31 changes: 25 additions & 6 deletions lighthouse-core/report/html/renderer/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ class Util {
*/
static formatNumber(number, granularity = 0.1) {
const coarseValue = Math.round(number / granularity) * granularity;
return coarseValue.toLocaleString();
return coarseValue.toLocaleString(Util.numberDateLocale);
}

/**
Expand All @@ -146,7 +146,8 @@ class Util {
* @return {string}
*/
static formatBytesToKB(size, granularity = 0.1) {
const kbs = (Math.round(size / 1024 / granularity) * granularity).toLocaleString();
const kbs = (Math.round(size / 1024 / granularity) * granularity)
.toLocaleString(Util.numberDateLocale);
return `${kbs}${NBSP}KB`;
}

Expand All @@ -157,7 +158,7 @@ class Util {
*/
static formatMilliseconds(ms, granularity = 10) {
const coarseTime = Math.round(ms / granularity) * granularity;
return `${coarseTime.toLocaleString()}${NBSP}ms`;
return `${coarseTime.toLocaleString(Util.numberDateLocale)}${NBSP}ms`;
}

/**
Expand All @@ -167,7 +168,7 @@ class Util {
*/
static formatSeconds(ms, granularity = 0.1) {
const coarseTime = Math.round(ms / 1000 / granularity) * granularity;
return `${coarseTime.toLocaleString()}${NBSP}s`;
return `${coarseTime.toLocaleString(Util.numberDateLocale)}${NBSP}s`;
}

/**
Expand All @@ -180,14 +181,14 @@ class Util {
month: 'short', day: 'numeric', year: 'numeric',
hour: 'numeric', minute: 'numeric', timeZoneName: 'short',
};
let formatter = new Intl.DateTimeFormat('en-US', options);
let formatter = new Intl.DateTimeFormat(Util.numberDateLocale, options);

// Force UTC if runtime timezone could not be detected.
// See https://github.com/GoogleChrome/lighthouse/issues/1056
const tz = formatter.resolvedOptions().timeZone;
if (!tz || tz.toLowerCase() === 'etc/unknown') {
options.timeZone = 'UTC';
formatter = new Intl.DateTimeFormat('en-US', options);
formatter = new Intl.DateTimeFormat(Util.numberDateLocale, options);
}
return formatter.format(new Date(date));
}
Expand Down Expand Up @@ -387,8 +388,26 @@ class Util {
summary: `${deviceEmulation}, ${summary}`,
};
}

/**
* Set the locale to be used for Util's number and date formatting functions.
* @param {LH.Locale} locale
*/
static setNumberDateLocale(locale) {
Util.numberDateLocale = locale;

// When testing, use a locale with more exciting numeric formatting
// @ts-ignore - TODO: until `de-DE` is in LH.Locale
if (Util.numberDateLocale === 'en-XA') Util.numberDateLocale = 'de-DE';
}
}

/**
* This value is updated on each run to the locale of the report
* @type {LH.Locale}
*/
Util.numberDateLocale = 'en-US';

Util.UIStrings = {
/** Disclaimer shown to users below the metric values (First Contentful Paint, Time to Interactive, etc) to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. */
varianceDisclaimer: 'Values are estimated and may vary.',
Expand Down
21 changes: 21 additions & 0 deletions lighthouse-core/test/report/html/renderer/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,27 @@ describe('util helpers', () => {
assert.equal(Util.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}d 4${NBSP}h 5${NBSP}s`);
});

// TODO: need ICU support in node on Travis/Appveyor
it.skip('formats based on locale', () => {
const number = 12346.858558;

const originalLocale = Util.numberDateLocale;
Util.setNumberDateLocale('de-DE');
assert.strictEqual(Util.formatNumber(number), '12.346,9');
Util.setNumberDateLocale(originalLocale); // reset
assert.strictEqual(Util.formatNumber(number), '12,346.9');
});

it.skip('uses decimal comma with en-XA test locale', () => {
const number = 12346.858558;

const originalLocale = Util.numberDateLocale;
Util.setNumberDateLocale('en-XA');
assert.strictEqual(Util.formatNumber(number), '12.346,9');
Util.setNumberDateLocale(originalLocale); // reset
assert.strictEqual(Util.formatNumber(number), '12,346.9');
});

it('calculates a score ratings', () => {
assert.equal(Util.calculateRating(0.0), 'fail');
assert.equal(Util.calculateRating(0.10), 'fail');
Expand Down