More Frontend QA Polish Changes (#1709)

Sort of relevant to #1696

- Improves a number of layout elements at smaller viewport sizes
(specifically, letting button groups wrap onto next lines & ensure
titles can't shrink to 0 width)
- Adds "No page title" to places where there'd normally be a page title
but isn't
- Applies grid styling to page list area to fix overflow issues 
- When selecting or loading with a page selected that's farther down on
the page list, the top of the page header could be scrolled away from,
and there'd be no way to scroll back because the area had `overflow:
hidden` applied
- Adds default width and height to image comparer element, so that it
displays correctly when images haven't loaded and doesn't change layout
when images load

---------

Co-authored-by: Henry Wilkinson <henry@wilkinson.graphics>

frontend/src/features/qa/page-list/ui/page.ts

@@ -1,4 +1,4 @@
-import { localized } from "@lit/localize";
+import { localized, msg } from "@lit/localize";
 import type { SlTooltip } from "@shoelace-style/shoelace";
 import clsx from "clsx";
 import { html, type PropertyValues } from "lit";
@@ -174,7 +174,8 @@ export class QaPage extends TailwindElement {
             </div>
           </sl-tooltip>
           <h5 class="truncate text-sm font-semibold text-black">
-            ${page.title}
+            ${page.title ||
+            html`<span class="opacity-50">${msg("No page title")}</span>`}
           </h5>
           <div class="truncate text-xs leading-4 text-blue-600">
             ${page.url}

frontend/src/pages/org/archived-item-qa/archived-item-qa.ts

@@ -326,12 +326,14 @@ export class ArchivedItemQA extends TailwindElement {
     return html`
       ${this.renderHidden()}
 
-      <article class="grid gap-x-6 gap-y-4 md:gap-y-0">
+      <article class="qa-grid grid gap-x-6 gap-y-0">
         <header
-          class="grid--header flex items-center justify-between gap-1 border-b py-2"
+          class="grid--header flex flex-wrap items-center justify-between gap-1 border-b py-2"
         >
           <div class="flex items-center gap-2 overflow-hidden">
-            <h1 class="flex-1 truncate text-base font-semibold leading-tight">
+            <h1
+              class="flex-1 flex-shrink-0 basis-32 truncate text-base font-semibold leading-tight"
+            >
               ${itemName}
             </h1>
             ${when(
@@ -351,7 +353,7 @@ export class ArchivedItemQA extends TailwindElement {
               `,
             )}
           </div>
-          <div>
+          <div class="ml-auto flex">
             <sl-button
               size="small"
               variant="text"
@@ -381,19 +383,23 @@ export class ArchivedItemQA extends TailwindElement {
         </header>
 
         <div
-          class="grid--pageToolbar flex items-center justify-between overflow-hidden border-b py-2"
+          class="grid--pageToolbar flex flex-wrap items-center justify-stretch gap-2 overflow-hidden border-b py-2 @container"
         >
           <h2
-            class="mr-4 truncate text-base font-semibold text-neutral-700"
+            class="flex-auto flex-shrink-0 flex-grow basis-32 truncate text-base font-semibold text-neutral-700"
-            title="${this.page ? this.page.title : nothing}"
+            title="${this.page?.title ?? ""}"
           >
-            ${this.page ? this.page.title || msg("no page title") : nothing}
+            ${this.page?.title ||
+            html`<span class="opacity-50">${msg("No page title")}</span>`}
           </h2>
-          <div class="flex gap-4">
+          <div
+            class="ml-auto flex flex-grow basis-auto flex-wrap justify-between gap-2 @lg:flex-grow-0"
+          >
             <sl-button
               size="small"
               @click=${this.navPrevPage}
               ?disabled=${!prevPage}
+              class="order-1"
             >
               <sl-icon slot="prefix" name="arrow-left"></sl-icon>
               ${msg("Previous Page")}
@@ -403,6 +409,7 @@ export class ArchivedItemQA extends TailwindElement {
                 "Approvals are temporarily disabled during analysis runs.",
               )}
               ?disabled=${!disableReview}
+              class="order-3 mx-auto flex w-full justify-center @lg:order-2 @lg:mx-0 @lg:w-auto"
             >
               <btrix-page-qa-approval
                 .authState=${this.authState}
@@ -420,6 +427,7 @@ export class ArchivedItemQA extends TailwindElement {
               ?disabled=${!nextPage}
               outline
               @click=${this.navNextPage}
+              class="order-2 @lg:order-3"
             >
               <sl-icon slot="suffix" name="arrow-right"></sl-icon>
               ${msg("Next Page")}
@@ -427,8 +435,10 @@ export class ArchivedItemQA extends TailwindElement {
           </div>
         </div>
 
-        <div class="grid--tabGroup flex flex-col">
+        <div class="grid--tabGroup flex min-w-0 flex-col">
-          <nav class="my-2 flex gap-2">
+          <nav
+            class="-mx-3 my-0 flex gap-2 overflow-x-auto px-3 py-2 lg:mx-0 lg:px-0"
+          >
             <btrix-navigation-button
               id="screenshot-tab"
               href=${`${crawlBaseUrl}/review/screenshots?${searchParams.toString()}`}
@@ -475,14 +485,16 @@ export class ArchivedItemQA extends TailwindElement {
           ${this.renderPanelToolbar()} ${this.renderPanel()}
         </div>
 
-        <section class="grid--pageList overflow-hidden">
+        <section
+          class="grid--pageList grid grid-rows-[auto_1fr] *:min-h-0 *:min-w-0"
+        >
           <h2
             class="my-4 text-base font-semibold leading-none text-neutral-800"
           >
             ${msg("Pages")}
           </h2>
           <btrix-qa-page-list
-            class="flex h-full flex-col"
+            class="flex flex-col"
             .qaRunId=${this.qaRunId}
             .itemPageId=${this.itemPageId}
             .pages=${this.pages}

frontend/src/pages/org/archived-item-qa/styles.ts

@@ -1,25 +1,23 @@
 import { css } from "lit";
 
-import { TWO_COL_SCREEN_MIN_CSS } from "@/components/ui/tab-list";
-
 export const styles = css`
   article > * {
     min-height: 0;
   }
 
-  .grid {
+  .qa-grid {
     grid-template:
       "header"
       "pageToolbar"
-      "tabNav"
       "tabGroup"
       "pageList";
     grid-template-columns: 100%;
     grid-template-rows: repeat(5, max-content);
   }
 
-  @media only screen and (min-width: ${TWO_COL_SCREEN_MIN_CSS}) {
+  /* Tailwind 'lg' responsive size */
-    .grid {
+  @media only screen and (min-width: 1024px) {
+    .qa-grid {
       /* TODO calculate screen space instead of hardcoding */
       height: 100vh;
       /* overflow: hidden; */

frontend/src/pages/org/archived-item-qa/ui/resources.ts

@@ -1,4 +1,5 @@
 import { msg } from "@lit/localize";
+import clsx from "clsx";
 import { html } from "lit";
 
 import type { ReplayData, ResourcesPayload } from "../types";
@@ -22,28 +23,32 @@ function renderDiff(
   ];
   const rows = [
     [
-      html`<span class=${tw`font-semibold capitalize`}
+      html`<span class="font-semibold capitalize"
         >${msg("All Resources")}</span
       >`,
-      html`<span class=${tw`font-semibold`}
+      html`<span class="font-semibold"
         >${crawlResources[TOTAL].good.toLocaleString()}</span
       >`,
-      html`<span class=${tw`font-semibold`}
+      html`<span class="font-semibold"
         >${crawlResources[TOTAL].bad.toLocaleString()}</span
       >`,
       html`<span
-        class="${tw`font-semibold`} ${crawlResources[TOTAL].good !==
+        class="${clsx(
-        qaResources[TOTAL].good
+          "font-semibold",
-          ? tw`text-danger`
+          crawlResources[TOTAL].good !== qaResources[TOTAL].good
-          : tw`text-neutral-700`}"
+            ? "text-danger"
+            : "text-neutral-700",
+        )}"
       >
         ${qaResources[TOTAL].good.toLocaleString()}
       </span>`,
       html`<span
-        class="${tw`font-semibold`} ${crawlResources[TOTAL].bad !==
+        class="${clsx(
-        qaResources[TOTAL].bad
+          "font-semibold",
-          ? tw`text-danger`
+          crawlResources[TOTAL].bad !== qaResources[TOTAL].bad
-          : tw`text-neutral-700`}"
+            ? "text-danger"
+            : "text-neutral-700",
+        )}"
       >
         ${qaResources[TOTAL].bad.toLocaleString()}
       </span>`,
@@ -51,7 +56,7 @@ function renderDiff(
     ...Object.keys(qaResources)
       .filter((key) => key !== TOTAL)
       .map((key) => [
-        html`<span class=${tw`capitalize`}>${key}</span>`,
+        html`<span class="capitalize">${key}</span>`,
         html`${Object.prototype.hasOwnProperty.call(crawlResources, key)
           ? crawlResources[key].good.toLocaleString()
           : 0}`,
@@ -78,35 +83,39 @@ function renderDiff(
   ];
 
   return html`
-    <btrix-data-table .columns=${columns} .rows=${rows}></btrix-data-table>
+    <btrix-data-table
+      class="block"
+      .columns=${columns}
+      .rows=${rows}
+    ></btrix-data-table>
   `;
 }
 
 export function renderResources(crawlData: ReplayData, qaData: ReplayData) {
   const noData = html`<div
-    class=${tw`flex h-full flex-col items-center justify-center gap-2 text-xs text-neutral-500`}
+    class="m-4 flex flex-col items-center justify-center gap-2 text-xs text-neutral-500"
   >
     <sl-icon name="slash-circle"></sl-icon>
     ${msg("Resources data not available")}
   </div>`;
 
   return html`
-    <div class=${tw`flex h-full flex-col outline`}>
+    <div class="flex h-full flex-col outline">
-      <div class=${tw`flex-1 overflow-auto overscroll-contain`}>
+      <div class="flex-1 overflow-auto overscroll-contain">
         ${crawlData && qaData
           ? crawlData.resources && qaData.resources
             ? renderDiff(crawlData.resources, qaData.resources)
             : noData
           : renderSpinner()}
       </div>
-      <footer class=${tw`pt-2 text-xs text-neutral-600`}>
+      <footer class="pt-2 text-xs text-neutral-600">
         <dl>
-          <div class=${tw`flex gap-1`}>
+          <div class="flex gap-1">
-            <dt class=${tw`font-semibold`}>${msg("Good:")}</dt>
+            <dt class="font-semibold">${msg("Good:")}</dt>
             <dd>${msg("Success (2xx) and Redirection (3xx) status codes")}</dd>
           </div>
-          <div class=${tw`flex gap-1`}>
+          <div class="flex gap-1">
-            <dt class=${tw`font-semibold`}>${msg("Bad:")}</dt>
+            <dt class="font-semibold">${msg("Bad:")}</dt>
             <dd>
               ${msg("Client error (4xx) and Server error (5xx) status codes")}
             </dd>

frontend/src/pages/org/archived-item-qa/ui/screenshots.ts

@@ -1,4 +1,5 @@
 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";
@@ -7,18 +8,24 @@ import type { ReplayData } from "../types";
 
 import { renderSpinner } from "./spinner";
 
-import { tw } from "@/utils/tailwind";
-
 function image(data: ReplayData) {
   if (!data?.blobUrl) {
     return html`<div
-      class=${tw`flex h-full w-full flex-col items-center justify-center gap-2 text-xs text-neutral-500`}
+      class="flex h-full w-full flex-col items-center justify-center gap-2 text-xs text-neutral-500"
     >
       <sl-icon name="slash-circle"></sl-icon>
       ${msg("Screenshot not available")}
     </div>`;
   }
-  return html` <img class=${tw`h-full w-full`} src=${data.blobUrl} /> `;
+  return html`
+    <img
+      class="h-full w-full"
+      width="1920"
+      height="1080"
+      alt=""
+      src=${data.blobUrl}
+    />
+  `;
 }
 
 export function renderScreenshots(
@@ -27,30 +34,36 @@ export function renderScreenshots(
   splitView: boolean,
 ) {
   const content = html`
-    <div class=${tw`flex${splitView ? "" : tw` justify-between`}`}>
+    <div class=${clsx("flex", !splitView && "justify-between")}>
       <h3
         id="crawlScreenshotHeading"
-        class=${tw`mb-2 font-semibold ${splitView ? tw`flex-1` : "flex-grow-0"}`}
+        class=${clsx(
+          "mb-2 font-semibold",
+          splitView ? "flex-1" : "flex-grow-0",
+        )}
       >
         ${msg("Screenshot during crawl")}
       </h3>
       <h3
         id="qaScreenshotHeading"
-        class=${tw`mb-2 font-semibold ${splitView ? tw`flex-1` : "flex-grow-0"}`}
+        class=${clsx(
+          "mb-2 font-semibold",
+          splitView ? "flex-1" : "flex-grow-0",
+        )}
       >
         ${msg("Screenshot from replay")}
       </h3>
     </div>
     ${splitView
-      ? html` <div class=${tw`flex flex-col gap-2 md:flex-row`}>
+      ? html` <div class="flex flex-col gap-2 md:flex-row">
           <div
-            class=${tw`aspect-video flex-1 overflow-hidden rounded-lg border bg-slate-50`}
+            class="aspect-video flex-1 overflow-hidden rounded-lg border bg-slate-50"
             aria-labelledby="crawlScreenshotHeading"
           >
             ${when(crawlData, image, renderSpinner)}
           </div>
           <div
-            class=${tw`aspect-video flex-1 overflow-hidden rounded-lg border bg-slate-50`}
+            class="aspect-video flex-1 overflow-hidden rounded-lg border bg-slate-50"
             aria-labelledby="qaScreenshotHeading"
           >
             ${when(qaData, image, renderSpinner)}
@@ -58,19 +71,15 @@ export function renderScreenshots(
         </div>`
       : html`
           <div
-            class=${tw`aspect-video overflow-hidden rounded-lg border bg-slate-50`}
+            class="aspect-video overflow-hidden rounded-lg border bg-slate-50"
           >
-            <sl-image-comparer>
+            <sl-image-comparer class="h-full w-full">
-              <img
+              <div slot="after" aria-labelledby="crawlScreenshotHeading">
-                slot="after"
+                ${when(crawlData, image, renderSpinner)}
-                src="${crawlData?.blobUrl || ""}"
+              </div>
-                aria-labelledby="crawlScreenshotHeading"
+              <div slot="before" aria-labelledby="qaScreenshotHeading">
-              />
+                ${when(qaData, image, renderSpinner)}
-              <img
+              </div>
-                slot="before"
-                src="${qaData?.blobUrl || ""}"
-                aria-labelledby="qaScreenshotHeading"
-              />
             </sl-image-comparer>
           </div>
         `}

frontend/src/pages/org/archived-item-qa/ui/text.ts

@@ -65,7 +65,7 @@ function renderDiff(
 
 export function renderText(crawlData: ReplayData, qaData: ReplayData) {
   const noData = html`<div
-    class=${tw`flex flex-col items-center justify-center gap-2 text-xs text-neutral-500`}
+    class="m-4 flex flex-col items-center justify-center gap-2 text-xs text-neutral-500"
   >
     <sl-icon name="slash-circle"></sl-icon>
     ${msg("Text data not available")}