From d8d386dd8c7f7d585c1efd60fa73cb71bd4e64cd Mon Sep 17 00:00:00 2001
From: Connor Clark <cjamcl@google.com>
Date: Tue, 12 May 2020 18:45:14 -0700
Subject: [PATCH] report(metrics): use css grid so metrics are aligned (#10695)

---
 .../renderer/performance-category-renderer.js | 15 ++----
 lighthouse-core/report/html/report-styles.css | 51 +++++++------------
 .../performance-category-renderer-test.js     |  3 +-
 lighthouse-viewer/test/viewer-test-pptr.js    |  4 +-
 4 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/lighthouse-core/report/html/renderer/performance-category-renderer.js b/lighthouse-core/report/html/renderer/performance-category-renderer.js
index f01c8262471e..6abff5f69975 100644
--- a/lighthouse-core/report/html/renderer/performance-category-renderer.js
+++ b/lighthouse-core/report/html/renderer/performance-category-renderer.js
@@ -132,19 +132,10 @@ class PerformanceCategoryRenderer extends CategoryRenderer {
     metricAuditsEl.append(..._toggleEl.childNodes);
 
     const metricAudits = category.auditRefs.filter(audit => audit.group === 'metrics');
+    const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-metrics-container');
 
-    const keyMetrics = metricAudits.slice(0, 3);
-    const otherMetrics = metricAudits.slice(3);
-
-    const metricsBoxesEl = this.dom.createChildOf(metricAuditsEl, 'div', 'lh-columns');
-    const metricsColumn1El = this.dom.createChildOf(metricsBoxesEl, 'div', 'lh-column');
-    const metricsColumn2El = this.dom.createChildOf(metricsBoxesEl, 'div', 'lh-column');
-
-    keyMetrics.forEach(item => {
-      metricsColumn1El.appendChild(this._renderMetric(item));
-    });
-    otherMetrics.forEach(item => {
-      metricsColumn2El.appendChild(this._renderMetric(item));
+    metricAudits.forEach(item => {
+      metricsBoxesEl.appendChild(this._renderMetric(item));
     });
 
     // 'Values are estimated and may vary' is used as the category description for PSI
diff --git a/lighthouse-core/report/html/report-styles.css b/lighthouse-core/report/html/report-styles.css
index 5182c997b2d7..d4870abc77fe 100644
--- a/lighthouse-core/report/html/report-styles.css
+++ b/lighthouse-core/report/html/report-styles.css
@@ -552,50 +552,33 @@
   display: none;
 }
 
-
 /* Perf Metric */
 
-.lh-columns {
-  display: flex;
-  width: 100%;
-}
-@media screen and (max-width: 640px) {
-  .lh-columns {
-    flex-wrap: wrap;
-
-  }
+.lh-metrics-container {
+  display: grid;
+  grid-template-rows: 1fr 1fr 1fr;
+  grid-auto-flow: column;
+  grid-column-gap: 24px;
 }
 
-.lh-column {
-  flex: 1;
-}
-.lh-column:first-of-type {
-  margin-right: 24px;
+.lh-metric {
+  border-top: 1px solid var(--report-border-color-secondary);
 }
 
-@media screen and (max-width: 800px) {
-  .lh-column:first-of-type {
-    margin-right: 8px;
+@media screen and (min-width: 640px) {
+  .lh-metric:nth-last-child(-n+2) {
+    border-bottom: 1px solid var(--report-border-color-secondary);
   }
 }
+
 @media screen and (max-width: 640px) {
-  .lh-column {
-    flex-basis: 100%;
-  }
-  .lh-column:first-of-type {
-    margin-right: 0px;
-  }
-  .lh-column:first-of-type .lh-metric:last-of-type {
-    border-bottom: 0;
+  .lh-metrics-container {
+    display: block;
   }
-}
 
-
-.lh-metric {
-  border-bottom: 1px solid var(--report-border-color-secondary);
-}
-.lh-metric:first-of-type {
-  border-top: 1px solid var(--report-border-color-secondary);
+  .lh-metric:nth-last-child(-n+1) {
+    border-bottom: 1px solid var(--report-border-color-secondary);
+  }
 }
 
 .lh-metric__innerwrap {
@@ -637,7 +620,7 @@
 
 /* No-JS toggle switch */
 /* Keep this selector sync'd w/ `magicSelector` in report-ui-features-test.js */
- .lh-metrics-toggle__input:checked ~ .lh-columns .lh-metric__description {
+ .lh-metrics-toggle__input:checked ~ .lh-metrics-container .lh-metric__description {
   display: block;
 }
 
diff --git a/lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js b/lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js
index 733528163764..a9016a2b018e 100644
--- a/lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js
+++ b/lighthouse-core/test/report/html/renderer/performance-category-renderer-test.js
@@ -240,7 +240,8 @@ describe('PerfCategoryRenderer', () => {
     let toggle;
     const metricsSelector = '.lh-audit-group--metrics';
     const toggleSelector = '.lh-metrics-toggle__input';
-    const magicSelector = '.lh-metrics-toggle__input:checked ~ .lh-columns .lh-metric__description';
+    const magicSelector =
+      '.lh-metrics-toggle__input:checked ~ .lh-metrics-container .lh-metric__description';
     let getDescriptionsAfterCheckedToggle;
 
     describe('works if there is a performance category', () => {
diff --git a/lighthouse-viewer/test/viewer-test-pptr.js b/lighthouse-viewer/test/viewer-test-pptr.js
index 7b57885c2d52..525203a338fd 100644
--- a/lighthouse-viewer/test/viewer-test-pptr.js
+++ b/lighthouse-viewer/test/viewer-test-pptr.js
@@ -192,7 +192,7 @@ describe('Lighthouse Viewer', () => {
       await viewerPage.goto(url);
 
       // Wait for report to render.
-      await viewerPage.waitForSelector('.lh-columns');
+      await viewerPage.waitForSelector('.lh-metrics-container');
 
       const interceptedUrl = new URL(interceptedRequest.url());
       expect(interceptedUrl.origin + interceptedUrl.pathname)
@@ -249,7 +249,7 @@ describe('Lighthouse Viewer', () => {
       await viewerPage.goto(url);
 
       // Wait for report to render.call out to PSI with specified categories
-      await viewerPage.waitForSelector('.lh-columns');
+      await viewerPage.waitForSelector('.lh-metrics-container');
 
       const interceptedUrl = new URL(interceptedRequest.url());
       expect(interceptedUrl.origin + interceptedUrl.pathname)