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.

Sign up for GitHub

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

Jump to bottom

i18n: localize units in report formatter #13830

Merged
merged 14 commits into from
Apr 19, 2022
Merged

i18n: localize units in report formatter #13830

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
504e62a
first pass, localize units
connorjclark Apr 6, 2022
0b88142
localize formatDuration
connorjclark Apr 6, 2022
7dd7d01
hmm
connorjclark Apr 6, 2022
08bc98b
update
connorjclark Apr 7, 2022
b4a8e23
Update lighthouse-core/audits/bootup-time.js
connorjclark Apr 7, 2022
61c23f5
pr
connorjclark Apr 7, 2022
6232d93
kb/s
connorjclark Apr 7, 2022
9e82306
update
connorjclark Apr 8, 2022
325d84b
fix
connorjclark Apr 8, 2022
0d3cbc6
fix
connorjclark Apr 8, 2022
350a33a
tweak
connorjclark Apr 8, 2022
a9c7388
fix
connorjclark Apr 8, 2022
7db81d2
fix
connorjclark Apr 8, 2022
fc20aad
Merge remote-tracking branch 'origin/master' into i18n-formatter-units
connorjclark Apr 19, 2022
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
2 changes: 1 addition & 1 deletion flow-report/src/summary/category.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ const SummaryTooltip: FunctionComponent<{
{
!displayAsFraction && category.score !== null && <>
<span> · </span>
<span>{i18n.formatNumber(category.score * 100)}</span>
<span>{i18n.formatInteger(category.score * 100)}</span>
</>
}
</div>
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/audits/bootup-time.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ class BootupTime extends Audit {
totalBootupTime
);


connorjclark marked this conversation as resolved.
Show resolved Hide resolved
return {
score,
numericValue: totalBootupTime,
Expand Down
10 changes: 5 additions & 5 deletions lighthouse-core/util-commonjs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, ` +
Copy link
Member

Choose a reason for hiding this comment

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

can do unit: 'kilobit-per-second' for these since it's Kbps (not...Kibps?). It'll be kb/s instead of Kbps though

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sounds better to me!

`${Util.i18n.formatInteger(throttling.uploadThroughputKbps)}${NBSP}Kbps up (DevTools)`;

const isSlow4G = () => {
return requestLatencyMs === 150 * 3.75 &&
Expand All @@ -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;
Expand Down
142 changes: 90 additions & 52 deletions report/renderer/i18n.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,58 @@ 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;
}

get strings() {
return this._strings;
}

/**
* @param {number} number
* @param {number} granularity
* @param {Intl.NumberFormatOptions} opts
* @return {string}
*/
_formatNumberWithGranularity(number, granularity, opts = {}) {
opts = {...opts};

This comment was marked as resolved.

Sign in to view

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah i had my cs prof yelling in my head "don't mutate input parameters!" so i copied it

const log10 = -Math.log10(granularity);
if (!Number.isFinite(log10) || (granularity > 1 && Math.floor(log10) !== log10)) {
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 if (!Number.isFinite(log10) || !Number.isInteger(log10)) { is sufficient if we just want powers of 10

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have a test that uses 0.5 as a value here:

lighthouse/report/test/renderer/i18n-test.js

Line 63 in b4a8e23

granularity = 0.5;

and given we pass granularity through a details object throwing here may even be a breaking change... but good thing we are in pre-10.0 mode :)

hmmm throwing in report code is probably bad anyhow. wish we had a DCHECK (cc @exterkamp lol)

Copy link
Member

Choose a reason for hiding this comment

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

We have a test that uses 0.5 as a value here:

ha, well that looks like something that was supported so it was nice to add a test, rather that something we've ever used or wanted to use, but if we want to keep it, if (!Number.isFinite(log10) || (granularity > 1 && !Number.isInteger(log10))) is slightly easier to understand

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

for some reason i had no idea isInteger existed

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".
Copy link
Member

Choose a reason for hiding this comment

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

signDisplay: 'negative' will finally let this be handled how signDisplay should have defaulted in the first place :) Firefox only for now though.

if (Object.is(number, -0)) number = 0;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could have done if (number === 0) number = 0 but it was just too much.

Copy link
Member

Choose a reason for hiding this comment

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

What if JS added ==== so -0 ==== 0 would be false 🤪

Copy link
Member

Choose a reason for hiding this comment

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

IEEE-754 requires that they compare as equal 🤓


return new Intl.NumberFormat(this._locale, opts).format(number).replace(' ', NBSP2);
Copy link
Member

Choose a reason for hiding this comment

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

It seems like it should be fine to do the replace thing, though it feels dirty and maybe there's somehow an issue with rtl or something? 🤷

Do we still need it? I assume we do, but it would be cool if our various layout initiatives over the years made it unnecessary to keep them from breaking across lines

Copy link
Member

Choose a reason for hiding this comment

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

Interestingly, for new Intl.NumberFormat('fr', {style: 'unit', unit: 'second', unitDisplay: 'long'}).format(5) ('5 secondes'), the second character is NBSP2, but that doesn't appear true for most (all?) other languages.

new Intl.NumberFormat('fr', {style: 'unit', unit: 'second', unitDisplay: 'short'}) has \u202f (narrow no-break space) as its second character.

Also, languages disagree on spacing of units :/
new Intl.NumberFormat('en-US', {style: 'unit', unit: 'byte', unitDisplay: 'narrow'}).format(5) -> '5B'
new Intl.NumberFormat('en-US', {style: 'unit', unit: 'byte', unitDisplay: 'narrow'}).format(5) -> '5 B'

Copy link
Collaborator Author

@connorjclark connorjclark Apr 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

omg this is infuriatingly inconsistent. no way this is due to locale differences, these control characters are formatting and what locale would expect numbers to be disassociated from their unit by newlines?? this has to be a mistake in the localization library...

your fr example is odd because the CLDR data has 32 space for long but narrow non-break space for short, based on Intl output I'd expect the former to show NBSP2. (narrow has no whitespace)

One can sort of see reason for there being a non-breaking space between the number and the short string but not for the longer one, as without that number the shorter strings being further from their number gives them far less context.

(your last example is the same two lines of code btw. i'd expect narrow to never include any whitespace. I see de does though... I think for some languages CLDR doesn't bother defining narrows strings so it defaults to short)

Copy link
Member

Choose a reason for hiding this comment

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

One can sort of see reason for there being a non-breaking space between the number and the short string but not for the longer one, as without that number the shorter strings being further from their number gives them far less context.

totally, but why isn't it the same across languages?? Or at least, why wouldn't english (or german or spanish or...) do it as well?

(your last example is the same two lines of code btw. i'd expect narrow to never include any whitespace)

oops, I meant de for the second one:

new Intl.NumberFormat('en-US', {style: 'unit', unit: 'byte', unitDisplay: 'narrow'}).format(5) -> '5B'
new Intl.NumberFormat('de', {style: 'unit', unit: 'byte', unitDisplay: 'narrow'}).format(5) -> '5 B'

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Do we still need it? I assume we do, but it would be cool if our various layout initiatives over the years made it unnecessary to keep them from breaking across lines

image

hmm....

}

/**
* Format number.
* @param {number} number
* @param {number=} granularity Number of decimal places to include. Defaults to 0.1.
* @return {string}
*/
formatNumber(number, granularity = 0.1) {
Copy link
Collaborator Author

@connorjclark connorjclark Apr 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

perhaps this default should be 1? it would match the previous behavior, such that integers are formatted without a fractional component

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

instead I made a new function formatInteger

Copy link
Member

Choose a reason for hiding this comment

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

yeah, it's reasonable to show the extra fractional digit when granularity is 0.1 (which is what #11489 did for bytes), and maybe we don't really want 0.1 as the default based on how we usually don't do anything about it, but that's annoying to audit and formatInteger is a great solution :)

const coarseValue = Math.round(number / granularity) * granularity;
return this._numberFormatter.format(coarseValue);
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);
}

/**
Expand All @@ -49,7 +82,7 @@ export class I18n {
* @return {string}
*/
formatPercent(number) {
return this._percentFormatter.format(number);
return new Intl.NumberFormat(this._locale, {style: 'percent'}).format(number);
}

/**
Expand All @@ -58,9 +91,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`;
}

/**
Expand All @@ -69,9 +100,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`;
}

/**
Expand All @@ -80,9 +109,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',
});
}

/**
Expand All @@ -93,25 +124,10 @@ 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._formatNumberWithGranularity(size, granularity, {
style: 'unit',
unit: 'byte',
unitDisplay: 'narrow',
});
}

Expand All @@ -121,10 +137,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',
});
}

/**
Expand All @@ -133,8 +150,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',
});
}

/**
Expand All @@ -154,10 +174,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));
Expand All @@ -169,6 +189,10 @@ export class I18n {
* @return {string}
*/
formatDuration(timeInMilliseconds) {
Copy link
Collaborator Author

@connorjclark connorjclark Apr 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

just realized the comment for this function was wrong

1d 2h 13m 52s

we had NBSP all up in there...

// 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) {
return 'None';
Expand All @@ -177,19 +201,33 @@ export class I18n {
/** @type {Array<string>} */
const parts = [];
/** @type {Record<string, number>} */
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<string, string>} */
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)
Copy link
Member

Choose a reason for hiding this comment

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

this seems like it could be problematic to use 0 here for languages with different (abbreviated) unit names for plurals? Might not be a problem for any of our languages, but this also seems to be a very roundabout way vs concatenating calls to _formatNumberWithGranularity. What's the motivation? May be worth a comment

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

good point, better to call that function directly

.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}`);
}
});

Expand Down
10 changes: 5 additions & 5 deletions report/renderer/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,9 +431,9 @@ class Util {
case 'devtools': {
const {cpuSlowdownMultiplier, requestLatencyMs} = throttling;
cpuThrottling = `${Util.i18n.formatNumber(cpuSlowdownMultiplier)}x slowdown (DevTools)`;
Copy link
Member

Choose a reason for hiding this comment

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

probably formatInteger on the cpu throttling as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

While we don't ever assign this value to non-integer numbers, others might via the CLI.

Copy link
Member

Choose a reason for hiding this comment

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

fair point. 2.0x looks kind of dumb, though :)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hold up why don't we just do ${cpuSlowdownMultiplier}x

this entire string gets a F for i18n atm anyway so we still need to do this correctly with a ICU message

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oh right, arabic

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 &&
Expand All @@ -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;
Expand Down
26 changes: 22 additions & 4 deletions report/test/renderer/i18n-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,12 @@ 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');
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', () => {
Expand Down Expand Up @@ -106,6 +109,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;
Expand All @@ -114,7 +132,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', () => {
Expand All @@ -125,7 +143,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', () => {
Expand Down
2 changes: 1 addition & 1 deletion report/test/renderer/util-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
4 changes: 2 additions & 2 deletions treemap/app/src/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ class TreemapUtil {
* @param {string} unit
*/
static format(value, unit) {
Copy link
Member

Choose a reason for hiding this comment

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

Do we use this function anywhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

🤭

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

lol i'll delete it

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}`;
}

Expand Down