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 v2: check/x boolean audits, expand details failing audits, optimal val #1963

Merged
merged 10 commits into from
Apr 7, 2017
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
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ coverage

lighthouse-cli/results
lighthouse-cli/performance-experiment/experiment-database/experiment-data*
results
results.html
last-run-results.html

Expand Down
13 changes: 12 additions & 1 deletion docs/hacking-tips.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,20 @@ Use [`--trace-warnings`](https://medium.com/@jasnell/introducing-process-warning
node --trace-warnings lighthouse-cli http://example.com
```

## Updating fixture dumps

`lighthouse-core/test/results/samples_v2.json` is generated from running LH against
dbw_tester.html. To update this file, run:

```sh
npm run start -- --output=json --output-path=lighthouse-core/test/results/sample_v2.json http://localhost:8080/dobetterweb/dbw_tester.html
```

After updating, consider deleting any irrelevant changes from the diff (exact timings, timestamps, etc). Be sure to run the tests.

## Iterating on the v2 report

This'll generate new reports from the same results json.
This will generate new reports from the same results json.
Copy link
Member

Choose a reason for hiding this comment

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

harsh


```sh
# capture some results first:
Expand Down
9 changes: 5 additions & 4 deletions lighthouse-core/report/v2/report-generator.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
'use strict';

const fs = require('fs');
const path = require('path');

const REPORT_TEMPLATE = fs.readFileSync(path.join(__dirname, './report-template.html'), 'utf8');
const REPORT_TEMPLATE = fs.readFileSync(__dirname + '/report-template.html', 'utf8');
// TODO: Setup a gulp pipeline to concat and minify the renderer files?
const REPORT_JAVASCRIPT = fs.readFileSync(path.join(__dirname, './report-renderer.js'), 'utf8');
const REPORT_CSS = fs.readFileSync(path.join(__dirname, './report-styles.css'), 'utf8');
const REPORT_JAVASCRIPT = fs.readFileSync(__dirname + '/report-renderer.js', 'utf8');
const REPORT_CSS = fs.readFileSync(__dirname + '/report-styles.css', 'utf8');
const REPORT_TEMPLATES = fs.readFileSync(__dirname + '/templates.html', 'utf8');

class ReportGeneratorV2 {
/**
Expand Down Expand Up @@ -102,6 +102,7 @@ class ReportGeneratorV2 {
{search: '%%LIGHTHOUSE_JSON%%', replacement: sanitizedJson},
{search: '%%LIGHTHOUSE_JAVASCRIPT%%', replacement: sanitizedJavascript},
{search: '/*%%LIGHTHOUSE_CSS%%*/', replacement: REPORT_CSS},
{search: '%%LIGHTHOUSE_TEMPLATES%%', replacement: REPORT_TEMPLATES},
]);
}
}
Expand Down
130 changes: 88 additions & 42 deletions lighthouse-core/report/v2/report-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,31 @@
*/
'use strict';

/**
* @fileoverview The entry point for rendering the Lighthouse report based on the JSON output.
* This file is injected into the report HTML along with the JSON report.
*
* Dummy text for ensuring report robustness: </script> pre$`post %%LIGHTHOUSE_JSON%%
*/

/* eslint-env browser */
/* eslint indent: [2, 2, { "SwitchCase": 1, "outerIIFEBody": 0 }] */

