From 504e62aa399a65faab7275a25b695ef6e1728fe5 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 6 Apr 2022 15:15:49 -0700 Subject: [PATCH 01/13] first pass, localize units --- report/renderer/i18n.js | 94 +++++++++++++++++-------------- report/test/renderer/i18n-test.js | 8 +-- 2 files changed, 55 insertions(+), 47 deletions(-) diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index 0e67d4fd7965..20d832fa254f 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -22,9 +22,7 @@ export class I18n { // When testing, use a locale with more exciting numeric formatting. if (locale === 'en-XA') locale = 'de'; - this._numberDateLocale = locale; - this._numberFormatter = new Intl.NumberFormat(locale); - this._percentFormatter = new Intl.NumberFormat(locale, {style: 'percent'}); + this._locale = locale; this._strings = strings; } @@ -32,6 +30,31 @@ export class I18n { return this._strings; } + /** + * @param {number} number + * @param {number} granularity + * @param {Intl.NumberFormatOptions} opts + * @return {string} + */ + _formatNumberWithGranularity(number, granularity, opts = {}) { + opts = {...opts}; + const log10 = -Math.log10(granularity); + if (!Number.isFinite(log10) || (granularity > 1 && Math.floor(log10) !== log10)) { + throw new Error(`granularity of ${granularity} is invalid`); + } + + if (granularity < 1) { + opts.minimumFractionDigits = opts.maximumFractionDigits = Math.ceil(log10); + } + + number = Math.round(number / granularity) * granularity; + + // Avoid displaying a negative value that rounds to zero as "0". + if (Object.is(number, -0)) number = 0; + + return new Intl.NumberFormat(this._locale, opts).format(number).replace(' ', NBSP2); + } + /** * Format number. * @param {number} number @@ -39,8 +62,7 @@ export class I18n { * @return {string} */ formatNumber(number, granularity = 0.1) { - const coarseValue = Math.round(number / granularity) * granularity; - return this._numberFormatter.format(coarseValue); + return this._formatNumberWithGranularity(number, granularity); } /** @@ -49,7 +71,7 @@ export class I18n { * @return {string} */ formatPercent(number) { - return this._percentFormatter.format(number); + return new Intl.NumberFormat(this._locale, {style: 'percent'}).format(number); } /** @@ -58,9 +80,7 @@ export class I18n { * @return {string} */ formatBytesToKiB(size, granularity = 0.1) { - const formatter = this._byteFormatterForGranularity(granularity); - const kbs = formatter.format(Math.round(size / 1024 / granularity) * granularity); - return `${kbs}${NBSP2}KiB`; + return this._formatNumberWithGranularity(size / KiB, granularity) + `${NBSP2}KiB`; } /** @@ -69,9 +89,7 @@ export class I18n { * @return {string} */ formatBytesToMiB(size, granularity = 0.1) { - const formatter = this._byteFormatterForGranularity(granularity); - const kbs = formatter.format(Math.round(size / (1024 ** 2) / granularity) * granularity); - return `${kbs}${NBSP2}MiB`; + return this._formatNumberWithGranularity(size / MiB, granularity) + `${NBSP2}MiB`; } /** @@ -80,9 +98,11 @@ export class I18n { * @return {string} */ formatBytes(size, granularity = 1) { - const formatter = this._byteFormatterForGranularity(granularity); - const kbs = formatter.format(Math.round(size / granularity) * granularity); - return `${kbs}${NBSP2}bytes`; + return this._formatNumberWithGranularity(size, granularity, { + style: 'unit', + unit: 'byte', + unitDisplay: 'long', + }); } /** @@ -93,26 +113,7 @@ export class I18n { formatBytesWithBestUnit(size, granularity = 0.1) { if (size >= MiB) return this.formatBytesToMiB(size, granularity); if (size >= KiB) return this.formatBytesToKiB(size, granularity); - return this.formatNumber(size, granularity) + '\xa0B'; - } - - /** - * Format bytes with a constant number of fractional digits, i.e. for a granularity of 0.1, 10 becomes '10.0' - * @param {number} granularity Controls how coarse the displayed value is - * @return {Intl.NumberFormat} - */ - _byteFormatterForGranularity(granularity) { - // assume any granularity above 1 will not contain fractional parts, i.e. will never be 1.5 - let numberOfFractionDigits = 0; - if (granularity < 1) { - numberOfFractionDigits = -Math.floor(Math.log10(granularity)); - } - - return new Intl.NumberFormat(this._numberDateLocale, { - ...this._numberFormatter.resolvedOptions(), - maximumFractionDigits: numberOfFractionDigits, - minimumFractionDigits: numberOfFractionDigits, - }); + return this.formatBytes(size, granularity); } /** @@ -121,10 +122,11 @@ export class I18n { * @return {string} */ formatMilliseconds(ms, granularity = 10) { - const coarseTime = Math.round(ms / granularity) * granularity; - return coarseTime === 0 - ? `${this._numberFormatter.format(0)}${NBSP2}ms` - : `${this._numberFormatter.format(coarseTime)}${NBSP2}ms`; + return this._formatNumberWithGranularity(ms, granularity, { + style: 'unit', + unit: 'millisecond', + unitDisplay: 'short', + }); } /** @@ -133,8 +135,11 @@ export class I18n { * @return {string} */ formatSeconds(ms, granularity = 0.1) { - const coarseTime = Math.round(ms / 1000 / granularity) * granularity; - return `${this._numberFormatter.format(coarseTime)}${NBSP2}s`; + return this._formatNumberWithGranularity(ms / 1000, granularity, { + style: 'unit', + unit: 'second', + unitDisplay: 'short', + }); } /** @@ -154,10 +159,10 @@ export class I18n { // and https://github.com/GoogleChrome/lighthouse/pull/9822 let formatter; try { - formatter = new Intl.DateTimeFormat(this._numberDateLocale, options); + formatter = new Intl.DateTimeFormat(this._locale, options); } catch (err) { options.timeZone = 'UTC'; - formatter = new Intl.DateTimeFormat(this._numberDateLocale, options); + formatter = new Intl.DateTimeFormat(this._locale, options); } return formatter.format(new Date(date)); @@ -169,6 +174,9 @@ export class I18n { * @return {string} */ formatDuration(timeInMilliseconds) { + // There is a proposal for a Intl.DurationFormat. + // https://github.com/tc39/proposal-intl-duration-format + let timeInSeconds = timeInMilliseconds / 1000; if (Math.round(timeInSeconds) === 0) { return 'None'; diff --git a/report/test/renderer/i18n-test.js b/report/test/renderer/i18n-test.js index e5aff96c6eff..43d2bc150688 100644 --- a/report/test/renderer/i18n-test.js +++ b/report/test/renderer/i18n-test.js @@ -22,8 +22,8 @@ const NBSP = '\xa0'; describe('util helpers', () => { it('formats a number', () => { const i18n = new I18n('en', {...Util.UIStrings}); - assert.strictEqual(i18n.formatNumber(10), '10'); - assert.strictEqual(i18n.formatNumber(100.01), '100'); + assert.strictEqual(i18n.formatNumber(10), '10.0'); + assert.strictEqual(i18n.formatNumber(100.01), '100.0'); assert.strictEqual(i18n.formatNumber(13000.456), '13,000.5'); }); @@ -114,7 +114,7 @@ describe('util helpers', () => { assert.strictEqual(i18n.formatNumber(number), '12.346,9'); assert.strictEqual(i18n.formatBytesToKiB(number), `12,1${NBSP}KiB`); assert.strictEqual(i18n.formatMilliseconds(number), `12.350${NBSP}ms`); - assert.strictEqual(i18n.formatSeconds(number), `12,3${NBSP}s`); + assert.strictEqual(i18n.formatSeconds(number), `12,3${NBSP}Sek.`); }); it('uses decimal comma with en-XA test locale', () => { @@ -125,7 +125,7 @@ describe('util helpers', () => { assert.strictEqual(i18n.formatNumber(number), '12.346,9'); assert.strictEqual(i18n.formatBytesToKiB(number), `12,1${NBSP}KiB`); assert.strictEqual(i18n.formatMilliseconds(number), `12.350${NBSP}ms`); - assert.strictEqual(i18n.formatSeconds(number), `12,3${NBSP}s`); + assert.strictEqual(i18n.formatSeconds(number), `12,3${NBSP}Sek.`); }); it('should not crash on unknown locales', () => { From 0b8814274bc2f10dfee6b8d21a6cc9515fce997e Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 6 Apr 2022 15:44:48 -0700 Subject: [PATCH 02/13] localize formatDuration --- report/renderer/i18n.js | 35 ++++++++++++++++++++++--------- report/test/renderer/i18n-test.js | 15 +++++++++++++ 2 files changed, 40 insertions(+), 10 deletions(-) diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index 20d832fa254f..2d3ae6075bfe 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -176,6 +176,7 @@ export class I18n { formatDuration(timeInMilliseconds) { // There is a proposal for a Intl.DurationFormat. // https://github.com/tc39/proposal-intl-duration-format + // Until then, we do things a bit more manually. let timeInSeconds = timeInMilliseconds / 1000; if (Math.round(timeInSeconds) === 0) { @@ -185,19 +186,33 @@ export class I18n { /** @type {Array} */ const parts = []; /** @type {Record} */ - const unitLabels = { - d: 60 * 60 * 24, - h: 60 * 60, - m: 60, - s: 1, + const unitToSecondsPer = { + day: 60 * 60 * 24, + hour: 60 * 60, + minute: 60, + second: 1, + }; + /** @type {Record} */ + const unitToDefaultLabel = { + day: 'd', + hour: 'h', + minute: 'm', + second: 's', }; - Object.keys(unitLabels).forEach(label => { - const unit = unitLabels[label]; - const numberOfUnits = Math.floor(timeInSeconds / unit); + Object.keys(unitToSecondsPer).forEach(unit => { + const unitFormatter = new Intl.NumberFormat(this._locale, { + style: 'unit', + unit, + unitDisplay: 'narrow', + }); + const label = unitFormatter.formatToParts(0) + .find(p => p.type === 'unit')?.value || unitToDefaultLabel[unit]; + const secondsPerUnit = unitToSecondsPer[unit]; + const numberOfUnits = Math.floor(timeInSeconds / secondsPerUnit); if (numberOfUnits > 0) { - timeInSeconds -= numberOfUnits * unit; - parts.push(`${numberOfUnits}\xa0${label}`); + timeInSeconds -= numberOfUnits * secondsPerUnit; + parts.push(`${numberOfUnits}${NBSP2}${label}`); } }); diff --git a/report/test/renderer/i18n-test.js b/report/test/renderer/i18n-test.js index 43d2bc150688..165c405fc139 100644 --- a/report/test/renderer/i18n-test.js +++ b/report/test/renderer/i18n-test.js @@ -106,6 +106,21 @@ describe('util helpers', () => { assert.equal(i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}d 4${NBSP}h 5${NBSP}s`); }); + it('formats a duration based on locale', () => { + let i18n = new I18n('de', {...Util.UIStrings}); + assert.equal(i18n.formatDuration(60 * 1000), `1${NBSP}Min.`); + assert.equal(i18n.formatDuration(60 * 60 * 1000 + 5000), `1${NBSP}Std. 5${NBSP}Sek.`); + assert.equal( + i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}T 4${NBSP}Std. 5${NBSP}Sek.`); + + // idk? + i18n = new I18n('ar', {...Util.UIStrings}); + // assert.equal(i18n.formatDuration(60 * 1000), `1${NBSP}د`); + // assert.equal(i18n.formatDuration(60 * 60 * 1000 + 5000), `1${NBSP}س 5${NBSP}ث`); + // assert.equal( + // i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}T 4${NBSP}Std. 5${NBSP}Sek.`); + }); + it('formats numbers based on locale', () => { // Requires full-icu or Intl polyfill. const number = 12346.858558; From 7dd7d014fa0f182f6b13b07a081f1e7255f5b9c5 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 6 Apr 2022 16:31:46 -0700 Subject: [PATCH 03/13] hmm --- flow-report/src/summary/category.tsx | 2 +- report/renderer/i18n.js | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index 198971526324..ef57f79ff712 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -132,7 +132,7 @@ const SummaryTooltip: FunctionComponent<{ { !displayAsFraction && category.score !== null && <> · - {i18n.formatNumber(category.score * 100)} + {i18n.formatNumber(category.score * 100, 1)} } diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index 2d3ae6075bfe..7ea5be5951ab 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -113,7 +113,11 @@ export class I18n { formatBytesWithBestUnit(size, granularity = 0.1) { if (size >= MiB) return this.formatBytesToMiB(size, granularity); if (size >= KiB) return this.formatBytesToKiB(size, granularity); - return this.formatBytes(size, granularity); + return this._formatNumberWithGranularity(size, granularity, { + style: 'unit', + unit: 'byte', + unitDisplay: 'narrow', + }); } /** From 08bc98b730f1a77f22b714a2831b49844f143492 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 6 Apr 2022 17:14:52 -0700 Subject: [PATCH 04/13] update --- flow-report/src/summary/category.tsx | 2 +- lighthouse-core/audits/bootup-time.js | 1 + lighthouse-core/util-commonjs.js | 10 +++++----- report/renderer/i18n.js | 11 +++++++++++ report/renderer/util.js | 10 +++++----- report/test/renderer/i18n-test.js | 3 +++ report/test/renderer/util-test.js | 2 +- treemap/app/src/util.js | 4 ++-- 8 files changed, 29 insertions(+), 14 deletions(-) diff --git a/flow-report/src/summary/category.tsx b/flow-report/src/summary/category.tsx index ef57f79ff712..363d4f966bd7 100644 --- a/flow-report/src/summary/category.tsx +++ b/flow-report/src/summary/category.tsx @@ -132,7 +132,7 @@ const SummaryTooltip: FunctionComponent<{ { !displayAsFraction && category.score !== null && <> · - {i18n.formatNumber(category.score * 100, 1)} + {i18n.formatInteger(category.score * 100)} } diff --git a/lighthouse-core/audits/bootup-time.js b/lighthouse-core/audits/bootup-time.js index af99e0cbd9b9..af1fb67c61a2 100644 --- a/lighthouse-core/audits/bootup-time.js +++ b/lighthouse-core/audits/bootup-time.js @@ -204,6 +204,7 @@ class BootupTime extends Audit { totalBootupTime ); + return { score, numericValue: totalBootupTime, diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index 3990a2091565..ae6a2238dbae 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -434,9 +434,9 @@ class Util { case 'devtools': { const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`; - networkThrottling = `${Util.i18n.formatNumber(requestLatencyMs)}${NBSP}ms HTTP RTT, ` + - `${Util.i18n.formatNumber(throttling.downloadThroughputKbps)}${NBSP}Kbps down, ` + - `${Util.i18n.formatNumber(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`; + networkThrottling = `${Util.i18n.formatMilliseconds(requestLatencyMs, 1)} HTTP RTT, ` + + `${Util.i18n.formatInteger(throttling.downloadThroughputKbps)}${NBSP}Kbps down, ` + + `${Util.i18n.formatInteger(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`; const isSlow4G = () => { return requestLatencyMs === 150 * 3.75 && @@ -449,8 +449,8 @@ class Util { case 'simulate': { const {cpuSlowdownMultiplier, rttMs, throughputKbps} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (Simulated)`; - networkThrottling = `${Util.i18n.formatNumber(rttMs)}${NBSP}ms TCP RTT, ` + - `${Util.i18n.formatNumber(throughputKbps)}${NBSP}Kbps throughput (Simulated)`; + networkThrottling = `${Util.i18n.formatMilliseconds(rttMs)} TCP RTT, ` + + `${Util.i18n.formatInteger(throughputKbps)}${NBSP}Kbps throughput (Simulated)`; const isSlow4G = () => { return rttMs === 150 && throughputKbps === 1.6 * 1024; diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index 7ea5be5951ab..a91a973b5710 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -65,6 +65,17 @@ export class I18n { return this._formatNumberWithGranularity(number, granularity); } + /** + * Format integer. + * Just like {@link formatNumber} but uses a granularity of 1, rounding to the nearest + * whole number. + * @param {number} number + * @return {string} + */ + formatInteger(number) { + return this._formatNumberWithGranularity(number, 1); + } + /** * Format percent. * @param {number} number 0–1 diff --git a/report/renderer/util.js b/report/renderer/util.js index b5723ecc8094..a45adec367a4 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -431,9 +431,9 @@ class Util { case 'devtools': { const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`; - networkThrottling = `${Util.i18n.formatNumber(requestLatencyMs)}${NBSP}ms HTTP RTT, ` + - `${Util.i18n.formatNumber(throttling.downloadThroughputKbps)}${NBSP}Kbps down, ` + - `${Util.i18n.formatNumber(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`; + networkThrottling = `${Util.i18n.formatMilliseconds(requestLatencyMs, 1)} HTTP RTT, ` + + `${Util.i18n.formatInteger(throttling.downloadThroughputKbps)}${NBSP}Kbps down, ` + + `${Util.i18n.formatInteger(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`; const isSlow4G = () => { return requestLatencyMs === 150 * 3.75 && @@ -446,8 +446,8 @@ class Util { case 'simulate': { const {cpuSlowdownMultiplier, rttMs, throughputKbps} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (Simulated)`; - networkThrottling = `${Util.i18n.formatNumber(rttMs)}${NBSP}ms TCP RTT, ` + - `${Util.i18n.formatNumber(throughputKbps)}${NBSP}Kbps throughput (Simulated)`; + networkThrottling = `${Util.i18n.formatMilliseconds(rttMs)} TCP RTT, ` + + `${Util.i18n.formatInteger(throughputKbps)}${NBSP}Kbps throughput (Simulated)`; const isSlow4G = () => { return rttMs === 150 && throughputKbps === 1.6 * 1024; diff --git a/report/test/renderer/i18n-test.js b/report/test/renderer/i18n-test.js index 165c405fc139..3497f3b6595b 100644 --- a/report/test/renderer/i18n-test.js +++ b/report/test/renderer/i18n-test.js @@ -25,6 +25,9 @@ describe('util helpers', () => { assert.strictEqual(i18n.formatNumber(10), '10.0'); assert.strictEqual(i18n.formatNumber(100.01), '100.0'); assert.strictEqual(i18n.formatNumber(13000.456), '13,000.5'); + assert.strictEqual(i18n.formatInteger(10), '10'); + assert.strictEqual(i18n.formatInteger(100.01), '100'); + assert.strictEqual(i18n.formatInteger(13000.6), '13,001'); }); it('formats a date', () => { diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index 058bf33cde58..b1996ff0078c 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -73,7 +73,7 @@ describe('util helpers', () => { // eslint-disable-next-line max-len assert.equal(descriptions.networkThrottling, '150\xa0ms TCP RTT, 1,600\xa0Kbps throughput (Simulated)'); - assert.equal(descriptions.cpuThrottling, '2x slowdown (Simulated)'); + assert.equal(descriptions.cpuThrottling, '2.0x slowdown (Simulated)'); }); describe('#prepareReportResult', () => { diff --git a/treemap/app/src/util.js b/treemap/app/src/util.js index 787d29a19c61..6abe713b1828 100644 --- a/treemap/app/src/util.js +++ b/treemap/app/src/util.js @@ -147,8 +147,8 @@ class TreemapUtil { * @param {string} unit */ static format(value, unit) { - if (unit === 'bytes') return this.i18n.formatBytes(value); - if (unit === 'time') return `${this.i18n.formatNumber(value)}\xa0ms`; + if (unit === 'byte') return this.i18n.formatBytes(value); + if (unit === 'ms') return this.i18n.formatMilliseconds(value); return `${this.i18n.formatNumber(value)}\xa0${unit}`; } From b4a8e238ccd6593fc3c6f7ba7f60174dd6533e84 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Apr 2022 16:05:58 -0700 Subject: [PATCH 05/13] Update lighthouse-core/audits/bootup-time.js Co-authored-by: Adam Raine <6752989+adamraine@users.noreply.github.com> --- lighthouse-core/audits/bootup-time.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lighthouse-core/audits/bootup-time.js b/lighthouse-core/audits/bootup-time.js index af1fb67c61a2..af99e0cbd9b9 100644 --- a/lighthouse-core/audits/bootup-time.js +++ b/lighthouse-core/audits/bootup-time.js @@ -204,7 +204,6 @@ class BootupTime extends Audit { totalBootupTime ); - return { score, numericValue: totalBootupTime, From 61c23f5fe67850087b96dd522fdea4bd20b0919c Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Apr 2022 16:45:04 -0700 Subject: [PATCH 06/13] pr --- report/renderer/i18n.js | 21 ++++++--------------- report/test/renderer/i18n-test.js | 17 +++++++++-------- 2 files changed, 15 insertions(+), 23 deletions(-) diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index a91a973b5710..f94b6a67acfa 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -207,27 +207,18 @@ export class I18n { minute: 60, second: 1, }; - /** @type {Record} */ - const unitToDefaultLabel = { - day: 'd', - hour: 'h', - minute: 'm', - second: 's', - }; Object.keys(unitToSecondsPer).forEach(unit => { - const unitFormatter = new Intl.NumberFormat(this._locale, { - style: 'unit', - unit, - unitDisplay: 'narrow', - }); - const label = unitFormatter.formatToParts(0) - .find(p => p.type === 'unit')?.value || unitToDefaultLabel[unit]; const secondsPerUnit = unitToSecondsPer[unit]; const numberOfUnits = Math.floor(timeInSeconds / secondsPerUnit); if (numberOfUnits > 0) { timeInSeconds -= numberOfUnits * secondsPerUnit; - parts.push(`${numberOfUnits}${NBSP2}${label}`); + const part = this._formatNumberWithGranularity(numberOfUnits, 1, { + style: 'unit', + unit, + unitDisplay: 'narrow', + }); + parts.push(part); } }); diff --git a/report/test/renderer/i18n-test.js b/report/test/renderer/i18n-test.js index 3497f3b6595b..aa5ad78523ad 100644 --- a/report/test/renderer/i18n-test.js +++ b/report/test/renderer/i18n-test.js @@ -104,9 +104,9 @@ describe('util helpers', () => { it('formats a duration', () => { const i18n = new I18n('en', {...Util.UIStrings}); - assert.equal(i18n.formatDuration(60 * 1000), `1${NBSP}m`); - assert.equal(i18n.formatDuration(60 * 60 * 1000 + 5000), `1${NBSP}h 5${NBSP}s`); - assert.equal(i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}d 4${NBSP}h 5${NBSP}s`); + assert.equal(i18n.formatDuration(60 * 1000), '1m'); + assert.equal(i18n.formatDuration(60 * 60 * 1000 + 5000), '1h 5s'); + assert.equal(i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), '1d 4h 5s'); }); it('formats a duration based on locale', () => { @@ -116,12 +116,13 @@ describe('util helpers', () => { assert.equal( i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}T 4${NBSP}Std. 5${NBSP}Sek.`); - // idk? + // Yes, this is actually backwards (s h d). i18n = new I18n('ar', {...Util.UIStrings}); - // assert.equal(i18n.formatDuration(60 * 1000), `1${NBSP}د`); - // assert.equal(i18n.formatDuration(60 * 60 * 1000 + 5000), `1${NBSP}س 5${NBSP}ث`); - // assert.equal( - // i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `1${NBSP}T 4${NBSP}Std. 5${NBSP}Sek.`); + /* eslint-disable no-irregular-whitespace */ + assert.equal(i18n.formatDuration(60 * 1000), `١${NBSP}د`); + assert.equal(i18n.formatDuration(60 * 60 * 1000 + 5000), `١${NBSP}س ٥${NBSP}ث`); + assert.equal(i18n.formatDuration(28 * 60 * 60 * 1000 + 5000), `١ ي ٤ س ٥ ث`); + /* eslint-enable no-irregular-whitespace */ }); it('formats numbers based on locale', () => { From 6232d9376e662e58aa0cd74ef1a6eb761b18a463 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 7 Apr 2022 16:50:46 -0700 Subject: [PATCH 07/13] kb/s --- lighthouse-core/util-commonjs.js | 6 +++--- report/renderer/i18n.js | 13 +++++++++++++ report/renderer/util.js | 6 +++--- report/test/renderer/util-test.js | 4 ++-- 4 files changed, 21 insertions(+), 8 deletions(-) diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index ae6a2238dbae..a8eeb0f57caf 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -435,8 +435,8 @@ class Util { const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`; networkThrottling = `${Util.i18n.formatMilliseconds(requestLatencyMs, 1)} HTTP RTT, ` + - `${Util.i18n.formatInteger(throttling.downloadThroughputKbps)}${NBSP}Kbps down, ` + - `${Util.i18n.formatInteger(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`; + `${Util.i18n.formatKbps(throttling.downloadThroughputKbps)} down, ` + + `${Util.i18n.formatKbps(throttling.uploadThroughputKbps)} up (DevTools)`; const isSlow4G = () => { return requestLatencyMs === 150 * 3.75 && @@ -450,7 +450,7 @@ class Util { const {cpuSlowdownMultiplier, rttMs, throughputKbps} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (Simulated)`; networkThrottling = `${Util.i18n.formatMilliseconds(rttMs)} TCP RTT, ` + - `${Util.i18n.formatInteger(throughputKbps)}${NBSP}Kbps throughput (Simulated)`; + `${Util.i18n.formatKbps(throughputKbps)} throughput (Simulated)`; const isSlow4G = () => { return rttMs === 150 && throughputKbps === 1.6 * 1024; diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index f94b6a67acfa..fcae90ca2efb 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -131,6 +131,19 @@ export class I18n { }); } + /** + * @param {number} size + * @param {number=} granularity Controls how coarse the displayed value is, defaults to 1 + * @return {string} + */ + formatKbps(size, granularity = 1) { + return this._formatNumberWithGranularity(size, granularity, { + style: 'unit', + unit: 'kilobit-per-second', + unitDisplay: 'short', + }); + } + /** * @param {number} ms * @param {number=} granularity Controls how coarse the displayed value is, defaults to 10 diff --git a/report/renderer/util.js b/report/renderer/util.js index a45adec367a4..0765f28fabaf 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -432,8 +432,8 @@ class Util { const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`; networkThrottling = `${Util.i18n.formatMilliseconds(requestLatencyMs, 1)} HTTP RTT, ` + - `${Util.i18n.formatInteger(throttling.downloadThroughputKbps)}${NBSP}Kbps down, ` + - `${Util.i18n.formatInteger(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`; + `${Util.i18n.formatKbps(throttling.downloadThroughputKbps)} down, ` + + `${Util.i18n.formatKbps(throttling.uploadThroughputKbps)} up (DevTools)`; const isSlow4G = () => { return requestLatencyMs === 150 * 3.75 && @@ -447,7 +447,7 @@ class Util { const {cpuSlowdownMultiplier, rttMs, throughputKbps} = throttling; cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (Simulated)`; networkThrottling = `${Util.i18n.formatMilliseconds(rttMs)} TCP RTT, ` + - `${Util.i18n.formatInteger(throughputKbps)}${NBSP}Kbps throughput (Simulated)`; + `${Util.i18n.formatKbps(throughputKbps)} throughput (Simulated)`; const isSlow4G = () => { return rttMs === 150 && throughputKbps === 1.6 * 1024; diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index b1996ff0078c..792dd72b4490 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -57,7 +57,7 @@ describe('util helpers', () => { }); // eslint-disable-next-line max-len - assert.equal(descriptions.networkThrottling, '565\xa0ms HTTP RTT, 1,400\xa0Kbps down, 600\xa0Kbps up (DevTools)'); + assert.equal(descriptions.networkThrottling, '565\xa0ms HTTP RTT, 1,400\xa0kb/s down, 600\xa0kb/s up (DevTools)'); assert.equal(descriptions.cpuThrottling, '4.5x slowdown (DevTools)'); }); @@ -72,7 +72,7 @@ describe('util helpers', () => { }); // eslint-disable-next-line max-len - assert.equal(descriptions.networkThrottling, '150\xa0ms TCP RTT, 1,600\xa0Kbps throughput (Simulated)'); + assert.equal(descriptions.networkThrottling, '150\xa0ms TCP RTT, 1,600\xa0kb/s throughput (Simulated)'); assert.equal(descriptions.cpuThrottling, '2.0x slowdown (Simulated)'); }); From 9e82306ae44aeddbd43d1756732d2bce653982b5 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 8 Apr 2022 12:17:46 -0700 Subject: [PATCH 08/13] update --- report/renderer/util.js | 10 ++++++++-- report/test/renderer/util-test.js | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/report/renderer/util.js b/report/renderer/util.js index 0765f28fabaf..85616e675aca 100644 --- a/report/renderer/util.js +++ b/report/renderer/util.js @@ -430,7 +430,10 @@ class Util { break; case 'devtools': { const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; - cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`; + // TODO: better api in i18n formatter such that this isn't needed. + const cpuGranularity = Number.isInteger(cpuSlowdownMultiplier) ? 1 : 0.1; + // eslint-disable-next-line max-len + cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier, cpuGranularity)}x slowdown (DevTools)`; networkThrottling = `${Util.i18n.formatMilliseconds(requestLatencyMs, 1)} HTTP RTT, ` + `${Util.i18n.formatKbps(throttling.downloadThroughputKbps)} down, ` + `${Util.i18n.formatKbps(throttling.uploadThroughputKbps)} up (DevTools)`; @@ -445,7 +448,10 @@ class Util { } case 'simulate': { const {cpuSlowdownMultiplier, rttMs, throughputKbps} = throttling; - cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (Simulated)`; + // TODO: better api in i18n formatter such that this isn't needed. + const cpuGranularity = Number.isInteger(cpuSlowdownMultiplier) ? 1 : 0.1; + // eslint-disable-next-line max-len + cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier, cpuGranularity)}x slowdown (Simulated)`; networkThrottling = `${Util.i18n.formatMilliseconds(rttMs)} TCP RTT, ` + `${Util.i18n.formatKbps(throughputKbps)} throughput (Simulated)`; diff --git a/report/test/renderer/util-test.js b/report/test/renderer/util-test.js index 792dd72b4490..6fad0e0d5914 100644 --- a/report/test/renderer/util-test.js +++ b/report/test/renderer/util-test.js @@ -73,7 +73,7 @@ describe('util helpers', () => { // eslint-disable-next-line max-len assert.equal(descriptions.networkThrottling, '150\xa0ms TCP RTT, 1,600\xa0kb/s throughput (Simulated)'); - assert.equal(descriptions.cpuThrottling, '2.0x slowdown (Simulated)'); + assert.equal(descriptions.cpuThrottling, '2x slowdown (Simulated)'); }); describe('#prepareReportResult', () => { From 325d84bae933f5180e5385df232a22ea58fae9ad Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 8 Apr 2022 12:19:04 -0700 Subject: [PATCH 09/13] fix --- treemap/app/src/util.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/treemap/app/src/util.js b/treemap/app/src/util.js index 6abe713b1828..694549e08756 100644 --- a/treemap/app/src/util.js +++ b/treemap/app/src/util.js @@ -142,16 +142,6 @@ class TreemapUtil { return /** @type {ParseSelector} */ (result); } - /** - * @param {number} value - * @param {string} unit - */ - static format(value, unit) { - if (unit === 'byte') return this.i18n.formatBytes(value); - if (unit === 'ms') return this.i18n.formatMilliseconds(value); - return `${this.i18n.formatNumber(value)}\xa0${unit}`; - } - /** * Given a list of items, return a function (a hasher) that will map keys to an item. * When a key is seen for the first time, the item returned is cached and will always From 0d3cbc64287fdf2c0adfa843f39e7d5f87546e5f Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 8 Apr 2022 12:35:27 -0700 Subject: [PATCH 10/13] fix --- lighthouse-core/util-commonjs.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/util-commonjs.js b/lighthouse-core/util-commonjs.js index a8eeb0f57caf..b6132687b2ba 100644 --- a/lighthouse-core/util-commonjs.js +++ b/lighthouse-core/util-commonjs.js @@ -433,7 +433,10 @@ class Util { break; case 'devtools': { const {cpuSlowdownMultiplier, requestLatencyMs} = throttling; - cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`; + // TODO: better api in i18n formatter such that this isn't needed. + const cpuGranularity = Number.isInteger(cpuSlowdownMultiplier) ? 1 : 0.1; + // eslint-disable-next-line max-len + cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier, cpuGranularity)}x slowdown (DevTools)`; networkThrottling = `${Util.i18n.formatMilliseconds(requestLatencyMs, 1)} HTTP RTT, ` + `${Util.i18n.formatKbps(throttling.downloadThroughputKbps)} down, ` + `${Util.i18n.formatKbps(throttling.uploadThroughputKbps)} up (DevTools)`; @@ -448,7 +451,10 @@ class Util { } case 'simulate': { const {cpuSlowdownMultiplier, rttMs, throughputKbps} = throttling; - cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (Simulated)`; + // TODO: better api in i18n formatter such that this isn't needed. + const cpuGranularity = Number.isInteger(cpuSlowdownMultiplier) ? 1 : 0.1; + // eslint-disable-next-line max-len + cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier, cpuGranularity)}x slowdown (Simulated)`; networkThrottling = `${Util.i18n.formatMilliseconds(rttMs)} TCP RTT, ` + `${Util.i18n.formatKbps(throughputKbps)} throughput (Simulated)`; From 350a33a1446496e095e90735a7f1fe240c26b2f8 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 8 Apr 2022 12:51:49 -0700 Subject: [PATCH 11/13] tweak --- report/renderer/i18n.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index fcae90ca2efb..c426094e5f3c 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -39,8 +39,8 @@ export class I18n { _formatNumberWithGranularity(number, granularity, opts = {}) { opts = {...opts}; const log10 = -Math.log10(granularity); - if (!Number.isFinite(log10) || (granularity > 1 && Math.floor(log10) !== log10)) { - throw new Error(`granularity of ${granularity} is invalid`); + if (!Number.isFinite(log10) || (granularity > 1 && !Number.isInteger(log10))) { + console.warn(`granularity of ${granularity} is invalid, results may be unexpected`); } if (granularity < 1) { From a9c738850f65b40e6e76bb12c305984374cec06e Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 8 Apr 2022 12:52:46 -0700 Subject: [PATCH 12/13] fix --- report/renderer/i18n.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index c426094e5f3c..6800c353d691 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -40,7 +40,8 @@ export class I18n { opts = {...opts}; const log10 = -Math.log10(granularity); if (!Number.isFinite(log10) || (granularity > 1 && !Number.isInteger(log10))) { - console.warn(`granularity of ${granularity} is invalid, results may be unexpected`); + console.warn(`granularity of ${granularity} is invalid, default to value of 1`); + granularity = 1; } if (granularity < 1) { From 7db81d2775800af797c955eabc1468584a5496d3 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 8 Apr 2022 12:52:57 -0700 Subject: [PATCH 13/13] fix --- report/renderer/i18n.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/report/renderer/i18n.js b/report/renderer/i18n.js index 6800c353d691..83b33c23b741 100644 --- a/report/renderer/i18n.js +++ b/report/renderer/i18n.js @@ -40,7 +40,7 @@ export class I18n { opts = {...opts}; const log10 = -Math.log10(granularity); if (!Number.isFinite(log10) || (granularity > 1 && !Number.isInteger(log10))) { - console.warn(`granularity of ${granularity} is invalid, default to value of 1`); + console.warn(`granularity of ${granularity} is invalid, defaulting to value of 1`); granularity = 1; }