From 744fb1a191b47a90561f4083a8f8b6e351ff4eac Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Thu, 10 Jan 2019 18:11:19 -0800 Subject: [PATCH] report: get rid of more clump/group/expandable crossover noise --- .../report/html/renderer/category-renderer.js | 134 ++++++++---------- .../renderer/performance-category-renderer.js | 6 +- .../html/renderer/pwa-category-renderer.js | 2 +- lighthouse-core/report/html/templates.html | 14 ++ 4 files changed, 81 insertions(+), 75 deletions(-) diff --git a/lighthouse-core/report/html/renderer/category-renderer.js b/lighthouse-core/report/html/renderer/category-renderer.js index 5a84a57818bd..0fa8be23b480 100644 --- a/lighthouse-core/report/html/renderer/category-renderer.js +++ b/lighthouse-core/report/html/renderer/category-renderer.js @@ -43,23 +43,11 @@ class CategoryRenderer { /** * Display info per top-level clump. Define on class to avoid race with Util init. */ - get _clumpDisplayInfo() { + get _clumpTitles() { return { - failed: { - className: 'lh-clump--failed', - }, - manual: { - title: Util.UIStrings.manualAuditsGroupTitle, - className: 'lh-clump--manual', - }, - passed: { - title: Util.UIStrings.passedAuditsGroupTitle, - className: 'lh-clump--passed', - }, - notApplicable: { - title: Util.UIStrings.notApplicableAuditsGroupTitle, - className: 'lh-clump--notapplicable', - }, + manual: Util.UIStrings.manualAuditsGroupTitle, + passed: Util.UIStrings.passedAuditsGroupTitle, + notApplicable: Util.UIStrings.notApplicableAuditsGroupTitle, }; } @@ -183,20 +171,13 @@ class CategoryRenderer { * Renders the group container for a group of audits. Individual audit elements can be added * directly to the returned element. * @param {LH.Result.ReportGroup} group - * @param {{expandable: boolean, itemCount?: number}} opts * @return {Element} */ - renderAuditGroup(group, opts) { - const expandable = opts.expandable; - const groupEl = this.dom.createElement(expandable ? 'details' : 'div', 'lh-audit-group'); - const summaryEl = this.dom.createChildOf(groupEl, expandable ? 'summary' : 'div'); + renderAuditGroup(group) { + const groupEl = this.dom.createElement('div', 'lh-audit-group'); + const summaryEl = this.dom.createChildOf(groupEl, 'div'); const summaryInnerEl = this.dom.createChildOf(summaryEl, 'div', 'lh-audit-group__summary'); const headerEl = this.dom.createChildOf(summaryInnerEl, 'div', 'lh-audit-group__header'); - const itemCountEl = this.dom.createChildOf(summaryInnerEl, 'div', 'lh-audit-group__itemcount'); - if (expandable) { - const chevronEl = summaryInnerEl.appendChild(this._createChevron()); - chevronEl.title = Util.UIStrings.auditGroupExpandTooltip; - } if (group.description) { const auditGroupDescription = this.dom.createElement('div', 'lh-audit-group__description'); @@ -205,10 +186,6 @@ class CategoryRenderer { } headerEl.textContent = group.title; - if (opts.itemCount) { - // TODO(i18n): support multiple locales here - itemCountEl.textContent = `${opts.itemCount} audits`; - } return groupEl; } @@ -251,10 +228,7 @@ class CategoryRenderer { // Push grouped audits as a group. const groupDef = groupDefinitions[groupId]; - // Expanded by default! - // 1. The 'failed' clump has all groups expanded. - // 2. If a clump is collapsed (passed, n/a), we want the groups within to already be expanded - const auditGroupElem = this.renderAuditGroup(groupDef, {expandable: false}); + const auditGroupElem = this.renderAuditGroup(groupDef); for (const auditRef of groupAuditRefs) { auditGroupElem.appendChild(this.renderAudit(auditRef, index++)); } @@ -280,45 +254,40 @@ class CategoryRenderer { } /** - * Renders a clump (a grouping of groups), under a status of failed, manual, - * passed, or notApplicable. The result ends up something like: - * - * clump (e.g. 'failed') - * ├── audit 1 (w/o group) - * ├── audit 2 (w/o group) - * ├── audit group - * | ├── audit 3 - * | └── audit 4 - * └── audit group - * ├── audit 5 - * └── audit 6 - * clump (e.g. 'manual') - * ├── audit 1 - * ├── audit 2 - * ⋮ - * @param {TopLevelClumpId} clumpId - * @param {{auditRefs: Array, groupDefinitions: Object, description?: string}} clumpOpts + * Take a set of audits and render in a top-level, expandable clump that starts + * in a collapsed state. + * @param {Exclude} clumpId + * @param {{auditRefs: Array, description?: string}} clumpOpts * @return {Element} */ - renderClump(clumpId, {auditRefs, groupDefinitions, description}) { - if (clumpId === 'failed') { - // Failed audit clump is always expanded and not nested in an lh-audit-group. - const failedElem = this.renderUnexpandableClump(auditRefs, groupDefinitions); - failedElem.classList.add('lh-clump', this._clumpDisplayInfo.failed.className); - return failedElem; + renderClump(clumpId, {auditRefs, description}) { + const clumpTmpl = this.dom.cloneTemplate('#tmpl-lh-clump', this.templateContext); + const clumpElement = this.dom.find('.lh-clump', clumpTmpl); + + const summaryInnerEl = this.dom.find('.lh-audit-group__summary', clumpElement); + const chevronEl = summaryInnerEl.appendChild(this._createChevron()); + chevronEl.title = Util.UIStrings.auditGroupExpandTooltip; + + const headerEl = this.dom.find('.lh-audit-group__header', clumpElement); + const title = this._clumpTitles[clumpId]; + headerEl.textContent = title; + if (description) { + const markdownDescriptionEl = this.dom.convertMarkdownLinkSnippets(description); + const auditGroupDescription = this.dom.createElement('div', 'lh-audit-group__description'); + auditGroupDescription.appendChild(markdownDescriptionEl); + clumpElement.appendChild(auditGroupDescription); } - const clumpInfo = this._clumpDisplayInfo[clumpId]; - // TODO: renderAuditGroup shouldn't be used to render a clump (since it *contains* audit groups). - const groupDef = {title: clumpInfo.title, description}; - const groupOpts = {expandable: true, itemCount: auditRefs.length}; - const clumpElem = this.renderAuditGroup(groupDef, groupOpts); - clumpElem.classList.add('lh-clump', clumpInfo.className); + const itemCountEl = this.dom.find('.lh-audit-group__itemcount', clumpElement); + // TODO(i18n): support multiple locales here + itemCountEl.textContent = `${auditRefs.length} audits`; - // For all non-failed clumps, we don't group - clumpElem.append(...auditRefs.map(this.renderAudit.bind(this))); + // Add all audit results to the clump. + const auditElements = auditRefs.map(this.renderAudit.bind(this)); + clumpElement.append(...auditElements); - return clumpElem; + clumpElement.classList.add(`lh-clump--${clumpId.toLowerCase()}`); + return clumpElement; } /** @@ -383,6 +352,23 @@ class CategoryRenderer { } /** + * Renders a set of top level sections (clumps), under a status of failed, manual, + * passed, or notApplicable. The result ends up something like: + * + * failed clump + * ├── audit 1 (w/o group) + * ├── audit 2 (w/o group) + * ├── audit group + * | ├── audit 3 + * | └── audit 4 + * └── audit group + * ├── audit 5 + * └── audit 6 + * other clump (e.g. 'manual') + * ├── audit 1 + * ├── audit 2 + * ├── … + * ⋮ * @param {LH.ReportResult.Category} category * @param {Object} [groupDefinitions] * @return {Element} @@ -409,12 +395,18 @@ class CategoryRenderer { } // Render each clump. - for (const [clumpId, clumpRefs] of clumps) { - if (clumpRefs.length === 0) continue; + for (const [clumpId, auditRefs] of clumps) { + if (auditRefs.length === 0) continue; + + if (clumpId === 'failed') { + const clumpElem = this.renderUnexpandableClump(auditRefs, groupDefinitions); + clumpElem.classList.add(`lh-clump--failed`); + element.appendChild(clumpElem); + continue; + } const description = clumpId === 'manual' ? category.manualDescription : undefined; - const clumpElem = this.renderClump(clumpId, {auditRefs: clumpRefs, groupDefinitions, - description}); + const clumpElem = this.renderClump(clumpId, {auditRefs, description}); element.appendChild(clumpElem); } diff --git a/lighthouse-core/report/html/renderer/performance-category-renderer.js b/lighthouse-core/report/html/renderer/performance-category-renderer.js index 252b667016bb..870bdfc4e79d 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); const keyMetrics = metricAudits.filter(a => a.weight >= 3); const otherMetrics = metricAudits.filter(a => a.weight < 3); @@ -176,7 +176,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer { const wastedMsValues = opportunityAudits.map(audit => this._getWastedMs(audit)); const maxWaste = Math.max(...wastedMsValues); const scale = Math.max(Math.ceil(maxWaste / 1000) * 1000, minimumScale); - const groupEl = this.renderAuditGroup(groups['load-opportunities'], {expandable: false}); + const groupEl = this.renderAuditGroup(groups['load-opportunities']); const tmpl = this.dom.cloneTemplate('#tmpl-lh-opportunity-header', this.templateContext); this.dom.find('.lh-load-opportunity__col--one', tmpl).textContent = @@ -202,7 +202,7 @@ class PerformanceCategoryRenderer extends CategoryRenderer { }); if (diagnosticAudits.length) { - const groupEl = this.renderAuditGroup(groups['diagnostics'], {expandable: false}); + const groupEl = this.renderAuditGroup(groups['diagnostics']); diagnosticAudits.forEach((item, i) => groupEl.appendChild(this.renderAudit(item, i))); groupEl.classList.add('lh-audit-group--diagnostics'); element.appendChild(groupEl); diff --git a/lighthouse-core/report/html/renderer/pwa-category-renderer.js b/lighthouse-core/report/html/renderer/pwa-category-renderer.js index 690e8da89bb1..7e07206f3eb0 100644 --- a/lighthouse-core/report/html/renderer/pwa-category-renderer.js +++ b/lighthouse-core/report/html/renderer/pwa-category-renderer.js @@ -40,7 +40,7 @@ class PwaCategoryRenderer extends CategoryRenderer { // Manual audits are still in a manual clump. const manualAuditRefs = auditRefs.filter(ref => ref.result.scoreDisplayMode === 'manual'); const manualElem = this.renderClump('manual', - {auditRefs: manualAuditRefs, groupDefinitions, description: category.manualDescription}); + {auditRefs: manualAuditRefs, description: category.manualDescription}); categoryElem.appendChild(manualElem); return categoryElem; diff --git a/lighthouse-core/report/html/templates.html b/lighthouse-core/report/html/templates.html index 0971ccd1f1f6..b7ac009867f5 100644 --- a/lighthouse-core/report/html/templates.html +++ b/lighthouse-core/report/html/templates.html @@ -52,6 +52,20 @@ + + +