diff --git a/plugins/thread-hover-cards/app.tsx b/plugins/thread-hover-cards/app.tsx index 4327513..bcb622d 100644 --- a/plugins/thread-hover-cards/app.tsx +++ b/plugins/thread-hover-cards/app.tsx @@ -821,6 +821,49 @@ function renderError(card: HTMLElement): void { ); } +type HoverCardRenderState = "complete" | "error" | "loading" | "summary"; + +function setHoverCardRenderState( + card: HTMLElement, + state: HoverCardRenderState, +): void { + card.dataset.bbHoverCardRenderState = state; + if (state === "loading" || state === "summary") { + card.setAttribute("aria-busy", "true"); + } else { + card.removeAttribute("aria-busy"); + } +} + +function nextPaint(): Promise { + return new Promise((resolve) => requestAnimationFrame(() => resolve())); +} + +async function waitForCardAssets(card: HTMLElement): Promise { + const fonts = document.fonts?.ready; + const images = Array.from(card.querySelectorAll("img")); + await Promise.all([ + fonts ?? Promise.resolve(), + ...images.map((image) => { + if (image.complete) return image.decode?.().catch(() => undefined); + return new Promise((resolve) => { + image.addEventListener("load", () => resolve(), { once: true }); + image.addEventListener("error", () => resolve(), { once: true }); + }); + }), + ]); +} + +async function markHoverCardComplete( + card: HTMLElement, + isCurrent: () => boolean, +): Promise { + await waitForCardAssets(card); + await nextPaint(); + await nextPaint(); + if (isCurrent()) setHoverCardRenderState(card, "complete"); +} + function renderSummary(card: HTMLElement, summary: ThreadSummary): void { const header = element("div", "bb-thread-hover-card__header"); const provider = element("div", "bb-thread-hover-card__provider"); @@ -1497,7 +1540,7 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl threadId: string, generation: number, hoverCard: HTMLDivElement, - ): void { + ): Promise { const cached = cache.get(threadId); if ( cached?.timingFetchedAt !== null && @@ -1511,10 +1554,10 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl outcome: "ok", threadId, }); - return; + return Promise.resolve(); } - void requestTiming(threadId) + return requestTiming(threadId) .then((timing) => { const current = cache.get(threadId); if (!current) return; @@ -1530,9 +1573,11 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl ) { timingRetriedForSummary.set(threadId, summaryStartedAt); timingRetryScheduled.add(threadId); - queueMicrotask(() => { - timingRetryScheduled.delete(threadId); - refreshTiming(threadId, generation, hoverCard); + return new Promise((resolve) => { + queueMicrotask(() => { + timingRetryScheduled.delete(threadId); + void refreshTiming(threadId, generation, hoverCard).then(resolve); + }); }); } return; @@ -1578,9 +1623,9 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl threadId: string, generation: number, hoverCard: HTMLDivElement, - ): void { + ): Promise { const cached = cache.get(threadId); - if (!cached?.summary.repository.isGitRepository) return; + if (!cached?.summary.repository.isGitRepository) return Promise.resolve(); const repositoryKey = pullRequestRepositoryIdentity( cached.summary.repository, ); @@ -1595,10 +1640,10 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl outcome: "ok", threadId, }); - return; + return Promise.resolve(); } - void requestPullRequest(threadId, repositoryKey) + return requestPullRequest(threadId, repositoryKey) .then(({ pullRequest, repository }) => { const current = cache.get(threadId); if ( @@ -1641,6 +1686,27 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl .catch(() => undefined); } + function finishRendering( + threadId: string, + generation: number, + hoverCard: HTMLDivElement, + ): void { + setHoverCardRenderState(hoverCard, "summary"); + void Promise.all([ + refreshTiming(threadId, generation, hoverCard), + refreshPullRequest(threadId, generation, hoverCard), + ]).then(() => + markHoverCardComplete( + hoverCard, + () => + !disposed && + generation === requestGeneration && + activeThreadId === threadId && + resolveActiveTrigger() !== null, + ), + ); + } + function resolveActiveTrigger(): HTMLAnchorElement | null { if (!activeThreadId) return null; if ( @@ -1723,6 +1789,7 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl const generation = requestGeneration; abortSupersededSummaryRequests(threadId); const hoverCard = ensureCard(); + setHoverCardRenderState(hoverCard, "loading"); hoverCard.hidden = false; hoverCard.classList.remove("is-visible"); void hoverCard.offsetWidth; @@ -1756,8 +1823,7 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl threadId, }); if (cacheIsFresh) { - refreshTiming(threadId, generation, hoverCard); - refreshPullRequest(threadId, generation, hoverCard); + finishRendering(threadId, generation, hoverCard); return; } } @@ -1799,8 +1865,7 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl (replacementPullRequestLink ?? resolveActiveTrigger())?.focus(); } requestAnimationFrame(positionCard); - refreshTiming(threadId, generation, hoverCard); - refreshPullRequest(threadId, generation, hoverCard); + finishRendering(threadId, generation, hoverCard); }) .catch((error) => { if ( @@ -1811,6 +1876,7 @@ function installHoverCards({ onOpen }: ThreadHoverCardOptions): HoverCardControl resolveActiveTrigger() ) { renderError(hoverCard); + setHoverCardRenderState(hoverCard, "error"); recordTiming({ cache: "miss", durationMs: elapsedMs(requestedAt), @@ -2226,16 +2292,24 @@ function installSectionHoverCards({ generation += 1; const requestGeneration = generation; const hoverCard = ensureCard(); + setHoverCardRenderState(hoverCard, "loading"); hoverCard.hidden = false; hoverCard.classList.remove("is-visible"); void hoverCard.offsetWidth; hoverCard.classList.add("is-visible"); const key = keyOf(target); const cached = cache.get(key); - if (cached) renderSectionSummary(hoverCard, cached.summary); - else renderLoading(hoverCard, "section"); + if (cached) { + renderSectionSummary(hoverCard, cached.summary); + setHoverCardRenderState(hoverCard, "summary"); + } else renderLoading(hoverCard, "section"); requestAnimationFrame(position); if (cached && Date.now() - cached.fetchedAt < SECTION_SUMMARY_CACHE_TTL_MS) { + void markHoverCardComplete( + hoverCard, + () => + !disposed && requestGeneration === generation && active !== null, + ); return; } @@ -2248,11 +2322,20 @@ function installSectionHoverCards({ if (disposed || requestGeneration !== generation) return; renderSectionSummary(hoverCard, summary); requestAnimationFrame(position); + setHoverCardRenderState(hoverCard, "summary"); + void markHoverCardComplete( + hoverCard, + () => + !disposed && + requestGeneration === generation && + active !== null, + ); }) .catch((error) => { if (disposed || requestGeneration !== generation || cached) return; if (isAbortError(error)) return; renderError(hoverCard); + setHoverCardRenderState(hoverCard, "error"); requestAnimationFrame(position); }); } diff --git a/plugins/thread-hover-cards/dist/app.js b/plugins/thread-hover-cards/dist/app.js index ff0c961..e121b8d 100644 --- a/plugins/thread-hover-cards/dist/app.js +++ b/plugins/thread-hover-cards/dist/app.js @@ -1778,6 +1778,37 @@ function renderError(card) { element("p", "bb-thread-hover-card__loading", "Summary unavailable") ); } +function setHoverCardRenderState(card, state) { + card.dataset.bbHoverCardRenderState = state; + if (state === "loading" || state === "summary") { + card.setAttribute("aria-busy", "true"); + } else { + card.removeAttribute("aria-busy"); + } +} +function nextPaint() { + return new Promise((resolve) => requestAnimationFrame(() => resolve())); +} +async function waitForCardAssets(card) { + const fonts = document.fonts?.ready; + const images = Array.from(card.querySelectorAll("img")); + await Promise.all([ + fonts ?? Promise.resolve(), + ...images.map((image) => { + if (image.complete) return image.decode?.().catch(() => void 0); + return new Promise((resolve) => { + image.addEventListener("load", () => resolve(), { once: true }); + image.addEventListener("error", () => resolve(), { once: true }); + }); + }) + ]); +} +async function markHoverCardComplete(card, isCurrent) { + await waitForCardAssets(card); + await nextPaint(); + await nextPaint(); + if (isCurrent()) setHoverCardRenderState(card, "complete"); +} function renderSummary(card, summary) { const header = element("div", "bb-thread-hover-card__header"); const provider = element("div", "bb-thread-hover-card__provider"); @@ -2339,9 +2370,9 @@ function installHoverCards({ onOpen }) { outcome: "ok", threadId }); - return; + return Promise.resolve(); } - void requestTiming(threadId).then((timing) => { + return requestTiming(threadId).then((timing) => { const current = cache.get(threadId); if (!current) return; const summaryStartedAt = current.summary.diagnostics.startedAt; @@ -2349,9 +2380,11 @@ function installHoverCards({ onOpen }) { if (!disposed && generation === requestGeneration && activeThreadId === threadId && resolveActiveTrigger() && timingRetriedForSummary.get(threadId) !== summaryStartedAt && !timingRetryScheduled.has(threadId)) { timingRetriedForSummary.set(threadId, summaryStartedAt); timingRetryScheduled.add(threadId); - queueMicrotask(() => { - timingRetryScheduled.delete(threadId); - refreshTiming(threadId, generation, hoverCard); + return new Promise((resolve) => { + queueMicrotask(() => { + timingRetryScheduled.delete(threadId); + void refreshTiming(threadId, generation, hoverCard).then(resolve); + }); }); } return; @@ -2384,7 +2417,7 @@ function installHoverCards({ onOpen }) { } function refreshPullRequest(threadId, generation, hoverCard) { const cached = cache.get(threadId); - if (!cached?.summary.repository.isGitRepository) return; + if (!cached?.summary.repository.isGitRepository) return Promise.resolve(); const repositoryKey = pullRequestRepositoryIdentity( cached.summary.repository ); @@ -2396,9 +2429,9 @@ function installHoverCards({ onOpen }) { outcome: "ok", threadId }); - return; + return Promise.resolve(); } - void requestPullRequest(threadId, repositoryKey).then(({ pullRequest, repository }) => { + return requestPullRequest(threadId, repositoryKey).then(({ pullRequest, repository }) => { const current = cache.get(threadId); if (!current || pullRequestRepositoryIdentity(repository) !== repositoryKey || pullRequestRepositoryIdentity(current.summary.repository) !== repositoryKey) { return; @@ -2424,6 +2457,18 @@ function installHoverCards({ onOpen }) { requestAnimationFrame(positionCard); }).catch(() => void 0); } + function finishRendering(threadId, generation, hoverCard) { + setHoverCardRenderState(hoverCard, "summary"); + void Promise.all([ + refreshTiming(threadId, generation, hoverCard), + refreshPullRequest(threadId, generation, hoverCard) + ]).then( + () => markHoverCardComplete( + hoverCard, + () => !disposed && generation === requestGeneration && activeThreadId === threadId && resolveActiveTrigger() !== null + ) + ); + } function resolveActiveTrigger() { if (!activeThreadId) return null; if (activeTrigger?.isConnected && threadIdFor(activeTrigger) === activeThreadId) { @@ -2475,6 +2520,7 @@ function installHoverCards({ onOpen }) { const generation = requestGeneration; abortSupersededSummaryRequests(threadId); const hoverCard = ensureCard(); + setHoverCardRenderState(hoverCard, "loading"); hoverCard.hidden = false; hoverCard.classList.remove("is-visible"); void hoverCard.offsetWidth; @@ -2504,8 +2550,7 @@ function installHoverCards({ onOpen }) { threadId }); if (cacheIsFresh) { - refreshTiming(threadId, generation, hoverCard); - refreshPullRequest(threadId, generation, hoverCard); + finishRendering(threadId, generation, hoverCard); return; } } @@ -2537,11 +2582,11 @@ function installHoverCards({ onOpen }) { (replacementPullRequestLink ?? resolveActiveTrigger())?.focus(); } requestAnimationFrame(positionCard); - refreshTiming(threadId, generation, hoverCard); - refreshPullRequest(threadId, generation, hoverCard); + finishRendering(threadId, generation, hoverCard); }).catch((error) => { if (!cached && !disposed && generation === requestGeneration && activeThreadId === threadId && resolveActiveTrigger()) { renderError(hoverCard); + setHoverCardRenderState(hoverCard, "error"); recordTiming({ cache: "miss", durationMs: elapsedMs(requestedAt), @@ -2864,16 +2909,23 @@ function installSectionHoverCards({ generation += 1; const requestGeneration = generation; const hoverCard = ensureCard(); + setHoverCardRenderState(hoverCard, "loading"); hoverCard.hidden = false; hoverCard.classList.remove("is-visible"); void hoverCard.offsetWidth; hoverCard.classList.add("is-visible"); const key = keyOf(target); const cached = cache.get(key); - if (cached) renderSectionSummary(hoverCard, cached.summary); - else renderLoading(hoverCard, "section"); + if (cached) { + renderSectionSummary(hoverCard, cached.summary); + setHoverCardRenderState(hoverCard, "summary"); + } else renderLoading(hoverCard, "section"); requestAnimationFrame(position); if (cached && Date.now() - cached.fetchedAt < SECTION_SUMMARY_CACHE_TTL_MS) { + void markHoverCardComplete( + hoverCard, + () => !disposed && requestGeneration === generation && active !== null + ); return; } void requestSummary(target).then((summary) => { @@ -2884,10 +2936,16 @@ function installSectionHoverCards({ if (disposed || requestGeneration !== generation) return; renderSectionSummary(hoverCard, summary); requestAnimationFrame(position); + setHoverCardRenderState(hoverCard, "summary"); + void markHoverCardComplete( + hoverCard, + () => !disposed && requestGeneration === generation && active !== null + ); }).catch((error) => { if (disposed || requestGeneration !== generation || cached) return; if (isAbortError(error)) return; renderError(hoverCard); + setHoverCardRenderState(hoverCard, "error"); requestAnimationFrame(position); }); } diff --git a/plugins/thread-hover-cards/test/app.test.mjs b/plugins/thread-hover-cards/test/app.test.mjs index 256bdfc..ebc76dc 100644 --- a/plugins/thread-hover-cards/test/app.test.mjs +++ b/plugins/thread-hover-cards/test/app.test.mjs @@ -594,6 +594,8 @@ trigger.dispatchEvent(pointerOver); const card = window.document.getElementById("bb-thread-hover-card"); assert.ok(card, "opens the hover card in the pointer event turn"); assert.equal(card.hidden, false); +assert.equal(card.dataset.bbHoverCardRenderState, "loading"); +assert.equal(card.getAttribute("aria-busy"), "true"); assert.deepEqual( requestBodies, [{ threadId: "thr_1" }], @@ -610,6 +612,8 @@ assert.match(card.textContent, /Loading thread summary/); await new Promise((resolve) => setTimeout(resolve, 20)); assert.equal(card.hidden, false); +assert.equal(card.dataset.bbHoverCardRenderState, "complete"); +assert.equal(card.hasAttribute("aria-busy"), false); assert.equal(card.dataset.bbPlugin, "thread-hover-cards"); assert.equal(card.hasAttribute("data-bb-portaled-overlay"), true); assert.equal(trigger.getAttribute("aria-describedby"), "bb-thread-hover-card"); @@ -1424,12 +1428,20 @@ testNow += 2_100; delayNextTimingFor.add("thr_summary_before_timing"); await closeAndOpenThread("thr_summary_before_timing", 10); assert.ok(delayedTimingResponses.has("thr_summary_before_timing")); +assert.equal( + reloadedCard.dataset.bbHoverCardRenderState, + "summary", + "does not advertise screenshot readiness while timing is still pending", +); +assert.equal(reloadedCard.getAttribute("aria-busy"), "true"); assert.ok( reloadedCard.querySelector(".bb-thread-hover-card__runtime"), "keeps hydrated runtime while summary resolves before timing", ); delayedTimingResponses.get("thr_summary_before_timing")?.(); await new Promise((resolve) => setTimeout(resolve, 20)); +assert.equal(reloadedCard.dataset.bbHoverCardRenderState, "complete"); +assert.equal(reloadedCard.hasAttribute("aria-busy"), false); assert.ok(reloadedCard.querySelector(".bb-thread-hover-card__runtime")); assert.equal( timingRequestBodies.filter( @@ -1526,6 +1538,18 @@ assert.doesNotMatch( "ignores a late PR response for the previous branch", ); +delayNextPullRequestFor.add("thr_slow_pr"); +await closeAndOpenThread("thr_slow_pr", 20); +assert.ok(delayedPullRequestResponses.has("thr_slow_pr")); +assert.equal( + reloadedCard.dataset.bbHoverCardRenderState, + "summary", + "does not advertise screenshot readiness while PR content is still pending", +); +delayedPullRequestResponses.get("thr_slow_pr")?.(); +await new Promise((resolve) => setTimeout(resolve, 20)); +assert.equal(reloadedCard.dataset.bbHoverCardRenderState, "complete"); + for (const expected of [ { threadId: "thr_blocked_pr", @@ -1699,6 +1723,7 @@ await new Promise((resolve) => setTimeout(resolve, 20)); const sectionCard = window.document.getElementById("bb-section-hover-card"); assert.ok(sectionCard, "opens a card from the section header row"); assert.equal(sectionCard.hidden, false); +assert.equal(sectionCard.dataset.bbHoverCardRenderState, "complete"); assert.equal( designHeader.toggle.getAttribute("aria-describedby"), "bb-section-hover-card", @@ -1769,6 +1794,20 @@ sectionGroup.append(delayedAHeader.row, delayedBHeader.row); hoverOver(delayedBHeader.title); await new Promise((resolve) => setTimeout(resolve, 20)); assert.match(sectionCard.textContent, /2 threads/); + +delayNextSectionFor.add("Delayed A"); +hoverOver(delayedAHeader.title); +await new Promise((resolve) => setTimeout(resolve, 0)); +assert.equal(sectionCard.dataset.bbHoverCardRenderState, "loading"); +assert.equal(sectionCard.getAttribute("aria-busy"), "true"); +delayedSectionResponses.get("Delayed A")?.(); +await new Promise((resolve) => setTimeout(resolve, 20)); +assert.equal(sectionCard.dataset.bbHoverCardRenderState, "complete"); +assert.equal(sectionCard.hasAttribute("aria-busy"), false); + +hoverOver(delayedBHeader.title); +await new Promise((resolve) => setTimeout(resolve, 20)); +testNow += 4_100; delayNextSectionFor.add("Delayed A"); hoverOver(delayedAHeader.title); await new Promise((resolve) => setTimeout(resolve, 0));