From 0e1a4081279a9b151fae9283cc322e1b477e3535 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Mon, 10 Apr 2017 14:14:32 -0700 Subject: [PATCH 1/3] refactor: split report-renderer into multiple files --- .../report/v2/renderer/details-renderer.js | 91 ++++++++++++++ lighthouse-core/report/v2/renderer/dom.js | 59 ++++++++++ .../v2/{ => renderer}/report-renderer.js | 111 ++---------------- lighthouse-core/report/v2/report-generator.js | 7 +- .../v2/{ => renderer}/report-renderer-test.js | 61 ++++++---- 5 files changed, 204 insertions(+), 125 deletions(-) create mode 100644 lighthouse-core/report/v2/renderer/details-renderer.js create mode 100644 lighthouse-core/report/v2/renderer/dom.js rename lighthouse-core/report/v2/{ => renderer}/report-renderer.js (62%) rename lighthouse-core/test/report/v2/{ => renderer}/report-renderer-test.js (81%) diff --git a/lighthouse-core/report/v2/renderer/details-renderer.js b/lighthouse-core/report/v2/renderer/details-renderer.js new file mode 100644 index 000000000000..936fdc04f6e6 --- /dev/null +++ b/lighthouse-core/report/v2/renderer/details-renderer.js @@ -0,0 +1,91 @@ +/** + * Copyright 2017 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'; + +class DetailsRenderer { + /** + * @param {!DOM} dom + */ + constructor(dom) { + this._dom = dom; + } + + /** + * @param {!DetailsJSON} details + * @return {!Element} + */ + render(details) { + switch (details.type) { + case 'text': + return this._renderText(details); + case 'block': + return this._renderBlock(details); + case 'list': + return this._renderList(details); + default: + throw new Error(`Unknown type: ${details.type}`); + } + } + + /** + * @param {!DetailsJSON} text + * @return {!Element} + */ + _renderText(text) { + const element = this._dom.createElement('div', 'lighthouse-text'); + element.textContent = text.text; + return element; + } + + /** + * @param {!DetailsJSON} block + * @return {!Element} + */ + _renderBlock(block) { + const element = this._dom.createElement('div', 'lighthouse-block'); + for (const item of block.items) { + element.appendChild(this.render(item)); + } + return element; + } + + /** + * @param {!DetailsJSON} list + * @return {!Element} + */ + _renderList(list) { + const element = this._dom.createElement('details', 'lighthouse-list'); + if (list.header) { + const summary = this._dom.createElement('summary', 'lighthouse-list__header'); + summary.textContent = list.header.text; + element.appendChild(summary); + } + + const items = this._dom.createElement('div', 'lighthouse-list__items'); + for (const item of list.items) { + items.appendChild(this.render(item)); + } + element.appendChild(items); + return element; + } +} + +if (typeof module !== 'undefined' && module.exports) { + module.exports = DetailsRenderer; +} + +/** @typedef {{type: string, text: string|undefined, header: DetailsJSON|undefined, items: Array|undefined}} */ +let DetailsJSON; // eslint-disable-line no-unused-vars diff --git a/lighthouse-core/report/v2/renderer/dom.js b/lighthouse-core/report/v2/renderer/dom.js new file mode 100644 index 000000000000..2ad200bfb963 --- /dev/null +++ b/lighthouse-core/report/v2/renderer/dom.js @@ -0,0 +1,59 @@ +/** + * Copyright 2017 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'; + +class DOM { + /** + * @param {!Document} document + */ + constructor(document) { + this._document = document; + } + + /** + * @param {string} name + * @param {string=} className + * @param {!Object=} attrs Attribute key/val pairs. + * @return {!Element} + */ + createElement(name, className, attrs = {}) { + const element = this._document.createElement(name); + if (className) { + element.className = className; + } + Object.keys(attrs).forEach(key => { + 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); + } +} + +if (typeof module !== 'undefined' && module.exports) { + module.exports = DOM; +} diff --git a/lighthouse-core/report/v2/report-renderer.js b/lighthouse-core/report/v2/renderer/report-renderer.js similarity index 62% rename from lighthouse-core/report/v2/report-renderer.js rename to lighthouse-core/report/v2/renderer/report-renderer.js index 531bd2ffa412..e232f8190842 100644 --- a/lighthouse-core/report/v2/report-renderer.js +++ b/lighthouse-core/report/v2/renderer/report-renderer.js @@ -22,7 +22,7 @@ * Dummy text for ensuring report robustness: pre$`post %%LIGHTHOUSE_JSON%% */ -/* eslint-env browser */ +/* globals DOM, DetailsRenderer */ const RATINGS = { PASS: {label: 'pass', minScore: 75}, @@ -59,7 +59,8 @@ class ReportRenderer { * @param {!Document} document */ constructor(document) { - this._document = document; + this._dom = new DOM(document); + this._detailsRenderer = new DetailsRenderer(this._dom); } /** @@ -74,36 +75,6 @@ class ReportRenderer { } } - /** - * @param {string} name - * @param {string=} className - * @param {!Object=} attrs Attribute key/val pairs. - * @return {!Element} - */ - _createElement(name, className, attrs = {}) { - const element = this._document.createElement(name); - if (className) { - element.className = className; - } - Object.keys(attrs).forEach(key => { - 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 @@ -130,7 +101,7 @@ class ReportRenderer { * @return {!Element} */ _renderAuditScore(audit) { - const tmpl = this._cloneTemplate('#tmpl-lighthouse-audit-score'); + const tmpl = this._dom.cloneTemplate('#tmpl-lighthouse-audit-score'); const scoringMode = audit.result.scoringMode; const description = audit.result.helpText; @@ -147,7 +118,7 @@ class ReportRenderer { 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)); + header.appendChild(this._detailsRenderer.render(audit.result.details)); } return this._populateScore(tmpl, audit.score, scoringMode, title, description); @@ -158,76 +129,17 @@ class ReportRenderer { * @return {!Element} */ _renderCategoryScore(category) { - const tmpl = this._cloneTemplate('#tmpl-lighthouse-category-score'); + const tmpl = this._dom.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} - */ - _renderDetails(details) { - switch (details.type) { - case 'text': - return this._renderText(details); - case 'block': - return this._renderBlock(details); - case 'list': - return this._renderList(details); - default: - throw new Error(`Unknown type: ${details.type}`); - } - } - - /** - * @param {!DetailsJSON} text - * @return {!Element} - */ - _renderText(text) { - const element = this._createElement('div', 'lighthouse-text'); - element.textContent = text.text; - return element; - } - - /** - * @param {!DetailsJSON} block - * @return {!Element} - */ - _renderBlock(block) { - const element = this._createElement('div', 'lighthouse-block'); - for (const item of block.items) { - element.appendChild(this._renderDetails(item)); - } - return element; - } - - /** - * @param {!DetailsJSON} list - * @return {!Element} - */ - _renderList(list) { - const element = this._createElement('details', 'lighthouse-list'); - if (list.header) { - const summary = this._createElement('summary', 'lighthouse-list__header'); - summary.textContent = list.header.text; - element.appendChild(summary); - } - - const items = this._createElement('div', 'lighthouse-list__items'); - for (const item of list.items) { - items.appendChild(this._renderDetails(item)); - } - element.appendChild(items); - return element; - } - /** * @param {!Error} e * @return {!Element} */ _renderException(e) { - const element = this._createElement('div', 'lighthouse-exception'); + const element = this._dom.createElement('div', 'lighthouse-exception'); element.textContent = String(e.stack); return element; } @@ -237,7 +149,7 @@ class ReportRenderer { * @return {!Element} */ _renderReport(report) { - const element = this._createElement('div', 'lighthouse-report'); + const element = this._dom.createElement('div', 'lighthouse-report'); for (const category of report.reportCategories) { element.appendChild(this._renderCategory(category)); } @@ -249,7 +161,7 @@ class ReportRenderer { * @return {!Element} */ _renderCategory(category) { - const element = this._createElement('div', 'lighthouse-category'); + const element = this._dom.createElement('div', 'lighthouse-category'); element.appendChild(this._renderCategoryScore(category)); for (const audit of category.audits) { element.appendChild(this._renderAudit(audit)); @@ -262,7 +174,7 @@ class ReportRenderer { * @return {!Element} */ _renderAudit(audit) { - const element = this._createElement('div', 'lighthouse-audit'); + const element = this._dom.createElement('div', 'lighthouse-audit'); element.appendChild(this._renderAuditScore(audit)); return element; } @@ -272,9 +184,6 @@ if (typeof module !== 'undefined' && module.exports) { module.exports = ReportRenderer; } -/** @typedef {{type: string, text: string|undefined, header: DetailsJSON|undefined, items: Array|undefined}} */ -let DetailsJSON; // eslint-disable-line no-unused-vars - /** @typedef {{id: string, weight: number, score: number, result: {description: string, displayValue: string, helpText: string, score: number|boolean, details: DetailsJSON|undefined}}} */ let AuditJSON; // eslint-disable-line no-unused-vars diff --git a/lighthouse-core/report/v2/report-generator.js b/lighthouse-core/report/v2/report-generator.js index 71aa13a29550..e8d57c7ee116 100644 --- a/lighthouse-core/report/v2/report-generator.js +++ b/lighthouse-core/report/v2/report-generator.js @@ -19,8 +19,11 @@ const fs = require('fs'); 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(__dirname + '/report-renderer.js', 'utf8'); +const REPORT_JAVASCRIPT = [ + fs.readFileSync(__dirname + '/renderer/dom.js', 'utf8'), + fs.readFileSync(__dirname + '/renderer/details-renderer.js', 'utf8'), + fs.readFileSync(__dirname + '/renderer/report-renderer.js', 'utf8'), +].join(';\n'); const REPORT_CSS = fs.readFileSync(__dirname + '/report-styles.css', 'utf8'); const REPORT_TEMPLATES = fs.readFileSync(__dirname + '/templates.html', 'utf8'); diff --git a/lighthouse-core/test/report/v2/report-renderer-test.js b/lighthouse-core/test/report/v2/renderer/report-renderer-test.js similarity index 81% rename from lighthouse-core/test/report/v2/report-renderer-test.js rename to lighthouse-core/test/report/v2/renderer/report-renderer-test.js index 8d3590bb3b1e..59135725b214 100644 --- a/lighthouse-core/test/report/v2/report-renderer-test.js +++ b/lighthouse-core/test/report/v2/renderer/report-renderer-test.js @@ -20,40 +20,41 @@ const assert = require('assert'); const fs = require('fs'); const jsdom = require('jsdom'); -const ReportRenderer = require('../../../report/v2/report-renderer.js'); -const sampleResults = require('../../results/sample_v2.json'); +const DOM = require('../../../../report/v2/renderer/dom.js'); +const DetailsRenderer = require('../../../../report/v2/renderer/details-renderer.js'); +const ReportRenderer = require('../../../../report/v2/renderer/report-renderer.js'); +const sampleResults = require('../../../results/sample_v2.json'); -const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../report/v2/templates.html', 'utf8'); +const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../../report/v2/templates.html', 'utf8'); describe('ReportRenderer V2', () => { - describe('renderReport', () => { - const document = jsdom.jsdom(TEMPLATE_FILE); - const renderer = new ReportRenderer(document); + let renderer; + let dom; - it('should render a report', () => { - const output = renderer.renderReport(sampleResults); - assert.ok(output.classList.contains('lighthouse-report')); - }); + before(() => { + global.DOM = DOM; + global.DetailsRenderer = DetailsRenderer; + const document = jsdom.jsdom(TEMPLATE_FILE); + renderer = new ReportRenderer(document); + dom = new DOM(document); + }); - it('should render an exception for invalid input', () => { - const output = renderer.renderReport({ - get reportCategories() { - throw new Error(); - } - }); - assert.ok(output.classList.contains('lighthouse-exception')); - }); + after(() => { + global.DOM = undefined; + global.DetailsRenderer = undefined; + }); + describe('DOM', () => { describe('createElement', () => { it('creates a simple element using default values', () => { - const el = renderer._createElement('div'); + const el = dom.createElement('div'); assert.equal(el.localName, 'div'); assert.equal(el.className, ''); assert.equal(el.className, el.attributes.length); }); it('creates an element from parameters', () => { - const el = renderer._createElement( + const el = dom.createElement( 'div', 'class1 class2', {title: 'title attr', tabindex: 0}); assert.equal(el.localName, 'div'); assert.equal(el.className, 'class1 class2'); @@ -64,14 +65,30 @@ describe('ReportRenderer V2', () => { describe('cloneTemplate', () => { it('should clone a template', () => { - const clone = renderer._cloneTemplate('#tmpl-lighthouse-audit-score'); + const clone = dom.cloneTemplate('#tmpl-lighthouse-audit-score'); assert.ok(clone.querySelector('.lighthouse-score')); }); it('fails when template cannot be found', () => { - assert.throws(() => renderer._cloneTemplate('#unknown-selector')); + assert.throws(() => dom.cloneTemplate('#unknown-selector')); }); }); + }); + + describe('renderReport', () => { + it('should render a report', () => { + const output = renderer.renderReport(sampleResults); + assert.ok(output.classList.contains('lighthouse-report')); + }); + + it('should render an exception for invalid input', () => { + const output = renderer.renderReport({ + get reportCategories() { + throw new Error(); + } + }); + assert.ok(output.classList.contains('lighthouse-exception')); + }); it('renders an audit', () => { const audit = sampleResults.reportCategories[0].audits[0]; From 816022d85cafc8023ab6ba2c96ad4ebacefb46c3 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 12 Apr 2017 13:04:08 -0700 Subject: [PATCH 2/3] added details renderer test --- .../v2/renderer/details-renderer-test.js | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 lighthouse-core/test/report/v2/renderer/details-renderer-test.js diff --git a/lighthouse-core/test/report/v2/renderer/details-renderer-test.js b/lighthouse-core/test/report/v2/renderer/details-renderer-test.js new file mode 100644 index 000000000000..7d3e04f1a25d --- /dev/null +++ b/lighthouse-core/test/report/v2/renderer/details-renderer-test.js @@ -0,0 +1,123 @@ +/** + * Copyright 2017 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'; + +const assert = require('assert'); +const fs = require('fs'); +const jsdom = require('jsdom'); +const URL = require('../../../../lib/url-shim'); +const DOM = require('../../../../report/v2/renderer/dom.js'); +const DetailsRenderer = require('../../../../report/v2/renderer/details-renderer.js'); + +const TEMPLATE_FILE = fs.readFileSync(__dirname + '/../../../../report/v2/templates.html', 'utf8'); + +/* eslint-env mocha */ + +describe('DOM', () => { + let renderer; + + before(() => { + global.URL = URL; + const document = jsdom.jsdom(TEMPLATE_FILE); + const dom = new DOM(document); + renderer = new DetailsRenderer(dom); + }); + + after(() => { + global.URL = undefined; + }); + + describe('render', () => { + it('renders text', () => { + const el = renderer.render({type: 'text', text: 'My text content'}); + assert.equal(el.textContent, 'My text content'); + assert.ok(el.classList.contains('lighthouse-text'), 'adds classes'); + }); + + it('renders blocks', () => { + const el = renderer.render({ + type: 'block', + items: [ + {type: 'text', text: 'content 1'}, + {type: 'text', text: 'content 2'}, + ], + }); + + const children = el.querySelectorAll('.lighthouse-text'); + assert.equal(children.length, 2, 'renders children'); + assert.ok(el.classList.contains('lighthouse-block'), 'adds classes'); + }); + + it('renders lists with headers', () => { + const el = renderer.render({ + type: 'list', + header: {type: 'text', text: 'My Header'}, + items: [ + {type: 'text', text: 'content 1'}, + {type: 'text', text: 'content 2'}, + ], + }); + + const header = el.querySelector('.lighthouse-list__header'); + assert.equal(header.textContent, 'My Header', 'did not render header'); + + const items = el.querySelector('.lighthouse-list__items'); + assert.equal(items.children.length, 2, 'did not render children'); + }); + + it('renders lists without headers', () => { + const el = renderer.render({ + type: 'list', + items: [ + {type: 'text', text: 'content 1'}, + {type: 'text', text: 'content 2'}, + {type: 'text', text: 'content 3'}, + ], + }); + + const header = el.querySelector('.lighthouse-list__header'); + assert.ok(!header, 'rendered header'); + + const items = el.querySelector('.lighthouse-list__items'); + assert.equal(items.children.length, 3, 'did not render children'); + }); + + it('renders nested structures', () => { + const el = renderer.render({ + type: 'block', + items: [ + {type: 'text', text: 'content 1'}, + {type: 'text', text: 'content 2'}, + { + type: 'list', + header: {type: 'text', text: 'header'}, + items: [ + {type: 'text', text: 'sub-content 1'}, + {type: 'text', text: 'sub-content 2'}, + ] + }, + ], + }); + + const textChild = el.querySelector('.lighthouse-block > .lighthouse-text'); + const listChild = el.querySelector('.lighthouse-block > .lighthouse-list'); + const textSubChild = el.querySelector('.lighthouse-block .lighthouse-list .lighthouse-text'); + assert.ok(textChild, 'did not render text children'); + assert.ok(listChild, 'did not render list child'); + assert.ok(textSubChild, 'did not render sub-children'); + }); + }); +}); From 58fab6e1089fe7c22dbff2d78ae0020dfdb3aa2f Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 12 Apr 2017 14:15:41 -0700 Subject: [PATCH 3/3] fix closure scope --- lighthouse-core/report/v2/renderer/details-renderer.js | 2 +- lighthouse-core/report/v2/renderer/report-renderer.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lighthouse-core/report/v2/renderer/details-renderer.js b/lighthouse-core/report/v2/renderer/details-renderer.js index 936fdc04f6e6..582db9556324 100644 --- a/lighthouse-core/report/v2/renderer/details-renderer.js +++ b/lighthouse-core/report/v2/renderer/details-renderer.js @@ -88,4 +88,4 @@ if (typeof module !== 'undefined' && module.exports) { } /** @typedef {{type: string, text: string|undefined, header: DetailsJSON|undefined, items: Array|undefined}} */ -let DetailsJSON; // eslint-disable-line no-unused-vars +DetailsRenderer.DetailsJSON; // eslint-disable-line no-unused-expressions diff --git a/lighthouse-core/report/v2/renderer/report-renderer.js b/lighthouse-core/report/v2/renderer/report-renderer.js index 2ca1fcbce7c7..8ca875422bfc 100644 --- a/lighthouse-core/report/v2/renderer/report-renderer.js +++ b/lighthouse-core/report/v2/renderer/report-renderer.js @@ -185,7 +185,7 @@ if (typeof module !== 'undefined' && module.exports) { module.exports = ReportRenderer; } -/** @typedef {{id: string, weight: number, score: number, result: {description: string, displayValue: string, helpText: string, score: number|boolean, details: DetailsJSON|undefined}}} */ +/** @typedef {{id: string, weight: number, score: number, result: {description: string, displayValue: string, helpText: string, score: number|boolean, details: DetailsRenderer.DetailsJSON|undefined}}} */ let AuditJSON; // eslint-disable-line no-unused-vars /** @typedef {{name: string, weight: number, score: number, description: string, audits: Array}} */