Fix QA tab loading state (#1772)

- Prevents misleading "no text" message shown when switching tabs
- Reset data and show loading indicator when switching pages
- Fix replay tab height and visually differentiates empty replay window
from empty tab
- Refactors console.logs to debugs
This commit is contained in:
sua yoo 2024-04-30 18:16:12 -07:00 committed by GitHub
parent c5b69f814d
commit fef2b43072
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 61 additions and 36 deletions

View File

@ -176,7 +176,7 @@ export class ArchivedItemQA extends TailwindElement {
// Receive messages from replay-web-page windows // Receive messages from replay-web-page windows
void this.replaySwReg.then((reg) => { void this.replaySwReg.then((reg) => {
if (!reg) { if (!reg) {
console.log("[debug] no reg, listening to messages"); console.debug("no reg, listening to messages");
// window.addEventListener("message", this.onWindowMessage); // window.addEventListener("message", this.onWindowMessage);
} }
}); });
@ -207,17 +207,17 @@ export class ArchivedItemQA extends TailwindElement {
* if the sw is not present. * if the sw is not present.
*/ */
private async handleRwpMessage(sourceLoc: string) { private async handleRwpMessage(sourceLoc: string) {
console.log("[debug] handleRwpMessage", sourceLoc); console.debug("handleRwpMessage", sourceLoc);
// check if has /qa/ in path, then QA // check if has /qa/ in path, then QA
if (sourceLoc.indexOf("%2Fqa%2F") >= 0 && !this.qaDataRegistered) { if (sourceLoc.indexOf("%2Fqa%2F") >= 0 && !this.qaDataRegistered) {
this.qaDataRegistered = true; this.qaDataRegistered = true;
console.log("[debug] onWindowMessage qa", this.qaData); console.debug("onWindowMessage qa", this.qaData);
await this.fetchContentForTab({ qa: true }); await this.fetchContentForTab({ qa: true });
await this.updateComplete; await this.updateComplete;
// otherwise main crawl replay // otherwise main crawl replay
} else if (!this.crawlDataRegistered) { } else if (!this.crawlDataRegistered) {
this.crawlDataRegistered = true; this.crawlDataRegistered = true;
console.log("[debug] onWindowMessage crawl", this.crawlData); console.debug("onWindowMessage crawl", this.crawlData);
await this.fetchContentForTab(); await this.fetchContentForTab();
await this.updateComplete; await this.updateComplete;
} }
@ -248,15 +248,23 @@ export class ArchivedItemQA extends TailwindElement {
// Re-fetch when tab, archived item page, or QA run ID changes // Re-fetch when tab, archived item page, or QA run ID changes
// from an existing one, probably due to user interaction // from an existing one, probably due to user interaction
if (changedProperties.get("tab") || changedProperties.get("page")) { if (changedProperties.get("tab") || changedProperties.get("page")) {
if (this.tab === "screenshots") { if (changedProperties.get("page")) {
if (this.crawlData?.blobUrl) if (this.crawlData?.blobUrl)
URL.revokeObjectURL(this.crawlData.blobUrl); URL.revokeObjectURL(this.crawlData.blobUrl);
if (this.qaData?.blobUrl) URL.revokeObjectURL(this.qaData.blobUrl); if (this.qaData?.blobUrl) URL.revokeObjectURL(this.qaData.blobUrl);
// FIXME Set to null to render loading state, should be refactored
// to handle loading state separately in https://github.com/webrecorder/browsertrix/issues/1716
this.crawlData = null;
this.qaData = null;
} }
// TODO prefetch content for other tabs? // TODO prefetch content for other tabs?
void this.fetchContentForTab(); void this.fetchContentForTab();
void this.fetchContentForTab({ qa: true }); void this.fetchContentForTab({ qa: true });
} else if (changedProperties.get("qaRunId")) { } else if (changedProperties.get("qaRunId")) {
// FIXME Set to null to render loading state, should be refactored
// to handle loading state separately in https://github.com/webrecorder/browsertrix/issues/1716
this.qaData = null;
void this.fetchContentForTab({ qa: true }); void this.fetchContentForTab({ qa: true });
} }
} }
@ -867,7 +875,7 @@ export class ArchivedItemQA extends TailwindElement {
const replaySource = `/api/orgs/${this.orgId}/crawls/${this.itemId}${qa ? `/qa/${rwpId}` : ""}/replay.json`; const replaySource = `/api/orgs/${this.orgId}/crawls/${this.itemId}${qa ? `/qa/${rwpId}` : ""}/replay.json`;
const headers = this.authState?.headers; const headers = this.authState?.headers;
const config = JSON.stringify({ headers }); const config = JSON.stringify({ headers });
console.log("[debug] rendering rwp", rwpId); console.debug("rendering rwp", rwpId);
return guard( return guard(
[rwpId, this.page, this.authState], [rwpId, this.page, this.authState],
() => html` () => html`
@ -1107,8 +1115,8 @@ export class ArchivedItemQA extends TailwindElement {
const frameWindow = this.replayFrame?.contentWindow; const frameWindow = this.replayFrame?.contentWindow;
if (!page || !sourceId || !frameWindow) { if (!page || !sourceId || !frameWindow) {
console.log( console.debug(
"[debug] no page replaId or frameWindow", "no page replaId or frameWindow",
page, page,
sourceId, sourceId,
frameWindow, frameWindow,
@ -1129,9 +1137,8 @@ export class ArchivedItemQA extends TailwindElement {
const url = `/replay/w/${sourceId}/${urlPart}`; const url = `/replay/w/${sourceId}/${urlPart}`;
// TODO check status code // TODO check status code
if (tab === "replay") { console.log("replay?");
return { replayUrl: url };
}
const resp = await frameWindow.fetch(url); const resp = await frameWindow.fetch(url);
//console.log("resp:", resp); //console.log("resp:", resp);
@ -1140,14 +1147,19 @@ export class ArchivedItemQA extends TailwindElement {
throw resp.status; throw resp.status;
} }
if (tab === "replay") {
return { replayUrl: url };
}
if (tab === "screenshots") { if (tab === "screenshots") {
const blob = await resp.blob(); const blob = await resp.blob();
const blobUrl = URL.createObjectURL(blob) || ""; const blobUrl = URL.createObjectURL(blob) || "";
return { blobUrl }; return { blobUrl };
} else if (tab === "text") { }
if (tab === "text") {
const text = await resp.text(); const text = await resp.text();
return { text }; return { text };
} else { }
{
// tab === "resources" // tab === "resources"
const json = (await resp.json()) as { const json = (await resp.json()) as {
@ -1197,7 +1209,6 @@ export class ArchivedItemQA extends TailwindElement {
return { resources: Object.fromEntries(typeMap.entries()) }; return { resources: Object.fromEntries(typeMap.entries()) };
} }
return { text: "" };
}; };
try { try {
@ -1217,7 +1228,7 @@ export class ArchivedItemQA extends TailwindElement {
this.crawlDataRegistered = true; this.crawlDataRegistered = true;
} }
} catch (e: unknown) { } catch (e: unknown) {
console.log("[debug] error:", e); console.debug("error:", e);
// check if this endpoint is registered, if not, ensure re-render // check if this endpoint is registered, if not, ensure re-render
if (e === 404) { if (e === 404) {

View File

@ -11,7 +11,7 @@ import { tw } from "@/utils/tailwind";
export function renderReplay(crawlData: ReplayData) { export function renderReplay(crawlData: ReplayData) {
return html` return html`
<div <div
class=${tw`relative aspect-video overflow-hidden rounded-b-lg border-x border-b`} class=${tw`relative h-full overflow-hidden rounded-b-lg border-x border-b bg-slate-100 p-4 shadow-inner`}
> >
${guard([crawlData], () => ${guard([crawlData], () =>
when( when(
@ -19,7 +19,7 @@ export function renderReplay(crawlData: ReplayData) {
(replayUrl) => (replayUrl) =>
html`<iframe html`<iframe
src=${replayUrl} src=${replayUrl}
class=${tw`h-full w-full outline`} class=${tw`h-full w-full overflow-hidden rounded border bg-neutral-0 shadow-lg`}
></iframe>`, ></iframe>`,
renderSpinner, renderSpinner,
), ),

View File

@ -92,20 +92,18 @@ function renderDiff(
} }
export function renderResources(crawlData: ReplayData, qaData: ReplayData) { export function renderResources(crawlData: ReplayData, qaData: ReplayData) {
const noData = html`<div // const noData = html`<div
class="m-4 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> // <sl-icon name="slash-circle"></sl-icon>
${msg("Resources data not available")} // ${msg("Resources data not available")}
</div>`; // </div>`;
return html` return html`
<div class="flex h-full flex-col outline"> <div class="flex h-full flex-col outline">
<div class="flex-1 overflow-auto overscroll-contain"> <div class="flex-1 overflow-auto overscroll-contain">
${crawlData && qaData ${crawlData?.resources && qaData?.resources
? crawlData.resources && qaData.resources ? renderDiff(crawlData.resources, qaData.resources)
? renderDiff(crawlData.resources, qaData.resources)
: noData
: renderSpinner()} : renderSpinner()}
</div> </div>
<footer class="pt-2 text-xs text-neutral-600"> <footer class="pt-2 text-xs text-neutral-600">

View File

@ -4,12 +4,12 @@ import { html } from "lit";
import { guard } from "lit/directives/guard.js"; import { guard } from "lit/directives/guard.js";
import { when } from "lit/directives/when.js"; import { when } from "lit/directives/when.js";
import type { ReplayData } from "../types"; import type { BlobPayload, ReplayData } from "../types";
import { renderSpinner } from "./spinner"; import { renderSpinner } from "./spinner";
function image(data: ReplayData) { function image(blobUrl: BlobPayload["blobUrl"]) {
if (!data?.blobUrl) { if (!blobUrl) {
return html`<div 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" class="flex aspect-video h-full w-full flex-col items-center justify-center gap-2 bg-slate-50 text-xs text-neutral-500"
> >
@ -23,7 +23,7 @@ function image(data: ReplayData) {
width="1920" width="1920"
height="1080" height="1080"
alt="" alt=""
src=${data.blobUrl} src=${blobUrl}
/> />
`; `;
} }
@ -60,13 +60,21 @@ export function renderScreenshots(
class="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" aria-labelledby="crawlScreenshotHeading"
> >
${when(crawlData, image, renderSpinner)} ${when(
crawlData?.blobUrl !== undefined && crawlData.blobUrl,
image,
renderSpinner,
)}
</div> </div>
<div <div
class="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" aria-labelledby="qaScreenshotHeading"
> >
${when(qaData, image, renderSpinner)} ${when(
qaData?.blobUrl !== undefined && qaData.blobUrl,
image,
renderSpinner,
)}
</div> </div>
</div>` </div>`
: html` : html`
@ -75,10 +83,18 @@ export function renderScreenshots(
> >
<sl-image-comparer class="h-full w-full"> <sl-image-comparer class="h-full w-full">
<div slot="after" aria-labelledby="crawlScreenshotHeading"> <div slot="after" aria-labelledby="crawlScreenshotHeading">
${when(crawlData, image, renderSpinner)} ${when(
crawlData?.blobUrl !== undefined && crawlData.blobUrl,
image,
renderSpinner,
)}
</div> </div>
<div slot="before" aria-labelledby="qaScreenshotHeading"> <div slot="before" aria-labelledby="qaScreenshotHeading">
${when(qaData, image, renderSpinner)} ${when(
qaData?.blobUrl !== undefined && qaData.blobUrl,
image,
renderSpinner,
)}
</div> </div>
</sl-image-comparer> </sl-image-comparer>
</div> </div>

View File

@ -86,7 +86,7 @@ export function renderText(crawlData: ReplayData, qaData: ReplayData) {
> >
${guard([crawlData, qaData], () => ${guard([crawlData, qaData], () =>
when( when(
crawlData && qaData, crawlData?.text !== undefined && qaData?.text !== undefined,
() => html` () => html`
<div <div
class=${tw`flex min-h-full ${crawlData?.text && qaData?.text ? "" : tw`items-center justify-center`}`} class=${tw`flex min-h-full ${crawlData?.text && qaData?.text ? "" : tw`items-center justify-center`}`}