Fix QA navigation (#1731)

- Resolves https://github.com/webrecorder/browsertrix/issues/1705
- Resolves https://github.com/webrecorder/browsertrix/issues/1477

Changes:

- Takes user back to archived item QA tab after submitting review.
- Prevents clicking on page in QA tab when there's no analysis runs.
- Disables analysis-related sort options in QA tab when there's no
analysis runs.
- Handles pages without a title in QA tab.
- Adds initial loading state to QA review page list.

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

@@ -3,9 +3,11 @@ import type { SlChangeEvent, SlSelect } from "@shoelace-style/shoelace";
 import { html, type PropertyValues } from "lit";
 import { customElement, property, query } from "lit/decorators.js";
 import { repeat } from "lit/directives/repeat.js";
+import { when } from "lit/directives/when.js";
 
 import { TailwindElement } from "@/classes/TailwindElement";
 import { type PageChangeEvent } from "@/components/ui/pagination";
+import { renderSpinner } from "@/pages/org/archived-item-qa/ui/spinner";
 import type { APIPaginatedList, APISortQuery } from "@/types/api";
 import type { ArchivedItemQAPage } from "@/types/qa";
 
@@ -124,23 +126,28 @@ export class PageList extends TailwindElement {
       <div
         class="scrollContainer relative -mx-2 overflow-y-auto overscroll-contain px-2"
       >
-        ${this.pages?.total
+        ${when(
+          this.pages,
+          ({ total, items, page, pageSize }) =>
+            total
               ? html`
                   <div
                     class="sticky top-0 z-30 bg-gradient-to-b from-white to-white/85 backdrop-blur-sm"
                   >
-                <div class="mb-0.5 ml-2 border-b py-1 text-xs text-neutral-500">
+                    <div
-                  ${this.pages.total === this.totalPages
+                      class="mb-0.5 ml-2 border-b py-1 text-xs text-neutral-500"
+                    >
+                      ${total === this.totalPages
                         ? msg(
                             str`Showing all ${this.totalPages.toLocaleString()} pages`,
                           )
                         : msg(
-                        str`Showing ${this.pages.total.toLocaleString()} of ${this.totalPages.toLocaleString()} pages`,
+                            str`Showing ${total.toLocaleString()} of ${this.totalPages.toLocaleString()} pages`,
                           )}
                     </div>
                   </div>
                   ${repeat(
-                this.pages.items,
+                    items,
                     ({ id }) => id,
                     (page: ArchivedItemQAPage) => html`
                       <btrix-qa-page
@@ -156,9 +163,9 @@ export class PageList extends TailwindElement {
                   )}
                   <div class="my-2 flex justify-center">
                     <btrix-pagination
-                  page=${this.pages.page}
+                      page=${page}
-                  totalCount=${this.pages.total}
+                      totalCount=${total}
-                  size=${this.pages.pageSize}
+                      size=${pageSize}
                       compact
                       @page-change=${(e: PageChangeEvent) => {
                         e.stopPropagation();
@@ -184,7 +191,9 @@ export class PageList extends TailwindElement {
                 >
                   <sl-icon name="slash-circle"></sl-icon>
                   ${msg("No matching pages found")}
-            </div>`}
+                </div>`,
+          renderSpinner,
+        )}
       </div>
     `;
   }

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

@@ -13,6 +13,7 @@ import {
   type TemplateResult,
 } from "lit";
 import { customElement, property, query, state } from "lit/decorators.js";
+import { ifDefined } from "lit/directives/if-defined.js";
 import { when } from "lit/directives/when.js";
 import queryString from "query-string";
 
@@ -681,7 +682,7 @@ export class ArchivedItemDetailQA extends TailwindElement {
             class="label-same-line"
             label=${msg("Sort by:")}
             size="small"
-            value="approved.-1"
+            value=${this.qaRunId ? "approved.-1" : "url.1"}
             pill
             @sl-change=${(e: SlChangeEvent) => {
               const { value } = e.target as SlSelect;
@@ -697,11 +698,15 @@ export class ArchivedItemDetailQA extends TailwindElement {
           >
             <sl-option value="title.1">${msg("Title")}</sl-option>
             <sl-option value="url.1">${msg("URL")}</sl-option>
-            <sl-option value="notes.-1">${msg("Most Comments")}</sl-option>
+            <sl-option value="notes.-1" ?disabled=${!this.qaRunId}
-            <sl-option value="approved.-1"
+              >${msg("Most Comments")}</sl-option
-              >${msg("Recently Approved")}</sl-option
             >
-            <sl-option value="approved.1">${msg("Not Approved")}</sl-option>
+            <sl-option value="approved.-1" ?disabled=${!this.qaRunId}>
+              ${msg("Recently Approved")}
+            </sl-option>
+            <sl-option value="approved.1" ?disabled=${!this.qaRunId}>
+              ${msg("Not Approved")}
+            </sl-option>
           </sl-select>
         </div>
       </div>
@@ -709,6 +714,13 @@ export class ArchivedItemDetailQA extends TailwindElement {
   }
 
   private renderPageList() {
+    const pageTitle = (page: ArchivedItemPage) => html`
+      <div class="truncate font-medium">
+        ${page.title ||
+        html`<span class="opacity-50">${msg("No page title")}</span>`}
+      </div>
+      <div class="truncate text-xs leading-4 text-neutral-600">${page.url}</div>
+    `;
     return html`
       <btrix-table
         class="-mx-3 overflow-x-auto px-5"
@@ -727,28 +739,25 @@ export class ArchivedItemDetailQA extends TailwindElement {
           ${this.pages?.items.map(
             (page, idx) => html`
               <btrix-table-row
-                class="${idx > 0
+                class="${idx > 0 ? "border-t" : ""} ${this.qaRunId
-                  ? "border-t"
+                  ? "cursor-pointer transition-colors focus-within:bg-neutral-50 hover:bg-neutral-50"
-                  : ""} cursor-pointer select-none transition-colors focus-within:bg-neutral-50 hover:bg-neutral-50"
+                  : ""} select-none"
               >
                 <btrix-table-cell
                   class="block overflow-hidden"
-                  rowClickTarget="a"
+                  rowClickTarget=${ifDefined(this.qaRunId ? "a" : undefined)}
                 >
+                  ${this.qaRunId
+                    ? html`
                         <a
-                    class="truncate text-sm font-semibold"
+                          href=${`${this.navigate.orgBasePath}/items/${this.itemType}/${this.crawlId}/review/screenshots?qaRunId=${this.qaRunId}&itemPageId=${page.id}`}
-                    href=${`${
+                          title=${msg(str`Review "${page.title ?? page.url}"`)}
-                      this.navigate.orgBasePath
-                    }/items/${this.itemType}/${this.crawlId}/review/screenshots?qaRunId=${
-                      this.qaRunId || ""
-                    }&itemPageId=${page.id}`}
-                    title="${page.title ?? page.url}"
                           @click=${this.navigate.link}
-                    >${page.title}</a
                         >
-                  <div class="truncate text-xs leading-4 text-neutral-600">
+                          ${pageTitle(page)}
-                    ${page.url}
+                        </a>
-                  </div>
+                      `
+                    : pageTitle(page)}
                 </btrix-table-cell>
                 <btrix-table-cell
                   >${this.renderApprovalStatus(page)}</btrix-table-cell

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

@@ -376,9 +376,7 @@ export class ArchivedItemQA extends TailwindElement {
                 ?disabled=${disableReview}
               >
                 <sl-icon slot="prefix" name="patch-check"> </sl-icon>
-                ${this.item?.reviewStatus
+                ${msg("Finish Review")}
-                  ? msg("Update Review")
-                  : msg("Finish Review")}
               </sl-button>
             </sl-tooltip>
           </div>
@@ -1151,8 +1149,12 @@ export class ArchivedItemQA extends TailwindElement {
       }
 
       void this.reviewDialog?.hide();
+
+      this.navigate.to(
+        `${this.navigate.orgBasePath}/items/crawl/${this.itemId}#qa`,
+      );
       this.notify.toast({
-        message: msg("Submitted QA review."),
+        message: msg("Saved QA review."),
         variant: "success",
         icon: "check2-circle",
       });