From a9c3832e546344b9bfc46c52a0555816e9eae0f9 Mon Sep 17 00:00:00 2001 From: Matt Zeunert Date: Wed, 20 Feb 2019 15:08:45 +0000 Subject: [PATCH] core(details-renderer): add snippet details renderer (#6999) --- lighthouse-core/audits/audit.js | 66 ++++ lighthouse-core/lib/i18n/en-US.json | 8 + .../report/html/html-report-assets.js | 1 + .../report/html/renderer/details-renderer.js | 24 +- .../report/html/renderer/snippet-renderer.js | 364 ++++++++++++++++++ lighthouse-core/report/html/renderer/util.js | 47 +++ lighthouse-core/report/html/report-styles.css | 4 + lighthouse-core/report/html/templates.html | 139 +++++++ lighthouse-core/test/audits/audit-test.js | 108 ++++++ .../html/renderer/details-renderer-test.js | 25 ++ .../html/renderer/snippet-renderer-test.js | 293 ++++++++++++++ lighthouse-core/test/results/sample_v2.json | 2 + package.json | 2 +- proto/lighthouse-result.proto | 6 + proto/sample_v2_round_trip.json | 2 + types/audit-details.d.ts | 37 ++ types/html-renderer.d.ts | 3 + 17 files changed, 1128 insertions(+), 3 deletions(-) create mode 100644 lighthouse-core/report/html/renderer/snippet-renderer.js create mode 100644 lighthouse-core/test/report/html/renderer/snippet-renderer-test.js diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js index ce6188de9559..425388d3e393 100644 --- a/lighthouse-core/audits/audit.js +++ b/lighthouse-core/audits/audit.js @@ -125,6 +125,72 @@ class Audit { }; } + /** + * @param {LH.Audit.Details.List['items']} items + * @returns {LH.Audit.Details.List} + */ + static makeListDetails(items) { + return { + type: 'list', + items: items, + }; + } + + /** @typedef {{ + * content: string; + * title: string; + * lineMessages: LH.Audit.Details.SnippetValue['lineMessages']; + * generalMessages: LH.Audit.Details.SnippetValue['generalMessages']; + * node?: LH.Audit.Details.NodeValue; + * maxLineLength?: number; + * maxLinesAroundMessage?: number; + * }} SnippetInfo */ + /** + * @param {SnippetInfo} snippetInfo + * @return {LH.Audit.Details.SnippetValue} + */ + static makeSnippetDetails({ + content, + title, + lineMessages, + generalMessages, + node, + maxLineLength = 200, + maxLinesAroundMessage = 20, + }) { + const allLines = Audit._makeSnippetLinesArray(content, maxLineLength); + const lines = Util.filterRelevantLines(allLines, lineMessages, maxLinesAroundMessage); + return { + type: 'snippet', + lines, + title, + lineMessages, + generalMessages, + lineCount: allLines.length, + node, + }; + } + + /** + * @param {string} content + * @param {number} maxLineLength + * @returns {LH.Audit.Details.SnippetValue['lines']} + */ + static _makeSnippetLinesArray(content, maxLineLength) { + return content.split('\n').map((line, lineIndex) => { + const lineNumber = lineIndex + 1; + /** @type LH.Audit.Details.SnippetValue['lines'][0] */ + const lineDetail = { + content: line.slice(0, maxLineLength), + lineNumber, + }; + if (line.length > maxLineLength) { + lineDetail.truncated = true; + } + return lineDetail; + }); + } + /** * @param {LH.Audit.Details.Opportunity['headings']} headings * @param {LH.Audit.Details.Opportunity['items']} items diff --git a/lighthouse-core/lib/i18n/en-US.json b/lighthouse-core/lib/i18n/en-US.json index b49203d37bf6..ad046ddbc461 100644 --- a/lighthouse-core/lib/i18n/en-US.json +++ b/lighthouse-core/lib/i18n/en-US.json @@ -1311,6 +1311,14 @@ "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." }, + "lighthouse-core/report/html/renderer/util.js | snippetCollapseButtonLabel": { + "message": "Collapse snippet", + "description": "Label for button that only shows a few lines of the snippet when clicked" + }, + "lighthouse-core/report/html/renderer/util.js | snippetExpandButtonLabel": { + "message": "Expand snippet", + "description": "Label for button that shows all lines of the snippet when clicked" + }, "lighthouse-core/report/html/renderer/util.js | toplevelWarningsMessage": { "message": "There were issues affecting this run of Lighthouse:", "description": "Label shown preceding any important warnings that may have invalidated the entire report. For example, if the user has Chrome extensions installed, they may add enough performance overhead that Lighthouse's performance metrics are unreliable. If shown, this will be displayed at the top of the report UI." diff --git a/lighthouse-core/report/html/html-report-assets.js b/lighthouse-core/report/html/html-report-assets.js index 8ac7d203ea42..0a2b56c0a430 100644 --- a/lighthouse-core/report/html/html-report-assets.js +++ b/lighthouse-core/report/html/html-report-assets.js @@ -16,6 +16,7 @@ const REPORT_JAVASCRIPT = [ fs.readFileSync(require.resolve('details-element-polyfill'), 'utf8'), fs.readFileSync(__dirname + '/renderer/details-renderer.js', 'utf8'), fs.readFileSync(__dirname + '/renderer/crc-details-renderer.js', 'utf8'), + fs.readFileSync(__dirname + '/renderer/snippet-renderer.js', 'utf8'), fs.readFileSync(__dirname + '/../../lib/file-namer.js', 'utf8'), fs.readFileSync(__dirname + '/renderer/logger.js', 'utf8'), fs.readFileSync(__dirname + '/renderer/report-ui-features.js', 'utf8'), diff --git a/lighthouse-core/report/html/renderer/details-renderer.js b/lighthouse-core/report/html/renderer/details-renderer.js index 91a8384b125f..5e8bf852707f 100644 --- a/lighthouse-core/report/html/renderer/details-renderer.js +++ b/lighthouse-core/report/html/renderer/details-renderer.js @@ -16,7 +16,7 @@ */ 'use strict'; -/* globals self CriticalRequestChainRenderer Util URL */ +/* globals self CriticalRequestChainRenderer SnippetRenderer Util URL */ /** @typedef {import('./dom.js')} DOM */ /** @typedef {LH.Audit.Details.Opportunity} OpportunityDetails */ @@ -43,7 +43,7 @@ class DetailsRenderer { } /** - * @param {DetailsJSON|OpportunityDetails} details + * @param {DetailsJSON|OpportunityDetails|LH.Audit.Details.SnippetValue} details * @return {Element|null} */ render(details) { @@ -68,6 +68,11 @@ class DetailsRenderer { case 'table': // @ts-ignore - TODO(bckenny): Fix type hierarchy return this._renderTable(/** @type {TableDetailsJSON} */ (details)); + case 'list': + return this._renderList( + // @ts-ignore - TODO(bckenny): Fix type hierarchy + /** @type {LH.Audit.Details.List} */ (details) + ); case 'code': return this._renderCode(/** @type {DetailsJSON} */ (details)); case 'node': @@ -210,6 +215,21 @@ class DetailsRenderer { return element; } + /** + * @param {LH.Audit.Details.List} details + * @returns {Element} + */ + _renderList(details) { + const listContainer = this._dom.createElement('div', 'lh-list'); + + details.items.forEach(item => { + const snippetEl = SnippetRenderer.render(this._dom, this._templateContext, item, this); + listContainer.appendChild(snippetEl); + }); + + return listContainer; + } + /** * @param {TableDetailsJSON} details * @return {Element} diff --git a/lighthouse-core/report/html/renderer/snippet-renderer.js b/lighthouse-core/report/html/renderer/snippet-renderer.js new file mode 100644 index 000000000000..1a627b5a0d0d --- /dev/null +++ b/lighthouse-core/report/html/renderer/snippet-renderer.js @@ -0,0 +1,364 @@ +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/* globals self, Util */ + +/** @typedef {import('./details-renderer')} DetailsRenderer */ + +/** @enum {number} */ +const LineVisibility = { + /** Show regardless of whether the snippet is collapsed or expanded */ + ALWAYS: 0, + WHEN_COLLAPSED: 1, + WHEN_EXPANDED: 2, +}; + +/** @enum {number} */ +const LineContentType = { + /** A line of content */ + CONTENT_NORMAL: 0, + /** A line of content that's emphasized by setting the CSS background color */ + CONTENT_HIGHLIGHTED: 1, + /** Use when some lines are hidden, shows the "..." placeholder */ + PLACEHOLDER: 2, + /** A message about a line of content or the snippet in general */ + MESSAGE: 3, +}; + +/** @typedef {{ + content: string; + lineNumber: string | number; + contentType: LineContentType; + truncated?: boolean; + visibility?: LineVisibility; +}} LineDetails */ + +const classNamesByContentType = { + [LineContentType.CONTENT_NORMAL]: ['lh-snippet__line--content'], + [LineContentType.CONTENT_HIGHLIGHTED]: [ + 'lh-snippet__line--content', + 'lh-snippet__line--content-highlighted', + ], + [LineContentType.PLACEHOLDER]: ['lh-snippet__line--placeholder'], + [LineContentType.MESSAGE]: ['lh-snippet__line--message'], +}; + +/** + * @param {LH.Audit.Details.SnippetValue['lines']} lines + * @param {number} lineNumber + * @return {{line?: LH.Audit.Details.SnippetValue['lines'][0], previousLine?: LH.Audit.Details.SnippetValue['lines'][0]}} + */ +function getLineAndPreviousLine(lines, lineNumber) { + return { + line: lines.find(l => l.lineNumber === lineNumber), + previousLine: lines.find(l => l.lineNumber === lineNumber - 1), + }; +} + +/** + * @param {LH.Audit.Details.SnippetValue["lineMessages"]} messages + * @param {number} lineNumber + */ +function getMessagesForLineNumber(messages, lineNumber) { + return messages.filter(h => h.lineNumber === lineNumber); +} + +/** + * @param {LH.Audit.Details.SnippetValue} details + * @returns {LH.Audit.Details.SnippetValue['lines']} + */ +function getLinesWhenCollapsed(details) { + const SURROUNDING_LINES_TO_SHOW_WHEN_COLLAPSED = 2; + return Util.filterRelevantLines( + details.lines, + details.lineMessages, + SURROUNDING_LINES_TO_SHOW_WHEN_COLLAPSED + ); +} + +/** + * Render snippet of text with line numbers and annotations. + * By default we only show a few lines around each annotation and the user + * can click "Expand snippet" to show more. + * Content lines with annotations are highlighted. + */ +class SnippetRenderer { + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LH.Audit.Details.SnippetValue} details + * @param {DetailsRenderer} detailsRenderer + * @param {function} toggleExpandedFn + * @return {DocumentFragment} + */ + static renderHeader(dom, tmpl, details, detailsRenderer, toggleExpandedFn) { + const linesWhenCollapsed = getLinesWhenCollapsed(details); + const canExpand = linesWhenCollapsed.length < details.lines.length; + + const header = dom.cloneTemplate('#tmpl-lh-snippet__header', tmpl); + dom.find('.lh-snippet__title', header).textContent = details.title; + + const { + snippetCollapseButtonLabel, + snippetExpandButtonLabel, + } = Util.UIStrings; + dom.find( + '.lh-snippet__btn-label-collapse', + header + ).textContent = snippetCollapseButtonLabel; + dom.find( + '.lh-snippet__btn-label-expand', + header + ).textContent = snippetExpandButtonLabel; + + const toggleExpandButton = dom.find('.lh-snippet__toggle-expand', header); + // If we're already showing all the available lines of the snippet, we don't need an + // expand/collapse button and can remove it from the DOM. + // If we leave the button in though, wire up the click listener to toggle visibility! + if (!canExpand) { + toggleExpandButton.remove(); + } else { + toggleExpandButton.addEventListener('click', () => toggleExpandedFn()); + } + + // We only show the source node of the snippet in DevTools because then the user can + // access the full element detail. Just being able to see the outer HTML isn't very useful. + if (details.node && dom.isDevTools()) { + const nodeContainer = dom.find('.lh-snippet__node', header); + nodeContainer.appendChild(detailsRenderer.renderNode(details.node)); + } + + return header; + } + + /** + * Renders a line (text content, message, or placeholder) as a DOM element. + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LineDetails} lineDetails + * @return {Element} + */ + static renderSnippetLine( + dom, + tmpl, + {content, lineNumber, truncated, contentType, visibility} + ) { + const clonedTemplate = dom.cloneTemplate('#tmpl-lh-snippet__line', tmpl); + const contentLine = dom.find('.lh-snippet__line', clonedTemplate); + const {classList} = contentLine; + + classNamesByContentType[contentType].forEach(typeClass => + classList.add(typeClass) + ); + + if (visibility === LineVisibility.WHEN_COLLAPSED) { + classList.add('lh-snippet__show-if-collapsed'); + } else if (visibility === LineVisibility.WHEN_EXPANDED) { + classList.add('lh-snippet__show-if-expanded'); + } + + const lineContent = content + (truncated ? '…' : ''); + const lineContentEl = dom.find('.lh-snippet__line code', contentLine); + if (contentType === LineContentType.MESSAGE) { + lineContentEl.appendChild(dom.convertMarkdownLinkSnippets(lineContent)); + } else { + lineContentEl.textContent = lineContent; + } + + dom.find( + '.lh-snippet__line-number', + contentLine + ).textContent = lineNumber.toString(); + + return contentLine; + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {{message: string}} message + * @return {Element} + */ + static renderMessage(dom, tmpl, message) { + return SnippetRenderer.renderSnippetLine(dom, tmpl, { + lineNumber: ' ', + content: message.message, + contentType: LineContentType.MESSAGE, + }); + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LineVisibility} visibility + * @return {Element} + */ + static renderOmittedLinesPlaceholder(dom, tmpl, visibility) { + return SnippetRenderer.renderSnippetLine(dom, tmpl, { + lineNumber: '…', + content: '', + visibility, + contentType: LineContentType.PLACEHOLDER, + }); + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LH.Audit.Details.SnippetValue} details + * @return {DocumentFragment} + */ + static renderSnippetContent(dom, tmpl, details) { + const template = dom.cloneTemplate('#tmpl-lh-snippet__content', tmpl); + const snippetEl = dom.find('.lh-snippet__snippet-inner', template); + + // First render messages that don't belong to specific lines + details.generalMessages.forEach(m => + snippetEl.append(SnippetRenderer.renderMessage(dom, tmpl, m)) + ); + // Then render the lines and their messages, as well as placeholders where lines are omitted + snippetEl.append(SnippetRenderer.renderSnippetLines(dom, tmpl, details)); + + return template; + } + + /** + * @param {DOM} dom + * @param {DocumentFragment} tmpl + * @param {LH.Audit.Details.SnippetValue} details + * @returns {DocumentFragment} + */ + static renderSnippetLines(dom, tmpl, details) { + const {lineMessages, generalMessages, lineCount, lines} = details; + const linesWhenCollapsed = getLinesWhenCollapsed(details); + const hasOnlyGeneralMessages = + generalMessages.length > 0 && lineMessages.length === 0; + + const lineContainer = dom.createFragment(); + + // When a line is not shown in the collapsed state we try to see if we also need an + // omitted lines placeholder for the expanded state, rather than rendering two separate + // placeholders. + let hasPendingOmittedLinesPlaceholderForCollapsedState = false; + + for (let lineNumber = 1; lineNumber <= lineCount; lineNumber++) { + const {line, previousLine} = getLineAndPreviousLine(lines, lineNumber); + const { + line: lineWhenCollapsed, + previousLine: previousLineWhenCollapsed, + } = getLineAndPreviousLine(linesWhenCollapsed, lineNumber); + + const showLineWhenCollapsed = !!lineWhenCollapsed; + const showPreviousLineWhenCollapsed = !!previousLineWhenCollapsed; + + // If we went from showing lines in the collapsed state to not showing them + // we need to render a placeholder + if (showPreviousLineWhenCollapsed && !showLineWhenCollapsed) { + hasPendingOmittedLinesPlaceholderForCollapsedState = true; + } + // If we are back to lines being visible in the collapsed and the placeholder + // hasn't been rendered yet then render it now + if ( + showLineWhenCollapsed && + hasPendingOmittedLinesPlaceholderForCollapsedState + ) { + lineContainer.append( + SnippetRenderer.renderOmittedLinesPlaceholder( + dom, + tmpl, + LineVisibility.WHEN_COLLAPSED + ) + ); + hasPendingOmittedLinesPlaceholderForCollapsedState = false; + } + + // Render omitted lines placeholder if we have not already rendered one for this gap + const isFirstOmittedLineWhenExpanded = !line && !!previousLine; + const isFirstLineOverallAndIsOmittedWhenExpanded = + !line && lineNumber === 1; + if ( + isFirstOmittedLineWhenExpanded || + isFirstLineOverallAndIsOmittedWhenExpanded + ) { + // In the collapsed state we don't show omitted lines placeholders around + // the edges of the snippet + const hasRenderedAllLinesVisibleWhenCollapsed = !linesWhenCollapsed.some( + l => l.lineNumber > lineNumber + ); + const onlyShowWhenExpanded = + hasRenderedAllLinesVisibleWhenCollapsed || lineNumber === 1; + lineContainer.append( + SnippetRenderer.renderOmittedLinesPlaceholder( + dom, + tmpl, + onlyShowWhenExpanded + ? LineVisibility.WHEN_EXPANDED + : LineVisibility.ALWAYS + ) + ); + hasPendingOmittedLinesPlaceholderForCollapsedState = false; + } + + if (!line) { + // Can't render the line if we don't know its content (instead we've rendered a placeholder) + continue; + } + + // Now render the line and any messages + const messages = getMessagesForLineNumber(lineMessages, lineNumber); + const highlightLine = messages.length > 0 || hasOnlyGeneralMessages; + const contentLineDetails = Object.assign({}, line, { + contentType: highlightLine + ? LineContentType.CONTENT_HIGHLIGHTED + : LineContentType.CONTENT_NORMAL, + visibility: lineWhenCollapsed + ? LineVisibility.ALWAYS + : LineVisibility.WHEN_EXPANDED, + }); + lineContainer.append( + SnippetRenderer.renderSnippetLine(dom, tmpl, contentLineDetails) + ); + + messages.forEach(message => { + lineContainer.append(SnippetRenderer.renderMessage(dom, tmpl, message)); + }); + } + + return lineContainer; + } + + /** + * @param {DOM} dom + * @param {ParentNode} templateContext + * @param {LH.Audit.Details.SnippetValue} details + * @param {DetailsRenderer} detailsRenderer + * @return {Element} + */ + static render(dom, templateContext, details, detailsRenderer) { + const tmpl = dom.cloneTemplate('#tmpl-lh-snippet', templateContext); + const snippetEl = dom.find('.lh-snippet', tmpl); + + const header = SnippetRenderer.renderHeader( + dom, + tmpl, + details, + detailsRenderer, + () => snippetEl.classList.toggle('lh-snippet--expanded') + ); + const content = SnippetRenderer.renderSnippetContent(dom, tmpl, details); + snippetEl.append(header, content); + + return snippetEl; + } +} + +// Allow Node require()'ing. +if (typeof module !== 'undefined' && module.exports) { + module.exports = SnippetRenderer; +} else { + self.SnippetRenderer = SnippetRenderer; +} diff --git a/lighthouse-core/report/html/renderer/util.js b/lighthouse-core/report/html/renderer/util.js index 6a752d517f34..5da98dfaaa64 100644 --- a/lighthouse-core/report/html/renderer/util.js +++ b/lighthouse-core/report/html/renderer/util.js @@ -449,6 +449,48 @@ class Util { // When testing, use a locale with more exciting numeric formatting if (Util.numberDateLocale === 'en-XA') Util.numberDateLocale = 'de'; } + + /** + * Returns only lines that are near a message, or the first few lines if there are + * no line messages. + * @param {LH.Audit.Details.SnippetValue['lines']} lines + * @param {LH.Audit.Details.SnippetValue['lineMessages']} lineMessages + * @param {number} surroundingLineCount Number of lines to include before and after + * the message. If this is e.g. 2 this function might return 5 lines. + */ + static filterRelevantLines(lines, lineMessages, surroundingLineCount) { + if (lineMessages.length === 0) { + // no lines with messages, just return the first bunch of lines + return lines.slice(0, surroundingLineCount * 2 + 1); + } + + const minGapSize = 3; + const lineNumbersToKeep = new Set(); + // Sort messages so we can check lineNumbersToKeep to see how big the gap to + // the previous line is. + lineMessages = lineMessages.sort((a, b) => (a.lineNumber || 0) - (b.lineNumber || 0)); + lineMessages.forEach(({lineNumber}) => { + let firstSurroundingLineNumber = lineNumber - surroundingLineCount; + let lastSurroundingLineNumber = lineNumber + surroundingLineCount; + + while (firstSurroundingLineNumber < 1) { + // make sure we still show (surroundingLineCount * 2 + 1) lines in total + firstSurroundingLineNumber++; + lastSurroundingLineNumber++; + } + // If only a few lines would be omitted normally then we prefer to include + // extra lines to avoid the tiny gap + if (lineNumbersToKeep.has(firstSurroundingLineNumber - minGapSize - 1)) { + firstSurroundingLineNumber -= minGapSize; + } + for (let i = firstSurroundingLineNumber; i <= lastSurroundingLineNumber; i++) { + const surroundingLineNumber = i; + lineNumbersToKeep.add(surroundingLineNumber); + } + }); + + return lines.filter(line => lineNumbersToKeep.has(line.lineNumber)); + } } /** @@ -496,6 +538,11 @@ Util.UIStrings = { /** 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:', + /** Label for button that shows all lines of the snippet when clicked */ + snippetExpandButtonLabel: 'Expand snippet', + /** Label for button that only shows a few lines of the snippet when clicked */ + snippetCollapseButtonLabel: 'Collapse snippet', + /** Explanation shown to users below performance results to inform them that the test was done with a 4G 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. */ lsPerformanceCategoryDescription: '[Lighthouse](https://developers.google.com/web/tools/lighthouse/) analysis of the current page on an emulated mobile network. 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. */ diff --git a/lighthouse-core/report/html/report-styles.css b/lighthouse-core/report/html/report-styles.css index 8ece38dc6be7..7915e7bd05cf 100644 --- a/lighthouse-core/report/html/report-styles.css +++ b/lighthouse-core/report/html/report-styles.css @@ -725,6 +725,10 @@ margin-top: var(--section-padding); } +.lh-list > div:not(:last-child) { + padding-bottom: 20px; +} + .lh-header-container { display: block; margin: 0 auto; diff --git a/lighthouse-core/report/html/templates.html b/lighthouse-core/report/html/templates.html index 7b5362bec540..c63bc666fd38 100644 --- a/lighthouse-core/report/html/templates.html +++ b/lighthouse-core/report/html/templates.html @@ -892,3 +892,142 @@ + + + + + diff --git a/lighthouse-core/test/audits/audit-test.js b/lighthouse-core/test/audits/audit-test.js index 0e4251a2ca0d..b45cd2202fed 100644 --- a/lighthouse-core/test/audits/audit-test.js +++ b/lighthouse-core/test/audits/audit-test.js @@ -99,4 +99,112 @@ describe('Audit', () => { assert.equal(result.score, null); assert.equal(result.scoreDisplayMode, 'error'); }); + + describe('makeSnippetDetails', () => { + const maxLinesAroundMessage = 10; + + it('Transforms code to lines array', () => { + const details = Audit.makeSnippetDetails({ + content: 'a\nb\nc', + title: 'Title', + lineMessages: [], + generalMessages: [], + }); + + assert.equal(details.lines.length, 3); + assert.deepEqual(details.lines[1], { + lineNumber: 2, + content: 'b', + }); + }); + + it('Truncates long lines', () => { + const details = Audit.makeSnippetDetails({ + content: Array(1001).join('-'), + title: 'Title', + lineMessages: [], + generalMessages: [], + }); + + assert.equal(details.lines[0].truncated, true); + assert.ok(details.lines[0].content.length < 1000); + }); + + function makeLines(lineCount) { + return Array(lineCount + 1).join('-\n'); + } + + it('Limits the number of lines if there are no line messages', () => { + const details = Audit.makeSnippetDetails({ + content: makeLines(100), + title: 'Title', + lineMessages: [], + generalMessages: [{ + message: 'General', + }], + maxLinesAroundMessage, + }); + expect(details.lines.length).toBe(2 * maxLinesAroundMessage + 1); + }); + + it('Does not omit lines if fewer than 4 lines would be omitted', () => { + const details = Audit.makeSnippetDetails({ + content: makeLines(200), + title: 'Title', + lineMessages: [ + // without the special logic for small gaps lines 71-73 would be missing + { + // putting last message first to make sure makeSnippetDetails doesn't depend on order + lineNumber: 84, + message: 'Message 2', + }, { + lineNumber: 60, + message: 'Message 1', + }], + generalMessages: [], + maxLinesAroundMessage, + }); + + const normalExpectedLineNumber = 2 * (maxLinesAroundMessage * 2 + 1); + assert.equal(details.lines.length, normalExpectedLineNumber + 3); + }); + + it('Limits the number of lines around line messages', () => { + const content = makeLines(99) + 'A\n' + makeLines(99) + '\nB'; + const allLines = content.split('\n'); + const details = Audit.makeSnippetDetails({ + content, + title: 'Title', + lineMessages: [{ + lineNumber: allLines.findIndex(l => l === 'A') + 1, + message: 'a', + }, { + lineNumber: allLines.findIndex(l => l === 'B') + 1, + message: 'b', + }], + generalMessages: [], + maxLinesAroundMessage, + }); + + // 2 line messages and their surounding lines, second line with message only has preceding lines + const lineCount = maxLinesAroundMessage * 3 + 2; + assert.equal(details.lines.length, lineCount); + const lastLine = details.lines.slice(-1)[0]; + assert.deepEqual(lastLine, { + lineNumber: 201, + content: 'B', + }); + }); + }); + + describe('makeListDetails', () => { + it('Generates list details', () => { + const details = Audit.makeListDetails([1, 2, 3]); + + assert.deepEqual(details, { + type: 'list', + items: [1, 2, 3], + }); + }); + }); }); diff --git a/lighthouse-core/test/report/html/renderer/details-renderer-test.js b/lighthouse-core/test/report/html/renderer/details-renderer-test.js index 19f54df5dcf1..d348860c1dd7 100644 --- a/lighthouse-core/test/report/html/renderer/details-renderer-test.js +++ b/lighthouse-core/test/report/html/renderer/details-renderer-test.js @@ -12,6 +12,7 @@ const URL = require('../../../../lib/url-shim'); const DOM = require('../../../../report/html/renderer/dom.js'); const Util = require('../../../../report/html/renderer/util.js'); const DetailsRenderer = require('../../../../report/html/renderer/details-renderer.js'); +const SnippetRenderer = require('../../../../report/html/renderer/snippet-renderer.js'); const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../../report/html/templates.html', 'utf8'); @@ -24,14 +25,17 @@ describe('DetailsRenderer', () => { beforeAll(() => { global.URL = URL; global.Util = Util; + global.SnippetRenderer = SnippetRenderer; const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window; const dom = new DOM(document); renderer = new DetailsRenderer(dom); + renderer.setTemplateContext(dom._document); }); afterAll(() => { global.URL = undefined; global.Util = undefined; + global.SnippetRenderer = undefined; }); describe('render', () => { @@ -119,6 +123,27 @@ describe('DetailsRenderer', () => { '--thumbnail not set'); }); + it('renders lists', () => { + const snippet = { + type: 'snippet', + lines: [{lineNumber: 1, content: ''}], + title: 'Some snippet', + lineMessages: [], + generalMessages: [], + lineCount: 100, + }; + + const el = renderer.render({ + type: 'list', + items: [snippet, snippet], + }); + + assert.equal(el.localName, 'div'); + assert.ok(el.classList.contains('lh-list'), 'has list class'); + assert.ok(el.children.length, 2, 'renders all items'); + assert.ok(el.children[0].textContent.includes('Some snippet'), 'renders item content'); + }); + it('renders links', () => { const linkText = 'Example Site'; const linkUrl = 'https://example.com/'; diff --git a/lighthouse-core/test/report/html/renderer/snippet-renderer-test.js b/lighthouse-core/test/report/html/renderer/snippet-renderer-test.js new file mode 100644 index 000000000000..5e242cf56294 --- /dev/null +++ b/lighthouse-core/test/report/html/renderer/snippet-renderer-test.js @@ -0,0 +1,293 @@ +/** + * @license Copyright 2019 Google Inc. All Rights Reserved. + * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 + * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. + */ +'use strict'; + +/* eslint-env jest */ + +const assert = require('assert'); +const fs = require('fs'); +const jsdom = require('jsdom'); +const Util = require('../../../../report/html/renderer/util.js'); +const DOM = require('../../../../report/html/renderer/dom.js'); +const SnippetRenderer = require('../../../../report/html/renderer/snippet-renderer.js'); + +const TEMPLATE_FILE = fs.readFileSync( + __dirname + '/../../../../report/html/templates.html', + 'utf8' +); + +/* Generates a snippet lines array like this (for a single range from 1 to 4): + [ + {number: 1, content 'L1'}, + {number: 2, content 'L2'}, + {number: 3, content 'L3'}, + {number: 4, content 'L4'} + ]; +*/ +function generateAvailableLinesArray(availableLineRanges) { + const lines = []; + availableLineRanges.forEach(({from, to}) => { + for (let i = from; i <= to; i++) { + lines.push({ + lineNumber: i, + content: 'L' + i, + }); + } + }); + return lines; +} + +function makeSnippetDetails({ + lineMessages, + generalMessages, + lines = null, + title = 'Snippet', + lineCount, +}) { + return { + type: 'snippet', + title: title, + lines: lines, + lineMessages, + generalMessages, + lineCount, + }; +} + +describe('DetailsRenderer', () => { + let dom; + + beforeAll(() => { + global.Util = Util; + const {document} = new jsdom.JSDOM(TEMPLATE_FILE).window; + dom = new DOM(document); + }); + + afterAll(() => { + global.Util = undefined; + }); + + function renderSnippet(details) { + const el = SnippetRenderer.render(dom, dom.document(), details, {}); + + return { + el, + contentLines: el.querySelectorAll('.lh-snippet__line--content'), + highlightedContentLines: el.querySelectorAll('.lh-snippet__line--content-highlighted'), + collapsedContentLines: el.querySelectorAll( + '.lh-snippet__line--content.lh-snippet__show-if-expanded' + ), + uncollapsedContentLines: el.querySelectorAll( + '.lh-snippet__line--content:not(.lh-snippet__show-if-expanded)' + ), + messageLines: el.querySelectorAll('.lh-snippet__line--message'), + omittedLinesIndicatorsWhenExpanded: el.querySelectorAll( + '.lh-snippet__line--placeholder:not(.lh-snippet__show-if-collapsed)' + ), + omittedLinesIndicatorsWhenCollapsed: el.querySelectorAll( + '.lh-snippet__line--placeholder:not(.lh-snippet__show-if-expanded)' + ), + title: el.querySelector('.lh-snippet__title'), + toggleExpandButton: el.querySelector('.lh-snippet__toggle-expand'), + }; + } + + it('Renders snippet with a message at the very top', () => { + const details = makeSnippetDetails({ + lineMessages: [ + { + lineNumber: 1, + message: 'Error', + }, + ], + generalMessages: [], + lines: generateAvailableLinesArray([{from: 1, to: 6}]), + lineCount: 100, + }); + const {contentLines, messageLines, collapsedContentLines} = renderSnippet(details); + + // 5 lines are visible, 1 is collapsed + assert.equal(collapsedContentLines.length, 1); + // All available lines are shown on expansion + assert.equal(contentLines.length, 6); + // 100 lines in total, so lines towards the end won't be shown + const lastLine = contentLines[contentLines.length - 1]; + assert.equal(lastLine.nextSibling.textContent.trim(), '…'); + + // Shows message for second line + assert.equal(messageLines[0].textContent.trim(), 'Error'); + assert.equal(messageLines[0].previousSibling.textContent.replace(/\s/g, ''), '1L1'); + }); + + it('Renders first few lines if there are no messages', () => { + const details = makeSnippetDetails({ + lineMessages: [], + generalMessages: [], + lines: generateAvailableLinesArray([{from: 1, to: 6}]), + lineCount: 100, + }); + const { + uncollapsedContentLines, + omittedLinesIndicatorsWhenExpanded, + omittedLinesIndicatorsWhenCollapsed, + highlightedContentLines, + } = renderSnippet(details); + const lastUncollapsedLine = uncollapsedContentLines[uncollapsedContentLines.length - 1]; + + // Shows first 5 visible lines + assert.equal(lastUncollapsedLine.textContent.replace(/\s/g, ''), '5L5'); + // "..." after the available lines, but only shows in expanded state + assert.equal(omittedLinesIndicatorsWhenExpanded.length, 1); + assert.equal(omittedLinesIndicatorsWhenCollapsed.length, 0); + // nothing is highlighted + assert.equal(highlightedContentLines.length, 0); + }); + + it('Renders first few lines if there are no messages for specific lines', () => { + const details = makeSnippetDetails({ + lineMessages: [], + generalMessages: [ + { + message: 'General error', + }, + ], + lines: generateAvailableLinesArray([{from: 1, to: 6}]), + lineCount: 100, + }); + const {uncollapsedContentLines, messageLines, highlightedContentLines} = renderSnippet(details); + const lastUncollapsedLine = uncollapsedContentLines[uncollapsedContentLines.length - 1]; + + // Shows message + assert.equal(messageLines.length, 1); + + // Shows first 5 visible lines + assert.equal(lastUncollapsedLine.textContent.replace(/\s/g, ''), '5L5'); + + // highlight everything (i.e. the 6 lines that are rendered) + assert.equal(highlightedContentLines.length, 6); + }); + + it('Renders snippet with multiple messages surrounded by other lines', () => { + const details = makeSnippetDetails({ + lineMessages: [ + { + lineNumber: 40, + message: 'Error 1', + }, + { + lineNumber: 70, + message: 'Error 2', + }, + ], + generalMessages: [], + lines: generateAvailableLinesArray([ + { + from: 30, + to: 50, + }, + { + from: 60, + to: 80, + }, + ]), + lineCount: 100, + }); + const { + collapsedContentLines, + omittedLinesIndicatorsWhenCollapsed, + omittedLinesIndicatorsWhenExpanded, + highlightedContentLines, + } = renderSnippet(details); + + // first available line is collapsed + assert.equal(collapsedContentLines[0].textContent.replace(/\s/g, ''), '30L30'); + + // puts omitted lines placeholder between the two messages + assert.equal(omittedLinesIndicatorsWhenCollapsed.length, 1); + // puts omitted lines placeholder between the two messages and around the whole snippet + assert.equal(omittedLinesIndicatorsWhenExpanded.length, 3); + + // both lines with messages are highlighted + assert.equal(highlightedContentLines.length, 2); + }); + + it('Can render both line-specific and non line-specific messages in one snippet', () => { + const details = makeSnippetDetails({ + lineMessages: [ + { + lineNumber: 5, + message: 'Error on line', + }, + ], + generalMessages: [ + { + message: 'General error', + }, + ], + lines: generateAvailableLinesArray([{from: 1, to: 6}]), + lineCount: 100, + }); + const {messageLines} = renderSnippet(details); + + assert.equal(messageLines.length, 2); + }); + + it('Renders a snippet header and allows toggling the expanded state', () => { + const details = makeSnippetDetails({ + title: 'Test Snippet', + lineMessages: [], + generalMessages: [], + lines: generateAvailableLinesArray([{from: 1, to: 6}]), + lineCount: 100, + }); + const {title, toggleExpandButton, el} = renderSnippet(details); + + // Renders title + assert.ok(title.textContent.includes('Test Snippet')); + // Renders toggle button + assert.ok(toggleExpandButton); + assert.ok(!el.classList.contains('lh-snippet--expanded')); + + toggleExpandButton.click(); + assert.ok(el.classList.contains('lh-snippet--expanded')); + }); + + it('Does not render toggle button if all available lines are already visible', () => { + const details = makeSnippetDetails({ + title: 'Test Snippet', + lineMessages: [], + generalMessages: [], + // We show all 5 lines by default, so there's nothing to expand + lines: generateAvailableLinesArray([{from: 1, to: 5}]), + }); + const {toggleExpandButton} = renderSnippet(details); + + assert.ok(!toggleExpandButton); + }); + + it('Adds ... to lines that have been truncated', () => { + const details = makeSnippetDetails({ + lineMessages: [], + generalMessages: [], + lines: [ + { + content: 'abc', + lineNumber: 1, + truncated: true, + }, + { + content: 'xyz', + lineNumber: 2, + }, + ], + lineCount: 2, + }); + const {contentLines} = renderSnippet(details); + + assert.ok(contentLines[0].textContent.includes('…')); + assert.ok(!contentLines[1].textContent.includes('…')); + }); +}); diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index c0315003d8ca..3153f3cbdb3e 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -4824,6 +4824,8 @@ "opportunitySavingsColumnLabel": "Estimated Savings", "passedAuditsGroupTitle": "Passed audits", "scorescaleLabel": "Score scale:", + "snippetCollapseButtonLabel": "Collapse snippet", + "snippetExpandButtonLabel": "Expand snippet", "toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:", "varianceDisclaimer": "Values are estimated and may vary.", "warningAuditsGroupTitle": "Passed audits but with warnings", diff --git a/package.json b/package.json index 23e6564f43bb..99831c2e16a7 100644 --- a/package.json +++ b/package.json @@ -187,7 +187,7 @@ }, { "path": "./dist/viewer/src/viewer.js", - "threshold": "65 Kb" + "threshold": "70 Kb" } ], "nyc": { diff --git a/proto/lighthouse-result.proto b/proto/lighthouse-result.proto index df0658e7e92a..fcc45bcb1ee6 100644 --- a/proto/lighthouse-result.proto +++ b/proto/lighthouse-result.proto @@ -334,6 +334,12 @@ message I18n { // The heading that is shown above a list of audits that have warnings string warning_audits_group_title = 17; + + // The label for the button to show all lines of a snippet + string snippet_expand_button_label = 18; + + // The label for the button to show only a few lines of a snippet + string snippet_collapse_button_label = 19; } // The message holding all formatted strings diff --git a/proto/sample_v2_round_trip.json b/proto/sample_v2_round_trip.json index 3e1c5d4c846a..0b6944e448e7 100644 --- a/proto/sample_v2_round_trip.json +++ b/proto/sample_v2_round_trip.json @@ -3726,6 +3726,8 @@ "opportunitySavingsColumnLabel": "Estimated Savings", "passedAuditsGroupTitle": "Passed audits", "scorescaleLabel": "Score scale:", + "snippetCollapseButtonLabel": "Collapse snippet", + "snippetExpandButtonLabel": "Expand snippet", "toplevelWarningsMessage": "There were issues affecting this run of Lighthouse:", "varianceDisclaimer": "Values are estimated and may vary.", "warningAuditsGroupTitle": "Passed audits but with warnings", diff --git a/types/audit-details.d.ts b/types/audit-details.d.ts index f2dd8a329ee0..50c2e9565ffb 100644 --- a/types/audit-details.d.ts +++ b/types/audit-details.d.ts @@ -65,6 +65,11 @@ declare global { diagnostic?: Diagnostic; } + export interface List { + type: 'list'; + items: SnippetValue[] + } + /** * A details type that is not rendered in the final report; usually used * for including diagnostic information in the LHR. Can contain anything. @@ -164,6 +169,38 @@ declare global { type: 'url'; value: string; } + + /** + * Snippet of text with line numbers and annotations. + */ + export interface SnippetValue { + type: 'snippet', + title: string, + /** Node where the content of this snippet came from. */ + node?: NodeValue, + /** + * The lines that should be rendered. For long snippets we only include important lines + * in the audit result. + */ + lines: { + content: string + /** Line number, starting from 1. */ + lineNumber: number; + truncated?: boolean + }[], + /** The total number of lines in the snippet, equal to lines.length for short snippets. */ + lineCount: number, + /** Messages that provide information about a specific lines. */ + lineMessages: { + /** Line number, starting from 1. */ + lineNumber: number, + message: string + }[]; + /** Messages that provide information about the snippet in general. */ + generalMessages: { + message: string + }[]; + } } } } diff --git a/types/html-renderer.d.ts b/types/html-renderer.d.ts index 6dcc65d110a8..453b24399eba 100644 --- a/types/html-renderer.d.ts +++ b/types/html-renderer.d.ts @@ -6,6 +6,7 @@ import _CategoryRenderer = require('../lighthouse-core/report/html/renderer/category-renderer.js'); import _CriticalRequestChainRenderer = require('../lighthouse-core/report/html/renderer/crc-details-renderer.js'); +import _SnippetRenderer = require('../lighthouse-core/report/html/renderer/snippet-renderer.js'); import _DetailsRenderer = require('../lighthouse-core/report/html/renderer/details-renderer.js'); import _DOM = require('../lighthouse-core/report/html/renderer/dom.js'); import _PerformanceCategoryRenderer = require('../lighthouse-core/report/html/renderer/performance-category-renderer.js'); @@ -19,6 +20,7 @@ import _FileNamer = require('../lighthouse-core/lib/file-namer.js'); declare global { var CategoryRenderer: typeof _CategoryRenderer; var CriticalRequestChainRenderer: typeof _CriticalRequestChainRenderer; + var SnippetRenderer: typeof _SnippetRenderer; var DetailsRenderer: typeof _DetailsRenderer; var DOM: typeof _DOM; var getFilenamePrefix: typeof _FileNamer.getFilenamePrefix; @@ -32,6 +34,7 @@ declare global { interface Window { CategoryRenderer: typeof _CategoryRenderer; CriticalRequestChainRenderer: typeof _CriticalRequestChainRenderer; + SnippetRenderer: typeof _SnippetRenderer; DetailsRenderer: typeof _DetailsRenderer; DOM: typeof _DOM; PerformanceCategoryRenderer: typeof _PerformanceCategoryRenderer;