From fceab88e3eee56fbd100677addc7192ca50996e1 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Tue, 4 Sep 2018 18:08:07 -0700 Subject: [PATCH 1/2] report(psi): add lab data summary sentence --- lighthouse-core/lib/locales/en-US.json | 4 ++++ lighthouse-core/report/html/renderer/psi.js | 15 ++++++++++++--- lighthouse-core/report/html/renderer/util.js | 3 +++ lighthouse-core/runner.js | 10 ++++++---- .../test/report/html/renderer/psi-test.js | 8 ++++++++ lighthouse-core/test/results/sample_v2.json | 1 + typings/lhr.d.ts | 2 +- 7 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lighthouse-core/lib/locales/en-US.json b/lighthouse-core/lib/locales/en-US.json index c28ad3bf1c3a..ed3d6fce38f7 100644 --- a/lighthouse-core/lib/locales/en-US.json +++ b/lighthouse-core/lib/locales/en-US.json @@ -451,6 +451,10 @@ "message": "Passed audits", "description": "Section heading shown above a list of audits that are passing. 'Passed' here refers to a passing grade. This section is collapsed by default, as the user should be focusing on the failed audits instead. Users can click this heading to reveal the list." }, + "lighthouse-core/report/html/renderer/util.js | psiDescription": { + "message": "[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.", + "description": "Explanation shown to users below performance results to inform them that the test was done with a 3G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation." + }, "lighthouse-core/report/html/renderer/util.js | scorescaleLabel": { "message": "Score scale:", "description": "Label preceding a pictorial explanation of the scoring scale: 0-50 is red (bad), 50-90 is orange (ok), 90-100 is green (good). These colors are used throughout the report to provide context for how good/bad a particular result is." diff --git a/lighthouse-core/report/html/renderer/psi.js b/lighthouse-core/report/html/renderer/psi.js index 93d81503fa31..63c7c3d75adf 100644 --- a/lighthouse-core/report/html/renderer/psi.js +++ b/lighthouse-core/report/html/renderer/psi.js @@ -18,6 +18,14 @@ /* globals self DOM PerformanceCategoryRenderer Util DetailsRenderer */ +/** + * @typedef PreparedLabData + * @property {Element} scoreGaugeEl + * @property {Element} perfCategoryEl + * @property {string|null} finalScreenshotDataUri + * @property {string} psiDescription + */ + /** * Returns all the elements that PSI needs to render the report @@ -31,10 +39,10 @@ * * @param {string} LHResultJsonString The stringified version of {LH.Result} * @param {Document} document The host page's window.document - * @return {{scoreGaugeEl: Element, perfCategoryEl: Element, finalScreenshotDataUri: string|null}} + * @return {PreparedLabData} */ function prepareLabData(LHResultJsonString, document) { - const lhResult = /** @type {LH.Result} */ JSON.parse(LHResultJsonString); + const lhResult = /** @type {LH.Result} */ (JSON.parse(LHResultJsonString)); const dom = new DOM(document); // Assume fresh styles needed on every call, so mark all template styles as unused. @@ -57,7 +65,8 @@ function prepareLabData(LHResultJsonString, document) { scoreGaugeWrapperEl.removeAttribute('href'); const finalScreenshotDataUri = _getFinalScreenshot(perfCategory); - return {scoreGaugeEl, perfCategoryEl, finalScreenshotDataUri}; + const psiDescription = lhResult.i18n.rendererFormattedStrings.psiDescription; + return {scoreGaugeEl, perfCategoryEl, finalScreenshotDataUri, psiDescription}; } /** diff --git a/lighthouse-core/report/html/renderer/util.js b/lighthouse-core/report/html/renderer/util.js index b059a2c75faf..972ccba4edb2 100644 --- a/lighthouse-core/report/html/renderer/util.js +++ b/lighthouse-core/report/html/renderer/util.js @@ -481,6 +481,9 @@ Util.UIStrings = { crcInitialNavigation: 'Initial Navigation', /** Label of value shown in the summary of critical request chains. Refers to the total amount of time (milliseconds) of the longest critical path chain/sequence of network requests. Example value: 2310 ms */ crcLongestDurationLabel: 'Maximum critical path latency:', + + /** Explanation shown to users below performance results to inform them that the test was done with a 3G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation. */ + psiDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.', }; if (typeof module !== 'undefined' && module.exports) { diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index 25facc675598..1962307f287d 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -138,12 +138,14 @@ class Runner { categories, categoryGroups: runOpts.config.groups || undefined, timing: {total: Date.now() - startTime}, + i18n: { + rendererFormattedStrings: i18n.getRendererFormattedStrings(settings.locale), + icuMessagePaths: {}, + }, }; - lhr.i18n = { - rendererFormattedStrings: i18n.getRendererFormattedStrings(settings.locale), - icuMessagePaths: i18n.replaceIcuMessageInstanceIds(lhr, settings.locale), - }; + // Replace ICU message references with localized strings; save replaced paths in lhr. + lhr.i18n.icuMessagePaths = i18n.replaceIcuMessageInstanceIds(lhr, settings.locale); const report = generateReport(lhr, settings.output); return {lhr, artifacts, report}; diff --git a/lighthouse-core/test/report/html/renderer/psi-test.js b/lighthouse-core/test/report/html/renderer/psi-test.js index 5fed4e78ed7c..50236581d4d3 100644 --- a/lighthouse-core/test/report/html/renderer/psi-test.js +++ b/lighthouse-core/test/report/html/renderer/psi-test.js @@ -113,4 +113,12 @@ describe('DOM', () => { assert.equal(datauri, null); }); }); + + describe('psiDescription', () => { + it('provides a description string', () => { + const {psiDescription} = prepareLabData(sampleResultsStr, document); + assert.equal(typeof psiDescription, 'string'); + assert.ok(psiDescription.length > 0); + }); + }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 230c95f8f598..dcc2313c0a2f 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -3417,6 +3417,7 @@ "opportunityResourceColumnLabel": "Opportunity", "opportunitySavingsColumnLabel": "Estimated Savings", "passedAuditsGroupTitle": "Passed audits", + "psiDescription": "[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.", "scorescaleLabel": "Score scale:", "toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:", "varianceDisclaimer": "Values are estimated and may vary.", diff --git a/typings/lhr.d.ts b/typings/lhr.d.ts index ce2c4b460165..09cf635bf2fb 100644 --- a/typings/lhr.d.ts +++ b/typings/lhr.d.ts @@ -57,7 +57,7 @@ declare global { /** Execution timings for the Lighthouse run */ timing: {total: number, [t: string]: number}; /** The record of all formatted string locations in the LHR and their corresponding source values. */ - i18n?: {rendererFormattedStrings: I18NRendererStrings, icuMessagePaths: I18NMessages}; + i18n: {rendererFormattedStrings: I18NRendererStrings, icuMessagePaths: I18NMessages}; } // Result namespace From 928782f74bba577d931b886c7d8f802c5c573185 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Wed, 5 Sep 2018 12:13:18 -0700 Subject: [PATCH 2/2] feedback --- lighthouse-core/lib/locales/en-US.json | 12 ++++++---- .../renderer/performance-category-renderer.js | 2 +- lighthouse-core/report/html/renderer/psi.js | 18 ++++++--------- lighthouse-core/report/html/renderer/util.js | 4 +++- .../test/report/html/renderer/psi-test.js | 23 ++++++++++++------- lighthouse-core/test/results/sample_v2.json | 3 ++- 6 files changed, 36 insertions(+), 26 deletions(-) diff --git a/lighthouse-core/lib/locales/en-US.json b/lighthouse-core/lib/locales/en-US.json index ed3d6fce38f7..0ce2f3a6cfbe 100644 --- a/lighthouse-core/lib/locales/en-US.json +++ b/lighthouse-core/lib/locales/en-US.json @@ -431,6 +431,14 @@ "message": "Report error: no audit information", "description": "An error string displayed next to a particular audit when it has errored, but not provided any specific error message." }, + "lighthouse-core/report/html/renderer/util.js | labDataTitle": { + "message": "Lab Data", + "description": "Title of the lab data section of the Performance category. Within this section are various speed metrics which quantify the pageload performance into values presented in seconds and milliseconds. \"Lab\" is an abbreviated form of \"laboratory\", and refers to the fact that the data is from a controlled test of a website, not measurements from real users visiting that site." + }, + "lighthouse-core/report/html/renderer/util.js | lsPerformanceCategoryDescription": { + "message": "[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.", + "description": "Explanation shown to users below performance results to inform them that the test was done with a 3G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation." + }, "lighthouse-core/report/html/renderer/util.js | manualAuditsGroupTitle": { "message": "Additional items to manually check", "description": "Section heading shown above a list of audits that were not computed by Lighthouse. They serve as a list of suggestions for the user to go and manually check. For example, Lighthouse can't automate testing cross-browser compatibility, so that is listed within this section, so the user is reminded to test it themselves. This section is collapsed by default, as the user should be focusing on the failed audits instead. Users can click this heading to reveal the list." @@ -451,10 +459,6 @@ "message": "Passed audits", "description": "Section heading shown above a list of audits that are passing. 'Passed' here refers to a passing grade. This section is collapsed by default, as the user should be focusing on the failed audits instead. Users can click this heading to reveal the list." }, - "lighthouse-core/report/html/renderer/util.js | psiDescription": { - "message": "[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.", - "description": "Explanation shown to users below performance results to inform them that the test was done with a 3G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation." - }, "lighthouse-core/report/html/renderer/util.js | scorescaleLabel": { "message": "Score scale:", "description": "Label preceding a pictorial explanation of the scoring scale: 0-50 is red (bad), 50-90 is orange (ok), 90-100 is green (good). These colors are used throughout the report to provide context for how good/bad a particular result is." diff --git a/lighthouse-core/report/html/renderer/performance-category-renderer.js b/lighthouse-core/report/html/renderer/performance-category-renderer.js index 379013d882c1..e9d54f4091a3 100644 --- a/lighthouse-core/report/html/renderer/performance-category-renderer.js +++ b/lighthouse-core/report/html/renderer/performance-category-renderer.js @@ -129,7 +129,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer { // Metrics const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics'); - const metricAuditsEl = this.renderAuditGroup(groups['metrics'], {expandable: false}); + const metricAuditsEl = this.renderAuditGroup(groups.metrics, {expandable: false}); const keyMetrics = metricAudits.filter(a => a.weight >= 3); const otherMetrics = metricAudits.filter(a => a.weight < 3); diff --git a/lighthouse-core/report/html/renderer/psi.js b/lighthouse-core/report/html/renderer/psi.js index 63c7c3d75adf..a97bb963f517 100644 --- a/lighthouse-core/report/html/renderer/psi.js +++ b/lighthouse-core/report/html/renderer/psi.js @@ -18,14 +18,6 @@ /* globals self DOM PerformanceCategoryRenderer Util DetailsRenderer */ -/** - * @typedef PreparedLabData - * @property {Element} scoreGaugeEl - * @property {Element} perfCategoryEl - * @property {string|null} finalScreenshotDataUri - * @property {string} psiDescription - */ - /** * Returns all the elements that PSI needs to render the report @@ -39,7 +31,7 @@ * * @param {string} LHResultJsonString The stringified version of {LH.Result} * @param {Document} document The host page's window.document - * @return {PreparedLabData} + * @return {{scoreGaugeEl: Element, perfCategoryEl: Element, finalScreenshotDataUri: string|null}} */ function prepareLabData(LHResultJsonString, document) { const lhResult = /** @type {LH.Result} */ (JSON.parse(LHResultJsonString)); @@ -53,6 +45,11 @@ function prepareLabData(LHResultJsonString, document) { if (!perfCategory) throw new Error(`No performance category. Can't make lab data section`); if (!reportLHR.categoryGroups) throw new Error(`No category groups found.`); + // Use custom title and description. + reportLHR.categoryGroups.metrics.title = lhResult.i18n.rendererFormattedStrings.labDataTitle; + reportLHR.categoryGroups.metrics.description = + lhResult.i18n.rendererFormattedStrings.lsPerformanceCategoryDescription; + const perfRenderer = new PerformanceCategoryRenderer(dom, new DetailsRenderer(dom)); // PSI environment string will ensure the categoryHeader and permalink elements are excluded const perfCategoryEl = perfRenderer.render(perfCategory, reportLHR.categoryGroups, 'PSI'); @@ -65,8 +62,7 @@ function prepareLabData(LHResultJsonString, document) { scoreGaugeWrapperEl.removeAttribute('href'); const finalScreenshotDataUri = _getFinalScreenshot(perfCategory); - const psiDescription = lhResult.i18n.rendererFormattedStrings.psiDescription; - return {scoreGaugeEl, perfCategoryEl, finalScreenshotDataUri, psiDescription}; + return {scoreGaugeEl, perfCategoryEl, finalScreenshotDataUri}; } /** diff --git a/lighthouse-core/report/html/renderer/util.js b/lighthouse-core/report/html/renderer/util.js index 972ccba4edb2..856f4cc20692 100644 --- a/lighthouse-core/report/html/renderer/util.js +++ b/lighthouse-core/report/html/renderer/util.js @@ -483,7 +483,9 @@ Util.UIStrings = { crcLongestDurationLabel: 'Maximum critical path latency:', /** Explanation shown to users below performance results to inform them that the test was done with a 3G network connection and to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. 'Lighthouse' becomes link text to additional documentation. */ - psiDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.', + lsPerformanceCategoryDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.', + /** Title of the lab data section of the Performance category. Within this section are various speed metrics which quantify the pageload performance into values presented in seconds and milliseconds. "Lab" is an abbreviated form of "laboratory", and refers to the fact that the data is from a controlled test of a website, not measurements from real users visiting that site. */ + labDataTitle: 'Lab Data', }; if (typeof module !== 'undefined' && module.exports) { diff --git a/lighthouse-core/test/report/html/renderer/psi-test.js b/lighthouse-core/test/report/html/renderer/psi-test.js index 50236581d4d3..6c17433fe248 100644 --- a/lighthouse-core/test/report/html/renderer/psi-test.js +++ b/lighthouse-core/test/report/html/renderer/psi-test.js @@ -94,6 +94,21 @@ describe('DOM', () => { prepareLabData(lhrWithoutGroupsStr, document); }, /no category groups/i); }); + + it('includes custom title and description', () => { + const {perfCategoryEl} = prepareLabData(sampleResultsStr, document); + const metricsGroupEl = perfCategoryEl.querySelector('.lh-audit-group--metrics'); + + // Assume using default locale. + const titleEl = metricsGroupEl.querySelector('.lh-audit-group__header'); + assert.equal(titleEl.textContent, Util.UIStrings.labDataTitle); + + // Description supports markdown links, so take everything after the last link. + const descriptionEnd = /[^)]+$/.exec(Util.UIStrings.lsPerformanceCategoryDescription)[0]; + assert.ok(descriptionEnd.length > 6); // If this gets too short, pick a different comparison :) + const descriptionEl = metricsGroupEl.querySelector('.lh-audit-group__description'); + assert.ok(descriptionEl.textContent.endsWith(descriptionEnd)); + }); }); }); @@ -113,12 +128,4 @@ describe('DOM', () => { assert.equal(datauri, null); }); }); - - describe('psiDescription', () => { - it('provides a description string', () => { - const {psiDescription} = prepareLabData(sampleResultsStr, document); - assert.equal(typeof psiDescription, 'string'); - assert.ok(psiDescription.length > 0); - }); - }); }); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index dcc2313c0a2f..bd2e2789eec8 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -3412,12 +3412,13 @@ "crcLongestDurationLabel": "Maximum critical path latency:", "errorLabel": "Error!", "errorMissingAuditInfo": "Report error: no audit information", + "labDataTitle": "Lab Data", + "lsPerformanceCategoryDescription": "[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.", "manualAuditsGroupTitle": "Additional items to manually check", "notApplicableAuditsGroupTitle": "Not applicable", "opportunityResourceColumnLabel": "Opportunity", "opportunitySavingsColumnLabel": "Estimated Savings", "passedAuditsGroupTitle": "Passed audits", - "psiDescription": "[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on emulated 3G. Values are estimated and may vary.", "scorescaleLabel": "Score scale:", "toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:", "varianceDisclaimer": "Values are estimated and may vary.",