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: rename i18n to i18n-formatter, move strings to Util #13933

Merged
merged 4 commits into from
Jan 23, 2023
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions core/util.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
* limitations under the License.
*/

/** @template T @typedef {import('./i18n').I18n<T>} I18n */
/** @typedef {import('./i18n-formatter').I18nFormatter} I18nFormatter */

const ELLIPSIS = '\u2026';
const NBSP = '\xa0';
Expand All @@ -42,9 +42,21 @@ const listOfTlds = [
];

class Util {
/** @type {I18n<typeof UIStrings>} */
/** @type {I18nFormatter} */
// @ts-expect-error: Is set in report renderer.
static i18n = null;
static strings = /** @type {typeof UIStrings} */ ({});

/**
* @param {Record<string, string>} providedStrings
*/
static applyStrings(providedStrings) {
this.strings = {
// Set missing renderer strings to default (english) values.
...UIStrings,
...providedStrings,
};
}

static get PASS_THRESHOLD() {
return PASS_THRESHOLD;
Expand Down Expand Up @@ -511,7 +523,7 @@ class Util {

switch (settings.throttlingMethod) {
case 'provided':
summary = networkThrottling = cpuThrottling = Util.i18n.strings.throttlingProvided;
summary = networkThrottling = cpuThrottling = Util.strings.throttlingProvided;
break;
case 'devtools': {
const {cpuSlowdownMultiplier, requestLatencyMs} = throttling;
Expand All @@ -526,7 +538,8 @@ class Util {
throttling.downloadThroughputKbps === 1.6 * 1024 * 0.9 &&
throttling.uploadThroughputKbps === 750 * 0.9;
};
summary = isSlow4G() ? Util.i18n.strings.runtimeSlow4g : Util.i18n.strings.runtimeCustom;
summary = isSlow4G() ?
Util.strings.runtimeSlow4g : Util.strings.runtimeCustom;
break;
}
case 'simulate': {
Expand All @@ -539,11 +552,12 @@ class Util {
const isSlow4G = () => {
return rttMs === 150 && throughputKbps === 1.6 * 1024;
};
summary = isSlow4G() ? Util.i18n.strings.runtimeSlow4g : Util.i18n.strings.runtimeCustom;
summary = isSlow4G() ?
Util.strings.runtimeSlow4g : Util.strings.runtimeCustom;
break;
}
default:
summary = cpuThrottling = networkThrottling = Util.i18n.strings.runtimeUnknown;
summary = cpuThrottling = networkThrottling = Util.strings.runtimeUnknown;
}

// devtools-entry.js always sets `screenEmulation.disabled` when using mobile emulation,
Expand All @@ -556,11 +570,11 @@ class Util {
settings.formFactor === 'mobile' :
settings.screenEmulation.mobile;

let deviceEmulation = Util.i18n.strings.runtimeMobileEmulation;
let deviceEmulation = Util.strings.runtimeMobileEmulation;
if (isScreenEmulationDisabled) {
deviceEmulation = Util.i18n.strings.runtimeNoEmulation;
deviceEmulation = Util.strings.runtimeNoEmulation;
} else if (!isScreenEmulationMobile) {
deviceEmulation = Util.i18n.strings.runtimeDesktopEmulation;
deviceEmulation = Util.strings.runtimeDesktopEmulation;
}

const screenEmulation = isScreenEmulationDisabled ?
Expand Down Expand Up @@ -805,6 +819,7 @@ const UIStrings = {
runtimeCustom: 'Custom throttling',
};
Util.UIStrings = UIStrings;
Util.strings = {...UIStrings};
Copy link
Member

Choose a reason for hiding this comment

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

What was the reason we set these "static" variables here rather than in the class declaration? Might be a good comment to add.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case, it's because moving UIStrings declaration to be before the class will be bad for git blame history. I'm not sure that's a particularly useful thing to call out in a comment tho


module.exports = {
Util,
Expand Down
20 changes: 12 additions & 8 deletions flow-report/src/i18n/i18n.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@ import {createContext, FunctionComponent} from 'preact';
import {useContext, useMemo} from 'preact/hooks';

import {formatMessage} from '../../../shared/localization/format';
import {I18n} from '../../../report/renderer/i18n';
import {I18nFormatter} from '../../../report/renderer/i18n-formatter';
import {UIStrings} from './ui-strings';
import {useFlowResult} from '../util';
import strings from './localized-strings.js';
import {Util} from '../../../report/renderer/util';

const I18nContext = createContext(new I18n('en-US', {...Util.UIStrings, ...UIStrings}));
const I18nContext = createContext({
formatter: new I18nFormatter('en-US'),
strings: {...Util.UIStrings, ...UIStrings},
});

function useLhrLocale() {
const flowResult = useFlowResult();
Expand Down Expand Up @@ -51,9 +54,7 @@ const I18nProvider: FunctionComponent = ({children}) => {
const {locale, lhrStrings} = useLhrLocale();

const i18n = useMemo(() => {
const i18n = new I18n(locale, {
// Set any missing lhr strings to default (english) values.
...Util.UIStrings,
Util.applyStrings({
// Preload with strings from the first lhr.
// Used for legacy report components imported into the flow report.
...lhrStrings,
Expand All @@ -64,10 +65,13 @@ const I18nProvider: FunctionComponent = ({children}) => {
});

// Initialize renderer util i18n for strings rendered in wrapped components.
// TODO: Don't attach global i18n to `Util`.
Util.i18n = i18n;
// TODO: Don't attach global formatter to `Util`.
Util.i18n = new I18nFormatter(locale);

return i18n;
return {
formatter: Util.i18n,
strings: Util.strings as typeof UIStrings & typeof Util.UIStrings,
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
strings: Util.strings as typeof UIStrings & typeof Util.UIStrings,
strings: Util.strings as (typeof UIStrings & typeof Util.UIStrings),

};
}, [locale, lhrStrings]);

return (
Expand Down
2 changes: 1 addition & 1 deletion flow-report/src/sidebar/sidebar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ const SidebarHeader: FunctionComponent<{title: string, date: string}> = ({title,
return (
<div className="SidebarHeader">
<div className="SidebarHeader__title">{title}</div>
<div className="SidebarHeader__date">{i18n.formatDateTime(date)}</div>
<div className="SidebarHeader__date">{i18n.formatter.formatDateTime(date)}</div>
</div>
);
};
Expand Down
2 changes: 1 addition & 1 deletion flow-report/src/summary/category.tsx
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.formatInteger(category.score * 100)}</span>
<span>{i18n.formatter.formatInteger(category.score * 100)}</span>
</>
}
</div>
Expand Down
18 changes: 9 additions & 9 deletions report/renderer/category-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ export class CategoryRenderer {
*/
get _clumpTitles() {
return {
warning: Util.i18n.strings.warningAuditsGroupTitle,
manual: Util.i18n.strings.manualAuditsGroupTitle,
passed: Util.i18n.strings.passedAuditsGroupTitle,
notApplicable: Util.i18n.strings.notApplicableAuditsGroupTitle,
warning: Util.strings.warningAuditsGroupTitle,
manual: Util.strings.manualAuditsGroupTitle,
passed: Util.strings.passedAuditsGroupTitle,
notApplicable: Util.strings.notApplicableAuditsGroupTitle,
};
}

Expand All @@ -62,7 +62,7 @@ export class CategoryRenderer {
* @return {!Element}
*/
populateAuditValues(audit, component) {
const strings = Util.i18n.strings;
const strings = Util.strings;
const auditEl = this.dom.find('.lh-audit', component);
auditEl.id = audit.result.id;
const scoreDisplayMode = audit.result.scoreDisplayMode;
Expand Down Expand Up @@ -334,8 +334,8 @@ export class CategoryRenderer {
el.append(descriptionEl);
}

this.dom.find('.lh-clump-toggletext--show', el).textContent = Util.i18n.strings.show;
this.dom.find('.lh-clump-toggletext--hide', el).textContent = Util.i18n.strings.hide;
this.dom.find('.lh-clump-toggletext--show', el).textContent = Util.strings.show;
this.dom.find('.lh-clump-toggletext--hide', el).textContent = Util.strings.hide;

clumpElement.classList.add(`lh-clump--${clumpId.toLowerCase()}`);
return el;
Expand Down Expand Up @@ -393,7 +393,7 @@ export class CategoryRenderer {
percentageEl.textContent = scoreOutOf100.toString();
if (category.score === null) {
percentageEl.textContent = '?';
percentageEl.title = Util.i18n.strings.errorLabel;
percentageEl.title = Util.strings.errorLabel;
}

// Render a numerical score if the category has applicable audits, or no audits whatsoever.
Expand All @@ -402,7 +402,7 @@ export class CategoryRenderer {
} else {
wrapper.classList.add(`lh-gauge__wrapper--not-applicable`);
percentageEl.textContent = '-';
percentageEl.title = Util.i18n.strings.notApplicableAuditsGroupTitle;
percentageEl.title = Util.strings.notApplicableAuditsGroupTitle;
}

this.dom.find('.lh-gauge__label', tmpl).textContent = category.title;
Expand Down
7 changes: 4 additions & 3 deletions report/renderer/crc-details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ class CriticalRequestChainRenderer {
if (!segment.hasChildren) {
const {startTime, endTime, transferSize} = segment.node.request;
const span = dom.createElement('span', 'lh-crc-node__chain-duration');
span.textContent = ' - ' + Util.i18n.formatMilliseconds((endTime - startTime) * 1000) + ', ';
span.textContent =
' - ' + Util.i18n.formatMilliseconds((endTime - startTime) * 1000) + ', ';
const span2 = dom.createElement('span', 'lh-crc-node__chain-duration');
span2.textContent = Util.i18n.formatBytesToKiB(transferSize, 0.01);

Expand Down Expand Up @@ -177,9 +178,9 @@ class CriticalRequestChainRenderer {
const containerEl = dom.find('.lh-crc', tmpl);

// Fill in top summary.
dom.find('.lh-crc-initial-nav', tmpl).textContent = Util.i18n.strings.crcInitialNavigation;
dom.find('.lh-crc-initial-nav', tmpl).textContent = Util.strings.crcInitialNavigation;
dom.find('.lh-crc__longest_duration_label', tmpl).textContent =
Util.i18n.strings.crcLongestDurationLabel;
Util.strings.crcLongestDurationLabel;
dom.find('.lh-crc__longest_duration', tmpl).textContent =
Util.i18n.formatMilliseconds(details.longestChain.duration);

Expand Down
13 changes: 2 additions & 11 deletions report/renderer/i18n.js → report/renderer/i18n-formatter.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,27 +9,18 @@ const NBSP2 = '\xa0';
const KiB = 1024;
const MiB = KiB * KiB;

/**
* @template T
*/
export class I18n {
export class I18nFormatter {
/**
* @param {LH.Locale} locale
* @param {T} strings
*/
constructor(locale, strings) {
constructor(locale) {
// When testing, use a locale with more exciting numeric formatting.
if (locale === 'en-XA') locale = 'de';

this._locale = locale;
this._strings = strings;
this._cachedNumberFormatters = new Map();
}

get strings() {
return this._strings;
}

/**
* @param {number} number
* @param {number|undefined} granularity
Expand Down
8 changes: 4 additions & 4 deletions report/renderer/performance-category-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer {
* @override
*/
render(category, groups, options) {
const strings = Util.i18n.strings;
const strings = Util.strings;
const element = this.dom.createElement('div', 'lh-category');
element.id = category.id;
element.append(this.renderCategoryHeader(category, groups, options));
Expand All @@ -200,8 +200,8 @@ export class PerformanceCategoryRenderer extends CategoryRenderer {
labelEl.htmlFor = checkboxId;
const showEl = this.dom.createChildOf(labelEl, 'span', 'lh-metrics-toggle__labeltext--show');
const hideEl = this.dom.createChildOf(labelEl, 'span', 'lh-metrics-toggle__labeltext--hide');
showEl.textContent = Util.i18n.strings.expandView;
hideEl.textContent = Util.i18n.strings.collapseView;
showEl.textContent = Util.strings.expandView;
hideEl.textContent = Util.strings.collapseView;

const metricsBoxesEl = this.dom.createElement('div', 'lh-metrics-container');
metricsGroupEl.insertBefore(metricsBoxesEl, metricsFooterEl);
Expand Down Expand Up @@ -333,7 +333,7 @@ export class PerformanceCategoryRenderer extends CategoryRenderer {
renderMetricAuditFilter(filterableMetrics, categoryEl) {
const metricFilterEl = this.dom.createElement('div', 'lh-metricfilter');
const textEl = this.dom.createChildOf(metricFilterEl, 'span', 'lh-metricfilter__text');
textEl.textContent = Util.i18n.strings.showRelevantAudits;
textEl.textContent = Util.strings.showRelevantAudits;

const filterChoices = /** @type {LH.ReportResult.AuditRef[]} */ ([
({acronym: 'All'}),
Expand Down
32 changes: 14 additions & 18 deletions report/renderer/report-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import {CategoryRenderer} from './category-renderer.js';
import {DetailsRenderer} from './details-renderer.js';
import {ElementScreenshotRenderer} from './element-screenshot-renderer.js';
import {I18n} from './i18n.js';
import {I18nFormatter} from './i18n-formatter.js';
import {PerformanceCategoryRenderer} from './performance-category-renderer.js';
import {PwaCategoryRenderer} from './pwa-category-renderer.js';
import {Util} from './util.js';
Expand Down Expand Up @@ -108,7 +108,7 @@ export class ReportRenderer {

this._renderMetaBlock(report, footer);

this._dom.find('.lh-footer__version_issue', footer).textContent = Util.i18n.strings.footerIssue;
this._dom.find('.lh-footer__version_issue', footer).textContent = Util.strings.footerIssue;
this._dom.find('.lh-footer__version', footer).textContent = report.lighthouseVersion;
return footer;
}
Expand All @@ -128,15 +128,15 @@ export class ReportRenderer {
const axeVersion = report.environment.credits?.['axe-core'];

const devicesTooltipTextLines = [
`${Util.i18n.strings.runtimeSettingsBenchmark}: ${benchmarkIndex}`,
`${Util.i18n.strings.runtimeSettingsCPUThrottling}: ${envValues.cpuThrottling}`,
`${Util.strings.runtimeSettingsBenchmark}: ${benchmarkIndex}`,
`${Util.strings.runtimeSettingsCPUThrottling}: ${envValues.cpuThrottling}`,
];
if (envValues.screenEmulation) {
devicesTooltipTextLines.push(
`${Util.i18n.strings.runtimeSettingsScreenEmulation}: ${envValues.screenEmulation}`);
`${Util.strings.runtimeSettingsScreenEmulation}: ${envValues.screenEmulation}`);
}
if (axeVersion) {
devicesTooltipTextLines.push(`${Util.i18n.strings.runtimeSettingsAxeVersion}: ${axeVersion}`);
devicesTooltipTextLines.push(`${Util.strings.runtimeSettingsAxeVersion}: ${axeVersion}`);
}

// [CSS icon class, textContent, tooltipText]
Expand All @@ -147,16 +147,16 @@ export class ReportRenderer {
`${envValues.deviceEmulation} with Lighthouse ${report.lighthouseVersion}`,
devicesTooltipTextLines.join('\n')],
['samples-one',
Util.i18n.strings.runtimeSingleLoad,
Util.i18n.strings.runtimeSingleLoadTooltip],
Util.strings.runtimeSingleLoad,
Util.strings.runtimeSingleLoadTooltip],
['stopwatch',
Util.i18n.strings.runtimeAnalysisWindow],
Util.strings.runtimeAnalysisWindow],
['networkspeed',
`${envValues.summary}`,
`${Util.i18n.strings.runtimeSettingsNetworkThrottling}: ${envValues.networkThrottling}`],
`${Util.strings.runtimeSettingsNetworkThrottling}: ${envValues.networkThrottling}`],
['chrome',
`Using ${chromeVer}` + (channel ? ` with ${channel}` : ''),
`${Util.i18n.strings.runtimeSettingsUANetwork}: "${report.environment.networkUserAgent}"`],
`${Util.strings.runtimeSettingsUANetwork}: "${report.environment.networkUserAgent}"`],
];

const metaItemsEl = this._dom.find('.lh-meta__items', footer);
Expand Down Expand Up @@ -184,7 +184,7 @@ export class ReportRenderer {

const container = this._dom.createComponent('warningsToplevel');
const message = this._dom.find('.lh-warnings__msg', container);
message.textContent = Util.i18n.strings.toplevelWarningsMessage;
message.textContent = Util.strings.toplevelWarningsMessage;

const warnings = [];
for (const warningString of report.runWarnings) {
Expand Down Expand Up @@ -260,12 +260,8 @@ export class ReportRenderer {
* @return {!DocumentFragment}
*/
_renderReport(report) {
const i18n = new I18n(report.configSettings.locale, {
// Set missing renderer strings to default (english) values.
...Util.UIStrings,
...report.i18n.rendererFormattedStrings,
});
Util.i18n = i18n;
Util.applyStrings(report.i18n.rendererFormattedStrings);
Util.i18n = new I18nFormatter(report.configSettings.locale);
Util.reportJson = report;

const detailsRenderer = new DetailsRenderer(this._dom, {
Expand Down
Loading