feat(server): consolidate build view warnings

GoogleChrome · Oct 30, 2019 · be6a263 · be6a263
1 parent 1bb903b
commit be6a263
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 82 deletions.
diff --git a/packages/server/src/ui/routes/build-view/build-view-empty.jsx b/packages/server/src/ui/routes/build-view/build-view-empty.jsx
diff --git a/...ui/routes/build-view/build-view-empty.css → ...routes/build-view/build-view-warnings.css b/...ui/routes/build-view/build-view-empty.css → ...routes/build-view/build-view-warnings.css
@@ -4,23 +4,25 @@
  * 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.
  */
 
-.build-view__empty {
+.build-view__warning {
   display: flex;
   flex-direction: row;
   align-items: center;
 
-  min-height: 110px;
-
-  font-size: var(--header-font-size);
+  min-height: 80px;
+  margin-bottom: calc(var(--base-spacing) * 2);
 }
 
-.build-view__empty i {
-  color: var(--improvement-color);
+.build-view__warning i {
   font-size: 40px;
   margin: 0 var(--base-spacing);
 }
 
-.build-view-empty__lhr-link {
+.build-view__warning div {
+  line-height: 1.5;
+}
+
+.build-view-warning__lhr-link {
   cursor: pointer;
   text-align: center;
   color: var(--improvement-color);

diff --git a/packages/server/src/ui/routes/build-view/build-view-warnings.jsx b/packages/server/src/ui/routes/build-view/build-view-warnings.jsx
@@ -6,54 +6,80 @@
 
 import {h, Fragment} from 'preact';
 import {Paper} from '../../components/paper';
+import './build-view-warnings.css';
+import {LhrViewerLink} from '../../components/lhr-viewer-link';
 
-/** @param {{build: LHCI.ServerCommand.Build, baseBuild: LHCI.ServerCommand.Build | null, baseLhr?: LH.Result, hasBaseOverride: boolean}} props */
+/** @param {{build: LHCI.ServerCommand.Build, baseBuild: LHCI.ServerCommand.Build | null, auditGroups: Array<any>, lhr: LH.Result, baseLhr?: LH.Result, hasBaseOverride: boolean}} props */
 export const BuildViewWarnings = props => {
-  const {build, baseBuild, baseLhr, hasBaseOverride} = props;
-  /** @type {Array<string>} */
-  const warningMessages = [];
+  const {build, baseBuild, baseLhr, auditGroups, hasBaseOverride} = props;
+
+  const lhrLinkEl = (
+    <Fragment>
+      <LhrViewerLink className="build-view-empty__lhr-link" lhr={props.lhr}>
+        jump straight to the Lighthouse report.
+      </LhrViewerLink>
+    </Fragment>
+  );
 
   if (!baseBuild) {
-    warningMessages.push('No base build could be found for this commit.');
+    return (
+      <Paper className="build-view__warning">
+        <i className="material-icons">sentiment_very_dissatisfied</i>
+        <div>
+          Oops, no base build could be found for this commit. Manually select a base build above, or{' '}
+          {lhrLinkEl}
+        </div>
+      </Paper>
+    );
+  }
+
+  if (build.hash === baseBuild.hash) {
+    return (
+      <Paper className="build-view__warning">
+        <i className="material-icons">sentiment_very_dissatisfied</i>
+        <div>
+          Oops, this base build is the same commit as the compare. Select a different base build to
+          explore differences, or {lhrLinkEl}
+        </div>
+      </Paper>
+    );
   }
 
-  if (baseBuild && build.hash === baseBuild.hash) {
-    warningMessages.push(
-      [
-        'This base build is the same commit as the compare.',
-        'Select a different base build to explore differences.',
-      ].join(' ')
+  if (!baseLhr) {
+    return (
+      <Paper className="build-view__warning">
+        <i className="material-icons">sentiment_very_dissatisfied</i>
+        <div>
+          Oops, this base build is missing a run for this URL. Select a different URL to explore
+          differences, or {lhrLinkEl}
+        </div>
+      </Paper>
     );
   }
 
-  if (baseBuild && !baseLhr) {
-    warningMessages.push('This base build is missing a run for this URL.');
+  if (baseBuild.hash !== build.ancestorHash && !hasBaseOverride) {
+    return (
+      <Paper className="build-view__warning">
+        <i className="material-icons">warning</i>
+        <div>
+          This base build is not the exact ancestor of the compare. Differences may not be due to
+          this specific commit.
+        </div>
+      </Paper>
+    );
   }
 
-  if (
-    baseBuild &&
-    baseBuild.hash !== build.ancestorHash &&
-    !warningMessages.length &&
-    !hasBaseOverride
-  ) {
-    warningMessages.push(
-      [
-        'This base build is not the target ancestor of the compare.',
-        'Differences may not be due to this specific commit.',
-      ].join(' ')
+  if (baseLhr && !auditGroups.length) {
+    return (
+      <Paper className="build-view__warning">
+        <i className="material-icons">sentiment_satisified_alt</i>
+        <div>
+          Woah, no differences found! Switch base builds to explore other differences, or{' '}
+          {lhrLinkEl}
+        </div>
+      </Paper>
     );
   }
 
-  return (
-    <Fragment>
-      {warningMessages.map(message => {
-        return (
-          <Paper className="build-view__warning" key={message}>
-            <i className="material-icons">warning</i>
-            <div>{message}</div>
-          </Paper>
-        );
-      })}
-    </Fragment>
-  );
+  return <Fragment />;
 };
diff --git a/packages/server/src/ui/routes/build-view/build-view.css b/packages/server/src/ui/routes/build-view/build-view.css
@@ -56,17 +56,6 @@
   justify-content: center;
 }
 
-.build-view__warning {
-  display: flex;
-  flex-direction: row;
-  align-items: center;
-}
-
-.build-view__warning i {
-  color: var(--average-color);
-  margin-right: calc(var(--base-spacing) / 2);
-}
-
 .build-selector-header {
   position: fixed;
   height: var(--page-header-height);

diff --git a/packages/server/src/ui/routes/build-view/build-view.jsx b/packages/server/src/ui/routes/build-view/build-view.jsx
@@ -27,7 +27,6 @@ import './build-view.css';
 import {BuildViewLegend} from './build-view-legend';
 import clsx from 'clsx';
 import {findAuditDiffs, getDiffSeverity} from '@lhci/utils/src/audit-diff-finder';
-import {BuildViewEmpty} from './build-view-empty';
 import {route} from 'preact-router';
 import {BuildViewOptions} from './build-view-options';
 import {BuildViewWarnings} from './build-view-warnings';
@@ -253,7 +252,9 @@ const BuildView_ = props => {
         />
         <div className="container">
           <BuildViewWarnings
+            lhr={lhr}
             build={props.build}
+            auditGroups={auditGroups}
             baseBuild={props.ancestorBuild}
             baseLhr={baseLhr}
             hasBaseOverride={props.hasBaseOverride}
@@ -277,7 +278,7 @@ const BuildView_ = props => {
               />
             </Fragment>
           ) : (
-            <BuildViewEmpty lhr={lhr} />
+            <Fragment />
           )}
         </div>
       </div>