(function() {
const RATINGS = {
GOOD: {label: 'good', minScore: 75},
PASS: {label: 'pass', minScore: 75},
AVERAGE: {label: 'average', minScore: 45},
POOR: {label: 'poor'}
FAIL: {label: 'fail'}
Copy link
Member

Choose a reason for hiding this comment

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

does PASS/FAIL work here? Seems weird to have a three state pass/fail system :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really a pass/fail system. Just for labeling/coloring right now. We'll probably need to expand in the future if we support things like grades. Seems fine for now.

Copy link
Member

Choose a reason for hiding this comment

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

Not really a pass/fail system. Just for labeling/coloring right now.

yeah, I was mostly just commenting on the names since PASS/FAIL is usually a binary state. Fine for now

};

/**
* Convert a score to a rating label.
* @param {number} value
* @param {number} score
* @return {string}
*/
function calculateRating(value) {
let rating = RATINGS.POOR.label;
if (value >= RATINGS.GOOD.minScore) {
rating = RATINGS.GOOD.label;
} else if (value >= RATINGS.AVERAGE.minScore) {
function calculateRating(score) {
let rating = RATINGS.FAIL.label;
if (score >= RATINGS.PASS.minScore) {
rating = RATINGS.PASS.label;
} else if (score >= RATINGS.AVERAGE.minScore) {
rating = RATINGS.AVERAGE.label;
}
return rating;
Expand All @@ -49,13 +54,7 @@ function formatNumber(number) {
return number.toLocaleString(undefined, {maximumFractionDigits: 1});
}

/**
* @fileoverview The entry point for rendering the Lighthouse report based on the JSON output.
* This file is injected into the report HTML along with the JSON report.
*
* Dummy text for ensuring report robustness: </script> pre$`post %%LIGHTHOUSE_JSON%%
*/
window.ReportRenderer = class ReportRenderer {
class ReportRenderer {
/**
* @param {!Document} document
*/
Expand All @@ -75,7 +74,7 @@ window.ReportRenderer = class ReportRenderer {
}
}

/**
/**
* @param {string} name
* @param {string=} className
* @param {!Object<string, string>=} attrs Attribute key/val pairs.
Expand All @@ -86,33 +85,84 @@ window.ReportRenderer = class ReportRenderer {
if (className) {
element.className = className;
}
for (const [key, val] of Object.entries(attrs)) {
element.setAttribute(key, val);
}
Object.keys(attrs).forEach(key => {
Copy link
Member

Choose a reason for hiding this comment

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

I knew we'd be running this in node before too long :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1

element.setAttribute(key, attrs[key]);
});
return element;
}

/**
* @param {string} selector
* @return {!DocumentFragment} A clone of the template content.
* @throws {Error}
*/
_cloneTemplate(selector) {
const template = this._document.querySelector(selector);
if (!template) {
throw new Error(`Template not found: template${selector}`);
}
return this._document.importNode(template.content, true);
}

/**
* @param {!DocumentFragment|!Element} element DOM node to populate with values.
* @param {number} score
* @param {string} scoringMode
* @param {string} title
* @param {string} description
* @return {!Element}
*/
_renderScore(score, title, description) {
const element = this._createElement('div', 'lighthouse-score');
_populateScore(element, score, scoringMode, title, description) {
// Fill in the blanks.
const valueEl = element.querySelector('.lighthouse-score__value');
valueEl.textContent = formatNumber(score);
valueEl.classList.add(`lighthouse-score__value--${calculateRating(score)}`,
`lighthouse-score__value--${scoringMode}`);

const value = element.appendChild(this._createElement('div', 'lighthouse-score__value'));
value.textContent = formatNumber(score);
value.classList.add(`lighthouse-score__value--${calculateRating(score)}`);

const column = element.appendChild(this._createElement('div', 'lighthouse-score__text'));
column.appendChild(this._createElement('div', 'lighthouse-score__title')).textContent = title;
column.appendChild(
this._createElement('div', 'lighthouse-score__description')).textContent = description;
element.querySelector('.lighthouse-score__title').textContent = title;
element.querySelector('.lighthouse-score__description').textContent = description;

return element;
}

/**
* @param {!AuditJSON} audit
* @return {!Element}
*/
_renderAuditScore(audit) {
const tmpl = this._cloneTemplate('#tmpl-lighthouse-audit-score');

const scoringMode = audit.result.scoringMode;
const description = audit.result.helpText;
let title = audit.result.description;

if (audit.result.displayValue) {
title += `: ${audit.result.displayValue}`;
}
if (audit.result.optimalValue) {
title += ` (target: ${audit.result.optimalValue})`;
}

// Append audit details to header section so the entire audit is within a <details>.
const header = tmpl.querySelector('.lighthouse-score__header');
header.open = audit.score < 100; // expand failed audits
if (audit.result.details) {
header.appendChild(this._renderDetails(audit.result.details));
}

return this._populateScore(tmpl, audit.score, scoringMode, title, description);
}

/**
* @param {!CategoryJSON} category
* @return {!Element}
*/
_renderCategoryScore(category) {
Copy link
Member

Choose a reason for hiding this comment

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

should these also be _renderAuditResult and _renderCategoryResult?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm actually going to change "_populateResult" back to _populateScore. It's populating the score dom with values.

Copy link
Member

Choose a reason for hiding this comment

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

I'm actually going to change _populateResult back to _populateScore. It's populating the score dom with values

my issue was that if someone was going into the codebase to see where audits' titles and descriptions are set, they probably wouldn't think it would be in _populateScore. If the issue is the template's id, that can change too :P

This is all going to change a lot in the near future, so if you feel strongly we can defer

const tmpl = this._cloneTemplate('#tmpl-lighthouse-category-score');
const score = Math.round(category.score);
return this._populateScore(tmpl, score, 'numeric', category.name, category.description);
}

/**
* @param {!DetailsJSON} details
* @return {!Element}
Expand Down Expand Up @@ -200,7 +250,7 @@ window.ReportRenderer = class ReportRenderer {
*/
_renderCategory(category) {
const element = this._createElement('div', 'lighthouse-category');
element.appendChild(this._renderScore(category.score, category.name, category.description));
element.appendChild(this._renderCategoryScore(category));
for (const audit of category.audits) {
element.appendChild(this._renderAudit(audit));
}
Expand All @@ -213,18 +263,14 @@ window.ReportRenderer = class ReportRenderer {
*/
_renderAudit(audit) {
const element = this._createElement('div', 'lighthouse-audit');
let title = audit.result.description;
if (audit.result.displayValue) {
title += ': ' + audit.result.displayValue;
}
element.appendChild(this._renderScore(audit.score, title, audit.result.helpText));
if (audit.result.details) {
element.appendChild(this._renderDetails(audit.result.details));
}
element.appendChild(this._renderAuditScore(audit));
return element;
}
};
})();
}

if (typeof module !== 'undefined' && module.exports) {
module.exports = ReportRenderer;
}

/** @typedef {{type: string, text: string|undefined, header: DetailsJSON|undefined, items: Array<DetailsJSON>|undefined}} */
let DetailsJSON; // eslint-disable-line no-unused-vars
Expand Down
Loading