From 6a94c64fa23633d339c1c3e57c00b7899bbb6547 Mon Sep 17 00:00:00 2001
From: Emma Segal-Grossman <hi@emma.cafe>
Date: Tue, 28 May 2024 14:02:49 -0400
Subject: [PATCH] ensure loader fills image swiper, and only display loader
 when replay data is nullish, not when it's loaded but there's no image data
 (#1819)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Closes #1818

- Refactors image renderers, and changes the condition for which the
loading indicator is displayed from "`<data>.blobUrl` exists and has a
truthey value" to "`<data>` is not nullish"
- Adds an additional class config to the shared QA loading indicator,
and uses it in the image slider view to add a background and aspect
ratio to the loading indicator's container so that it takes up the space
it should and has an opaque background

| Before | After |
| --- | --- |
| <img width="813" alt="Screenshot 2024-05-22 at 5 17 14 PM"
src="https://github.com/webrecorder/browsertrix/assets/5727389/fb8a3a83-a0c8-46c1-b59b-a59adff2a175">
| <img width="813" alt="Screenshot 2024-05-22 at 5 17 31 PM"
src="https://github.com/webrecorder/browsertrix/assets/5727389/6fe5d04a-ffa9-4fc3-bc3b-bdb9b7b30948">
|
---
 .../org/archived-item-qa/ui/screenshots.ts    | 36 ++++++++-----------
 .../pages/org/archived-item-qa/ui/spinner.ts  | 10 +++---
 2 files changed, 20 insertions(+), 26 deletions(-)

diff --git a/frontend/src/pages/org/archived-item-qa/ui/screenshots.ts b/frontend/src/pages/org/archived-item-qa/ui/screenshots.ts
index 50234a73..3ab61b8f 100644
--- a/frontend/src/pages/org/archived-item-qa/ui/screenshots.ts
+++ b/frontend/src/pages/org/archived-item-qa/ui/screenshots.ts
@@ -2,13 +2,14 @@ import { msg } from "@lit/localize";
 import clsx from "clsx";
 import { html } from "lit";
 import { guard } from "lit/directives/guard.js";
-import { when } from "lit/directives/when.js";
 
 import type { BlobPayload, ReplayData } from "../types";
 
 import { renderSpinner } from "./spinner";
 
-function image(blobUrl: BlobPayload["blobUrl"]) {
+import { tw } from "@/utils/tailwind";
+
+function image(blobUrl: BlobPayload["blobUrl"] | undefined) {
   if (!blobUrl) {
     return html`<div
       class="flex aspect-video h-full w-full flex-col items-center justify-center gap-2 bg-slate-50 text-xs text-neutral-500"
@@ -28,11 +29,18 @@ function image(blobUrl: BlobPayload["blobUrl"]) {
   `;
 }
 
+const imageSpinner = renderSpinner(tw`aspect-video bg-slate-50`);
+
+const renderImage = (data: ReplayData | null) =>
+  guard(data, () => (data != null ? image(data.blobUrl) : imageSpinner));
+
 export function renderScreenshots(
   crawlData: ReplayData,
   qaData: ReplayData,
   splitView: boolean,
 ) {
+  const crawlImage = renderImage(crawlData);
+  const qaImage = renderImage(qaData);
   const content = html`
     <div class=${clsx("flex", !splitView && "justify-between")}>
       <h3
@@ -60,21 +68,13 @@ export function renderScreenshots(
             class="aspect-video flex-1 overflow-hidden rounded-lg border bg-slate-50"
             aria-labelledby="crawlScreenshotHeading"
           >
-            ${when(
-              crawlData?.blobUrl !== undefined && crawlData.blobUrl,
-              image,
-              renderSpinner,
-            )}
+            ${crawlImage}
           </div>
           <div
             class="aspect-video flex-1 overflow-hidden rounded-lg border bg-slate-50"
             aria-labelledby="qaScreenshotHeading"
           >
-            ${when(
-              qaData?.blobUrl !== undefined && qaData.blobUrl,
-              image,
-              renderSpinner,
-            )}
+            ${qaImage}
           </div>
         </div>`
       : html`
@@ -83,18 +83,10 @@ export function renderScreenshots(
           >
             <sl-image-comparer class="h-full w-full">
               <div slot="after" aria-labelledby="crawlScreenshotHeading">
-                ${when(
-                  crawlData?.blobUrl !== undefined && crawlData.blobUrl,
-                  image,
-                  renderSpinner,
-                )}
+                ${crawlImage}
               </div>
               <div slot="before" aria-labelledby="qaScreenshotHeading">
-                ${when(
-                  qaData?.blobUrl !== undefined && qaData.blobUrl,
-                  image,
-                  renderSpinner,
-                )}
+                ${qaImage}
               </div>
             </sl-image-comparer>
           </div>
diff --git a/frontend/src/pages/org/archived-item-qa/ui/spinner.ts b/frontend/src/pages/org/archived-item-qa/ui/spinner.ts
index 70674929..2b63e6a4 100644
--- a/frontend/src/pages/org/archived-item-qa/ui/spinner.ts
+++ b/frontend/src/pages/org/archived-item-qa/ui/spinner.ts
@@ -1,10 +1,12 @@
+import clsx from "clsx";
 import { html } from "lit";
 
-import { tw } from "@/utils/tailwind";
-
-export function renderSpinner() {
+export function renderSpinner(className?: clsx.ClassValue) {
   return html`<div
-    class=${tw`flex h-full w-full items-center justify-center p-9 text-2xl`}
+    class=${clsx(
+      "flex h-full w-full items-center justify-center p-9 text-2xl",
+      className,
+    )}
   >
     <sl-spinner></sl-spinner>
   </div>`;