From 2f0fb8e52eab3d73500a9b327c03c2fb48154b93 Mon Sep 17 00:00:00 2001
From: Patrick Hulce <patrick.hulce@gmail.com>
Date: Mon, 28 Oct 2019 15:09:30 -0500
Subject: [PATCH] feat(server): add percentage deltas in table details

---
 packages/server/src/ui/app.css                |  2 +-
 .../audit-detail/audit-detail-pane.css        |  2 +-
 .../audit-detail/simple-details.jsx           | 27 ++++++++++++-------
 .../build-view/audit-detail/table-details.jsx | 18 ++++++++++---
 .../src/ui/routes/build-view/build-view.css   |  2 +-
 .../src/ui/routes/build-view/build-view.jsx   |  4 ++-
 types/utils.d.ts                              |  4 +--
 7 files changed, 39 insertions(+), 20 deletions(-)

diff --git a/packages/server/src/ui/app.css b/packages/server/src/ui/app.css
index bb8414182..505710f26 100644
--- a/packages/server/src/ui/app.css
+++ b/packages/server/src/ui/app.css
@@ -130,7 +130,7 @@ body {
 .container {
   width: 100%;
   max-width: 762px;
-  min-width: 400px;
+  min-width: 300px;
   margin: 0 auto;
 }
 
diff --git a/packages/server/src/ui/routes/build-view/audit-detail/audit-detail-pane.css b/packages/server/src/ui/routes/build-view/audit-detail/audit-detail-pane.css
index 9c97725b8..78fcc61c5 100644
--- a/packages/server/src/ui/routes/build-view/audit-detail/audit-detail-pane.css
+++ b/packages/server/src/ui/routes/build-view/audit-detail/audit-detail-pane.css
@@ -8,7 +8,7 @@
   position: fixed;
   top: var(--page-header-height);
   right: 0;
-  left: 50%;
+  left: 40%;
   bottom: 0;
   z-index: 5;
   padding-bottom: 70vh;
diff --git a/packages/server/src/ui/routes/build-view/audit-detail/simple-details.jsx b/packages/server/src/ui/routes/build-view/audit-detail/simple-details.jsx
index cbdc139b5..380b57757 100644
--- a/packages/server/src/ui/routes/build-view/audit-detail/simple-details.jsx
+++ b/packages/server/src/ui/routes/build-view/audit-detail/simple-details.jsx
@@ -6,35 +6,40 @@
 
 import {h} from 'preact';
 import './simple-details.css';
+import {getDiffLabel, getDeltaStats} from '@lhci/utils/src/audit-diff-finder.js';
 
-/** @param {{type: LH.DetailsType, baseValue: any, compareValue: any}} props */
+/** @param {{type: LH.DetailsType, baseValue: any, compareValue: any, diff?: LHCI.NumericItemAuditDiff}} props */
 export const SimpleDetails = props => {
   let type = props.type;
-  const {compareValue, baseValue} = props;
+  const {compareValue, baseValue, diff} = props;
   const value = compareValue === undefined ? baseValue : compareValue;
 
   if (typeof value === 'object' && value.type) {
     type = value.type;
   }
 
+  const label = diff ? getDiffLabel(diff) : 'neutral';
+
   const numericBase = Number.isFinite(baseValue) ? baseValue : 0;
   const numericCompare = Number.isFinite(compareValue) ? compareValue : 0;
-
-  let label = 'neutral';
-  if (numericCompare < numericBase) label = 'improvement';
-  if (numericCompare > numericBase) label = 'regression';
-
   const baseDisplay = `Base Value: ${Math.round(numericBase).toLocaleString()}`;
   const compareDisplay = `Compare Value: ${Math.round(numericCompare).toLocaleString()}`;
-  const title = `${baseDisplay}, ${compareDisplay}`;
+  const numericTitle = `${baseDisplay}, ${compareDisplay}`;
+  const deltaPercent =
+    diff && getDeltaStats(diff).percentAbsoluteDelta !== 1
+      ? ` (${(getDeltaStats(diff).percentAbsoluteDelta * 100).toLocaleString(undefined, {
+          maximumFractionDigits: 0,
+        })}%)`
+      : '';
 
   switch (type) {
     case 'bytes': {
       const kb = Math.abs((numericCompare - numericBase) / 1024);
       return (
-        <pre className={`simple-details--${label}`} data-tooltip={title}>
+        <pre className={`simple-details--${label}`} data-tooltip={numericTitle}>
           {numericCompare >= numericBase ? '+' : '-'}
           {kb.toLocaleString(undefined, {maximumFractionDigits: Math.abs(kb) < 1 ? 1 : 0})} KB
+          {deltaPercent}
         </pre>
       );
     }
@@ -42,9 +47,10 @@ export const SimpleDetails = props => {
     case 'timespanMs': {
       const ms = Math.abs(Math.round(numericCompare - numericBase));
       return (
-        <pre className={`simple-details--${label}`} data-tooltip={title}>
+        <pre className={`simple-details--${label}`} data-tooltip={numericTitle}>
           {numericCompare >= numericBase ? '+' : '-'}
           {ms.toLocaleString()} ms
+          {deltaPercent}
         </pre>
       );
     }
@@ -82,6 +88,7 @@ export const SimpleDetails = props => {
         <pre className={`simple-details--${label}`}>
           {numericCompare >= numericBase ? '+' : '-'}
           {Math.abs(numericCompare - numericBase).toLocaleString()}
+          {deltaPercent}
         </pre>
       );
     }
diff --git a/packages/server/src/ui/routes/build-view/audit-detail/table-details.jsx b/packages/server/src/ui/routes/build-view/audit-detail/table-details.jsx
index 1f7cf2729..05db72720 100644
--- a/packages/server/src/ui/routes/build-view/audit-detail/table-details.jsx
+++ b/packages/server/src/ui/routes/build-view/audit-detail/table-details.jsx
@@ -28,7 +28,7 @@ function isNumericValueType(itemType) {
 
 /** @param {{pair: LHCI.AuditPair}} props */
 export const TableDetails = props => {
-  const {audit, baseAudit, diffs} = props.pair;
+  const {audit, baseAudit, diffs: allDiffs} = props.pair;
   if (!audit.details) return <Fragment />;
   const {headings: compareHeadings, items: compareItems} = audit.details;
   if (!compareHeadings || !compareItems) return <Fragment />;
@@ -37,7 +37,7 @@ export const TableDetails = props => {
   const baseItems = (baseAudit && baseAudit.details && baseAudit.details.items) || [];
 
   const zippedItems = zipBaseAndCompareItems(baseItems, compareItems);
-  const sortedItems = sortZippedBaseAndCompareItems(diffs, zippedItems);
+  const sortedItems = sortZippedBaseAndCompareItems(allDiffs, zippedItems);
   const headings = compareHeadings.length ? compareHeadings : baseHeadings;
   // We'll insert the row label before the first numeric heading, or last if none is found.
   let insertRowLabelAfterIndex =
@@ -63,18 +63,27 @@ export const TableDetails = props => {
           </tr>
         </thead>
         <tbody>
-          {sortedItems.map(({base, compare}) => {
+          {sortedItems.map(({base, compare, diffs}) => {
             const definedItem = compare || base;
             // This should never be true, but make tsc happy
             if (!definedItem) return null;
 
             const key = `${base && base.index}-${compare && compare.index}`;
-            const state = getRowLabelForIndex(diffs, compare && compare.index, base && base.index);
+            const state = getRowLabelForIndex(
+              allDiffs,
+              compare && compare.index,
+              base && base.index
+            );
 
             return (
               <tr key={key}>
                 {headings.map((heading, j) => {
                   const itemType = heading.valueType || heading.itemType || 'unknown';
+                  const diff = diffs.find(
+                    /** @return {diff is LHCI.NumericItemAuditDiff} */
+                    diff => diff.type === 'itemDelta' && diff.itemKey === heading.key
+                  );
+
                   return (
                     <Fragment key={j}>
                       <td className={`table-column--${itemType}`}>
@@ -82,6 +91,7 @@ export const TableDetails = props => {
                           type={itemType}
                           compareValue={compare && compare.item[heading.key]}
                           baseValue={base && base.item[heading.key]}
+                          diff={diff}
                         />
                       </td>
                       {insertRowLabelAfterIndex === j ? (
diff --git a/packages/server/src/ui/routes/build-view/build-view.css b/packages/server/src/ui/routes/build-view/build-view.css
index e71640f48..c56481a0e 100644
--- a/packages/server/src/ui/routes/build-view/build-view.css
+++ b/packages/server/src/ui/routes/build-view/build-view.css
@@ -5,7 +5,7 @@
  */
 
 .build-view--with-audit-selection {
-  width: 50%;
+  width: 40%;
 }
 
 .build-view__scores-and-url {
diff --git a/packages/server/src/ui/routes/build-view/build-view.jsx b/packages/server/src/ui/routes/build-view/build-view.jsx
index 58c5c176b..1b8e16a9a 100644
--- a/packages/server/src/ui/routes/build-view/build-view.jsx
+++ b/packages/server/src/ui/routes/build-view/build-view.jsx
@@ -65,7 +65,9 @@ function computeAuditGroups(lhr, baseLhr, options) {
     const auditPairs = intermediateGroup.audits
       .map(audit => {
         const baseAudit = baseLhr && baseLhr.audits[audit.id || ''];
-        const diffs = baseAudit ? findAuditDiffs(baseAudit, audit, options) : [];
+        const diffs = baseAudit
+          ? findAuditDiffs(baseAudit, audit, {...options, synthesizeItemKeyDiffs: true})
+          : [];
         const maxSeverity = Math.max(...diffs.map(getDiffSeverity), 0);
         return {audit, baseAudit, diffs, maxSeverity, group: intermediateGroup.group};
       })
diff --git a/types/utils.d.ts b/types/utils.d.ts
index f3ff77446..23d4ade1f 100644
--- a/types/utils.d.ts
+++ b/types/utils.d.ts
@@ -41,8 +41,8 @@ declare global {
 
     export interface NumericItemAuditDiff extends BaseAuditDiff, BaseNumericAuditDiff {
       type: 'itemDelta';
-      baseItemIndex: number;
-      compareItemIndex: number;
+      baseItemIndex?: number;
+      compareItemIndex?: number;
       itemKey: string;
     }