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

core(details-renderer): snippet details renderer type #6999

Merged
merged 20 commits into from
Feb 20, 2019
Merged
Show file tree
Hide file tree
Changes from 16 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
66 changes: 66 additions & 0 deletions lighthouse-core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.Snippet['lineMessages'];
* generalMessages: LH.Audit.Details.Snippet['generalMessages'];
* node?: LH.Audit.Details.NodeValue;
* maxLineLength?: number;
* maxLinesAroundMessage?: number;
* }} SnippetInfo */
/**
* @param {SnippetInfo} snippetInfo
* @return {LH.Audit.Details.Snippet}
*/
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.Snippet['lines']}
*/
static _makeSnippetLinesArray(content, maxLineLength) {
return content.split('\n').map((line, lineIndex) => {
const lineNumber = lineIndex + 1;
/** @type LH.Audit.Details.Snippet['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
Expand Down
8 changes: 8 additions & 0 deletions lighthouse-core/lib/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -1135,6 +1135,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."
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/report/html/html-report-assets.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'),
Expand Down
34 changes: 32 additions & 2 deletions lighthouse-core/report/html/renderer/details-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
Expand All @@ -43,7 +43,7 @@ class DetailsRenderer {
}

/**
* @param {DetailsJSON|OpportunityDetails} details
* @param {DetailsJSON|OpportunityDetails|LH.Audit.Details.Snippet} details
* @return {Element|null}
*/
render(details) {
Expand All @@ -68,6 +68,15 @@ class DetailsRenderer {
case 'table':
// @ts-ignore - TODO(bckenny): Fix type hierarchy
return this._renderTable(/** @type {TableDetailsJSON} */ (details));
case 'list':
return this._renderList(
// @ts-ignore
mattzeunert marked this conversation as resolved.
Show resolved Hide resolved
/** @type {LH.Audit.Details.List} */ (details)
);
case 'snippet':
Copy link
Member

Choose a reason for hiding this comment

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

let's not add snippet here (it's just going to be immediately removed anyways)

return SnippetRenderer.render(this._dom, this._templateContext,
// @ts-ignore
/** @type {LH.Audit.Details.Snippet} */ (details), this);
case 'code':
return this._renderCode(/** @type {DetailsJSON} */ (details));
case 'node':
Expand Down Expand Up @@ -210,6 +219,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 => {
// @ts-ignore TODO(bckenny): this can never be null
listContainer.appendChild(this.render(item));
Copy link
Member

Choose a reason for hiding this comment

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

rather than call this.render(item), just call SnippetRenderer.render() directly (then also don't have to deal with null)

});

return listContainer;
}

/**
* @param {TableDetailsJSON} details
* @return {Element}
Expand Down Expand Up @@ -455,6 +479,12 @@ if (typeof module !== 'undefined' && module.exports) {
}} TableDetailsJSON
*/

/** @typedef {{
type: string,
items: Array<DetailsJSON>
}} ListDetailsJSON
*/
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 this can be deleted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup!


/** @typedef {{
type: string,
value: string,
Expand Down
Loading