report: expand groups within Passed Audits (#6930)

GoogleChrome · Jan 11, 2019 · bc23383 · bc23383
1 parent 0b223e5
commit bc23383
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 17 deletions.
diff --git a/lighthouse-core/report/html/renderer/category-renderer.js b/lighthouse-core/report/html/renderer/category-renderer.js
@@ -217,10 +217,9 @@ class CategoryRenderer {
    * array of audit and audit-group elements.
    * @param {Array<LH.ReportResult.AuditRef>} auditRefs
    * @param {Object<string, LH.Result.ReportGroup>} groupDefinitions
-   * @param {{expandable: boolean}} opts
    * @return {Array<Element>}
    */
-  _renderGroupedAudits(auditRefs, groupDefinitions, opts) {
+  _renderGroupedAudits(auditRefs, groupDefinitions) {
     // Audits grouped by their group (or under notAGroup).
     /** @type {Map<string, Array<LH.ReportResult.AuditRef>>} */
     const grouped = new Map();
@@ -252,7 +251,10 @@ class CategoryRenderer {
 
       // Push grouped audits as a group.
       const groupDef = groupDefinitions[groupId];
-      const auditGroupElem = this.renderAuditGroup(groupDef, opts);
+      // 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});
       for (const auditRef of groupAuditRefs) {
         auditGroupElem.appendChild(this.renderAudit(auditRef, index++));
       }
@@ -272,7 +274,7 @@ class CategoryRenderer {
    */
   renderUnexpandableClump(auditRefs, groupDefinitions) {
     const clumpElement = this.dom.createElement('div');
-    const elements = this._renderGroupedAudits(auditRefs, groupDefinitions, {expandable: false});
+    const elements = this._renderGroupedAudits(auditRefs, groupDefinitions);
     elements.forEach(elem => clumpElement.appendChild(elem));
     return clumpElement;
   }
@@ -291,7 +293,8 @@ class CategoryRenderer {
    *      ├── audit 5
    *      └── audit 6
    * clump (e.g. 'manual')
-   *   ├── …
+   *   ├── audit 1
+   *   ├── audit 2
    *   ⋮
    * @param {TopLevelClumpId} clumpId
    * @param {{auditRefs: Array<LH.ReportResult.AuditRef>, groupDefinitions: Object<string, LH.Result.ReportGroup>, description?: string}} clumpOpts
@@ -305,17 +308,15 @@ class CategoryRenderer {
       return failedElem;
     }
 
-    const expandable = true;
-    const elements = this._renderGroupedAudits(auditRefs, groupDefinitions, {expandable});
-
     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 opts = {expandable, itemCount: auditRefs.length};
-    const clumpElem = this.renderAuditGroup(groupDef, opts);
+    const groupOpts = {expandable: true, itemCount: auditRefs.length};
+    const clumpElem = this.renderAuditGroup(groupDef, groupOpts);
     clumpElem.classList.add('lh-clump', clumpInfo.className);
 
-    elements.forEach(elem => clumpElem.appendChild(elem));
+    // For all non-failed clumps, we don't group
+    clumpElem.append(...auditRefs.map(this.renderAudit.bind(this)));
 
     return clumpElem;
   }

diff --git a/lighthouse-core/test/report/html/renderer/category-renderer-test.js b/lighthouse-core/test/report/html/renderer/category-renderer-test.js
@@ -227,14 +227,16 @@ describe('CategoryRenderer', () => {
       assert.equal(failedAuditGroups.length, failedAuditTags.size);
     });
 
-    it('renders the passed audits grouped by group', () => {
+    it('renders the passed audits ungrouped', () => {
       const categoryDOM = renderer.render(category, sampleResults.categoryGroups);
       const passedAudits = category.auditRefs.filter(audit =>
           audit.result.scoreDisplayMode !== 'notApplicable' && audit.result.score === 1);
-      const passedAuditTags = new Set(passedAudits.map(audit => audit.group));
 
       const passedAuditGroups = categoryDOM.querySelectorAll('.lh-clump--passed .lh-audit-group');
-      assert.equal(passedAuditGroups.length, passedAuditTags.size);
+      const passedAuditsElems = categoryDOM.querySelectorAll('.lh-clump--passed .lh-audit');
+
+      assert.equal(passedAuditGroups.length, 0);
+      assert.equal(passedAuditsElems.length, passedAudits.length);
     });
 
     it('renders all the audits', () => {
@@ -293,15 +295,20 @@ describe('CategoryRenderer', () => {
     });
 
     it('gives each group a selectable class', () => {
+      const categoryClone = JSON.parse(JSON.stringify(category));
+      // Force all results to be Failed for accurate counting of groups.
+      categoryClone.auditRefs.forEach(ref => {
+        ref.result.score = 0;
+        ref.result.scoreDisplayMode = 'binary';
+      });
       const categoryGroupIds = new Set(category.auditRefs.filter(a => a.group).map(a => a.group));
       assert.ok(categoryGroupIds.size > 6); // Ensure there's something to test.
 
-      const categoryElem = renderer.render(category, sampleResults.categoryGroups);
+      const categoryElem = renderer.render(categoryClone, sampleResults.categoryGroups);
 
       categoryGroupIds.forEach(groupId => {
         const selector = `.lh-audit-group--${groupId}`;
-        // Could be multiple results (e.g. found in both passed and failed clumps).
-        assert.ok(categoryElem.querySelectorAll(selector).length >= 1,
+        assert.equal(categoryElem.querySelectorAll(selector).length, 1,
           `could not find '${selector}'`);
       });
     });