Compare commits

...

3 Commits

Author SHA1 Message Date
J
41811f0deb perf(issues): reconcile inbox issue-detail in place; defer comment composer (MUL-3841)
Phase 3 of the inbox switch-jank fix, after timeline virtualization still left
a freeze. Removes the two remaining synchronous costs on switch.

1. Drop the `key={issue_id}` on the inbox's <IssueDetail>. It forced a full
   unmount + remount of the entire subtree (sidebar, every picker, layout
   panels, breadcrumb) on every click. Without the key, switching issues just
   changes the `issueId` prop and React reconciles in place — the way the
   full-page issue route (`/issues/[id]`, desktop) already drives IssueDetail.
   Safe because IssueDetail resets its own per-issue state on id change:
   editors are keyed by id internally, optional-prop set re-seeds via
   seededIssueIdRef, desc attachments reset on [id], didHighlightRef
   self-corrects by comparison, and the Virtuoso timeline keeps its own
   key={wsId:id} so highlight landing still works. ErrorBoundary still resets
   per issue.

2. Defer the bottom comment composer one frame past paint via useDeferredMount
   (same mechanism as the description editor), so its Tiptap EditorView is no
   longer created inside the synchronous switch commit. A static placeholder of
   similar height holds the spot — no manual expand control, it hydrates on its
   own. This relies on last commit's render-phase reset so the deferral re-arms
   correctly now that the pane reconciles instead of remounting.

Tests: issue-detail asserts the composer still hydrates after paint. Verified:
@multica/views typecheck + eslint clean; vitest issues/components + inbox +
readonly-content = 172 passed.

Co-authored-by: multica-agent <github@multica.ai>
2026-06-29 21:26:04 +08:00
J
4b2394e1a9 perf(issues): virtualize long highlighted timelines; fix deferred-mount reset (MUL-3841)
Addresses the review on #4713 and the still-janky repro: the right-pane freeze
on inbox switch was the un-virtualized timeline, not the description editor.

1. Inbox comment notifications always carry a comment_id, which becomes
   `highlightCommentId` on IssueDetail — and the highlight path rendered the
   ENTIRE timeline flat (`items.map`), synchronously mounting every CommentCard
   (markdown + lowlight) on entry and unmounting them on exit. That is the
   multi-second freeze on comment-heavy issues. Now: short timelines stay flat
   (precise, cheap landing — unchanged); past FLAT_TIMELINE_LIMIT the highlight
   hands off to Virtuoso positioned on the target via `initialTopMostItemIndex`,
   so we never mount a long list up front. The deep-link center() scroll is
   gated to the flat path (Virtuoso owns its scroller); the highlight tint
   still applies in both.

2. useDeferredMount reset was effect-based, so on routes that do NOT remount on
   key change (the full-page issue route) the heavy child mounted synchronously
   on the first render and unmounted a tick later — the opposite of the goal.
   The tracked key now lives in state and the returned `ready` is derived from
   the comparison, so the first render after a key change always reports false.

Tests: long highlighted timeline uses Virtuoso (not flat); short stays flat;
useDeferredMount never reports ready on the first render after a key change.
Verified: @multica/views typecheck + eslint clean; vitest issues/components +
inbox = 145 passed.

Co-authored-by: multica-agent <github@multica.ai>
2026-06-29 20:22:09 +08:00
J
573e2f5f9d perf(issues): defer description editor mount and stagger inbox issue switch (MUL-3841)
Phase 1 of the inbox issue-switch jank fix. Two changes, both targeting the
synchronous main-thread work that runs when the inbox detail pane remounts
IssueDetail (it is keyed by issue_id, so switching = full remount).

1. Defer the description Tiptap editor past first paint via a new
   useDeferredMount hook. Until it hydrates we render the description as
   read-only markdown (visually identical, near-zero cost), keeping the heaviest
   ProseMirror EditorView creation off the critical switch commit. A mousedown on
   the placeholder upgrades it immediately so quick click-to-edit still lands;
   the automatic post-paint mount never steals focus.

2. Drive the detail pane off a useDeferredValue copy of the selection. The row
   highlight and mark-read stay urgent on click; the expensive remount renders at
   transition priority, so React keeps the previous detail painted and can drop
   intermediate work when the user clicks through the list quickly.

The comment composer is intentionally left untouched — it has a deliberate
"no manual expand control" product decision (covered by comment-composers.test).

Verified: pnpm --filter @multica/views typecheck; eslint on touched files;
vitest for use-deferred-mount, issue-detail (33/33), comment-composers,
comment-card, readonly-content, and inbox — all green.

Co-authored-by: multica-agent <github@multica.ai>
2026-06-29 19:56:42 +08:00
5 changed files with 418 additions and 109 deletions

View File

@@ -1,6 +1,6 @@
"use client";
import { useState, useEffect, useCallback, useMemo, useRef } from "react";
import { useState, useEffect, useCallback, useMemo, useRef, useDeferredValue } from "react";
import { useDefaultLayout } from "react-resizable-panels";
import { useQuery } from "@tanstack/react-query";
import { useWorkspaceId } from "@multica/core/hooks";
@@ -75,6 +75,17 @@ export function InboxPage() {
const selected = items.find((i) => (i.issue_id ?? i.id) === selectedKey) ?? null;
// The detail pane mounts a heavy IssueDetail subtree (keyed by issue_id, so
// it fully remounts on switch). Drive it off a deferred copy of the selection
// so clicking a row stays urgent — the list highlight + mark-read fire
// immediately on `selectedKey` — while the expensive remount renders at
// transition priority. React keeps the previous detail painted until the new
// one is ready and can drop intermediate work when the user clicks through
// the list quickly, which is what removes the "frozen on click" feel.
const deferredSelectedKey = useDeferredValue(selectedKey);
const detailSelected =
items.find((i) => (i.issue_id ?? i.id) === deferredSelectedKey) ?? null;
// Track the last key we actually resolved against the inbox list. Lets the
// fallback effect distinguish "shared-link to a notification not in our
// inbox" (never resolved → redirect to the issue page) from "item was in
@@ -286,18 +297,21 @@ export function InboxPage() {
</div>
);
const detailContent = selected?.issue_id ? (
// Key by issue_id (not inbox-item id): a new comment/reaction generates a
// new inbox notification for the same issue, and the dedup helper picks the
// newest one — keying on its id would remount IssueDetail on every event,
// wiping the comment composer draft and resetting scroll position.
<ErrorBoundary resetKeys={[selected.issue_id]}>
const detailContent = detailSelected?.issue_id ? (
// No `key` on IssueDetail: switching issues updates `issueId` in place and
// lets React reconcile the heavy subtree (sidebar, pickers, layout) instead
// of tearing it down and rebuilding it on every click — the full-page issue
// route already drives IssueDetail this way. IssueDetail resets its own
// per-issue state on issueId change (description/comment editors are keyed
// by id internally, optional-prop + attachment state reset via effects).
// ErrorBoundary still resets per issue so one issue's render error doesn't
// stick to the next.
<ErrorBoundary resetKeys={[detailSelected.issue_id]}>
<IssueDetail
key={selected.issue_id}
issueId={selected.issue_id}
issueId={detailSelected.issue_id}
defaultSidebarOpen={false}
layoutId="multica_inbox_issue_detail_layout"
highlightCommentId={selected.details?.comment_id ?? undefined}
highlightCommentId={detailSelected.details?.comment_id ?? undefined}
onDelete={() => {
// Issue deletion CASCADE-deletes the inbox item server-side, and the
// issue:deleted WS event prunes it from the inbox cache. Just clear
@@ -306,31 +320,31 @@ export function InboxPage() {
setSelectedKey("");
}}
onDone={() => {
handleArchive(selected.id);
handleArchive(detailSelected.id);
}}
/>
</ErrorBoundary>
) : selected ? (
) : detailSelected ? (
<div className="p-6">
<h2 className="text-lg font-semibold">{getInboxDisplayTitle(selected)}</h2>
<h2 className="text-lg font-semibold">{getInboxDisplayTitle(detailSelected)}</h2>
<p className="mt-1 text-sm text-muted-foreground">
{typeLabels[selected.type]} · {timeAgo(selected.created_at)}
{typeLabels[detailSelected.type]} · {timeAgo(detailSelected.created_at)}
</p>
{selected.body && (
{detailSelected.body && (
<div className="mt-4 whitespace-pre-wrap text-sm leading-relaxed text-foreground/80">
{selected.body}
{detailSelected.body}
</div>
)}
{selected.type === "quick_create_failed" && selected.details?.original_prompt && (
{detailSelected.type === "quick_create_failed" && detailSelected.details?.original_prompt && (
<div className="mt-4 rounded-md border bg-muted/40 p-3">
<p className="text-xs font-medium text-muted-foreground">
{t(($) => $.detail.original_input)}
</p>
<p className="mt-1 whitespace-pre-wrap text-sm">{selected.details.original_prompt}</p>
<p className="mt-1 whitespace-pre-wrap text-sm">{detailSelected.details.original_prompt}</p>
</div>
)}
<div className="mt-4 flex gap-2">
{selected.type === "quick_create_failed" && (
{detailSelected.type === "quick_create_failed" && (
<Button
size="sm"
onClick={() => {
@@ -338,8 +352,8 @@ export function InboxPage() {
// user can recover their input in the full editor instead of
// retyping. The agent picker hint becomes the assignee
// candidate (still editable).
const prompt = selected.details?.original_prompt ?? "";
const agentId = selected.details?.agent_id;
const prompt = detailSelected.details?.original_prompt ?? "";
const agentId = detailSelected.details?.agent_id;
useIssueDraftStore.getState().setDraft({
description: prompt,
...(agentId
@@ -355,7 +369,7 @@ export function InboxPage() {
<Button
variant="outline"
size="sm"
onClick={() => handleArchive(selected.id)}
onClick={() => handleArchive(detailSelected.id)}
>
<Archive className="mr-1.5 h-3.5 w-3.5" />
{t(($) => $.detail.archive)}
@@ -388,8 +402,10 @@ export function InboxPage() {
);
}
// Mobile: show detail full-screen when an item is selected
if (selected) {
// Mobile: show detail full-screen when an item is selected. Gate on the
// deferred selection so the screen swap and its content stay in lockstep
// (no blank frame between switching screens and the detail resolving).
if (detailSelected) {
return (
<div className="flex flex-1 flex-col min-h-0">
<div className="flex h-12 shrink-0 items-center border-b px-2">

View File

@@ -536,7 +536,12 @@ describe("IssueDetail (shared)", () => {
expect(screen.getByDisplayValue("Implement authentication")).toBeInTheDocument();
});
expect(screen.getByDisplayValue("Add JWT auth to the backend")).toBeInTheDocument();
// The description editor is deferred one animation frame past first paint
// (useDeferredMount) to keep its Tiptap mount off the synchronous
// issue-switch commit, so await it rather than asserting synchronously.
expect(
await screen.findByDisplayValue("Add JWT auth to the backend"),
).toBeInTheDocument();
});
it("opts the description editor into the unmount flush", async () => {
@@ -552,6 +557,18 @@ describe("IssueDetail (shared)", () => {
expect(description).toHaveAttribute("data-flush-on-unmount", "true");
});
it("hydrates the bottom comment composer after first paint (deferred, not synchronous)", async () => {
// The composer's Tiptap editor is mounted one animation frame past paint
// (useDeferredMount) so it stays out of the synchronous issue-switch
// commit. A static placeholder holds the spot meanwhile; await the real
// editor (its placeholder textarea) to confirm it still hydrates.
renderIssueDetail();
expect(
await screen.findByPlaceholderText("Leave a comment..."),
).toBeInTheDocument();
});
it("renders the issue title leaf as a link to the issue detail page", async () => {
renderIssueDetail();
@@ -1212,6 +1229,48 @@ describe("IssueDetail (shared)", () => {
).toContain("bg-[color-mix(in_srgb,var(--card)_95%,var(--brand)_5%)]");
});
});
it("keeps a short highlighted timeline on the flat (non-virtualized) path", async () => {
renderIssueDetailWithHighlight("comment-2");
await waitFor(() => {
expect(document.getElementById("comment-comment-2")).not.toBeNull();
});
// Short list → flat render, no Virtuoso. (Flat render lands precisely.)
expect(screen.queryByTestId("virtuoso-mock")).toBeNull();
});
it("virtualizes a long highlighted timeline instead of mounting every comment", async () => {
// Reproduces the inbox freeze: a comment-heavy issue opened with a
// highlight target used to flat-render every CommentCard synchronously.
// Past FLAT_TIMELINE_LIMIT it must hand off to Virtuoso (positioned on
// the target) so the page doesn't freeze on entry.
const longTimeline: TimelineEntry[] = Array.from({ length: 40 }, (_, i) => ({
type: "comment",
id: `c-${i}`,
actor_type: "member",
actor_id: "user-1",
content: `Comment ${i}`,
parent_id: null,
created_at: `2026-01-18T00:${String(i).padStart(2, "0")}:00Z`,
updated_at: `2026-01-18T00:${String(i).padStart(2, "0")}:00Z`,
comment_type: "comment",
})) as TimelineEntry[];
mockApiObj.listTimeline.mockResolvedValue(longTimeline);
renderIssueDetailWithHighlight("c-20");
// Virtuoso path is taken (the mock renders a virtuoso-mock wrapper), and
// the highlight tint still lands on the target row.
await waitFor(() => {
expect(screen.getByTestId("virtuoso-mock")).toBeInTheDocument();
});
await waitFor(() => {
expect(
document.getElementById("comment-c-20")?.querySelector(".ring-2"),
).not.toBeNull();
});
});
});
it("sends empty description when editor is cleared", async () => {

View File

@@ -30,7 +30,7 @@ import { Button } from "@multica/ui/components/ui/button";
import { ResizablePanelGroup, ResizablePanel, ResizableHandle } from "@multica/ui/components/ui/resizable";
import { Sheet, SheetContent } from "@multica/ui/components/ui/sheet";
import { useIsMobile } from "@multica/ui/hooks/use-mobile";
import { ContentEditor, type ContentEditorRef, TitleEditor, useFileDropZone, FileDropOverlay } from "../../editor";
import { ContentEditor, type ContentEditorRef, TitleEditor, ReadonlyContent, useFileDropZone, FileDropOverlay } from "../../editor";
import { FileUploadButton } from "@multica/ui/components/common/file-upload-button";
import {
Tooltip,
@@ -78,6 +78,7 @@ import { useRecentIssuesStore } from "@multica/core/issues/stores";
import { useIssueSelectionStore } from "@multica/core/issues/stores/selection-store";
import { BatchActionToolbar } from "./batch-action-toolbar";
import { useIssueTimeline } from "../hooks/use-issue-timeline";
import { useDeferredMount } from "../hooks/use-deferred-mount";
import { useIssueReactions } from "../hooks/use-issue-reactions";
import { useIssueSubscribers } from "../hooks/use-issue-subscribers";
import { ReactionBar } from "@multica/ui/components/common/reaction-bar";
@@ -433,6 +434,15 @@ function TimelineSkeleton() {
// activities" line that expands in place.
const LAST_ACTIVITY_BLOCK_VISIBLE_LIMIT = 8;
// Above this many timeline items, a deep-link/highlight target switches from
// flat render to a Virtuoso list positioned on the target. Flat render mounts
// every CommentCard (markdown + lowlight) synchronously, which is the
// multi-second freeze on entering/leaving a comment-heavy issue from the inbox
// (inbox notifications always carry a comment_id → highlight). Short timelines
// stay flat: the precise scroll/center landing is cheap and pixel-accurate
// there, and small lists never froze.
const FLAT_TIMELINE_LIMIT = 30;
// Collapsible wrapper for an activity block. Older blocks default to a single
// "N activities" summary line so the timeline isn't dominated by status /
// priority / assignee churn; the trailing block stays expanded because it
@@ -1061,6 +1071,12 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
return items.findIndex((it) => it.id === rootId);
}, [items, highlightCommentId, replyToRoot]);
// Render the highlighted timeline flat only while it is short enough that
// mounting every CommentCard up front is cheap. Past the threshold we hand
// the highlight off to Virtuoso (positioned on the target below) so we never
// synchronously mount a long comment list — the inbox freeze fix.
const flatTimeline = !!highlightCommentId && items.length <= FLAT_TIMELINE_LIMIT;
const {
reactions: issueReactions,
toggleReaction: handleToggleIssueReaction,
@@ -1172,50 +1188,57 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
}
}
const el = document.getElementById(`comment-${highlightCommentId}`);
const container = scrollContainerEl;
if (!el || !container) return;
didHighlightRef.current = highlightCommentId;
// Center the target comment WITHIN its own scroll container by driving the
// container's scrollTop directly — never native scrollIntoView. Native
// scrollIntoView is spec'd to scroll EVERY scrollable ancestor: on a cold
// mount where the timeline is still growing (streaming agent), the inner
// scroller can't satisfy centering on its own, so the scroll propagates up
// and moves the desktop shell's `overflow:hidden` wrapper — shoving the
// whole page, header included, off the top with no scrollbar to recover,
// until a resize reflows it (#3929). Scoping the scroll to `container`
// keeps it contained; re-centering across frames lands the comment
// precisely once async heights (markdown, code highlight, streamed replies)
// settle, instead of leaning on the ancestor scroll the way native did.
let rafId = 0;
let frames = 0;
let last = -1;
const center = () => {
const c = container.getBoundingClientRect();
const e = el.getBoundingClientRect();
const target = Math.max(
0,
container.scrollTop + (e.top - c.top) - (container.clientHeight - e.height) / 2,
);
container.scrollTop = target;
// Content is still laying out → the centered offset keeps shifting; keep
// re-centering until it stabilizes (within 1px) or we hit ~0.5s of frames.
if (Math.abs(target - last) > 1 && ++frames < 30) {
last = target;
rafId = requestAnimationFrame(center);
}
};
rafId = requestAnimationFrame(center);
if (flatTimeline) {
// Flat render: the target id is in the DOM. Center it WITHIN its own
// scroll container by driving the container's scrollTop directly — never
// native scrollIntoView. Native scrollIntoView is spec'd to scroll EVERY
// scrollable ancestor: on a cold mount where the timeline is still
// growing (streaming agent), the inner scroller can't satisfy centering
// on its own, so the scroll propagates up and moves the desktop shell's
// `overflow:hidden` wrapper — shoving the whole page, header included,
// off the top with no scrollbar to recover, until a resize reflows it
// (#3929). Scoping the scroll to `container` keeps it contained;
// re-centering across frames lands the comment precisely once async
// heights (markdown, code highlight, streamed replies) settle.
const el = document.getElementById(`comment-${highlightCommentId}`);
const container = scrollContainerEl;
if (!el || !container) return;
didHighlightRef.current = highlightCommentId;
let frames = 0;
let last = -1;
const center = () => {
const c = container.getBoundingClientRect();
const e = el.getBoundingClientRect();
const target = Math.max(
0,
container.scrollTop + (e.top - c.top) - (container.clientHeight - e.height) / 2,
);
container.scrollTop = target;
// Content is still laying out → the centered offset keeps shifting;
// keep re-centering until it stabilizes (within 1px) or ~0.5s of frames.
if (Math.abs(target - last) > 1 && ++frames < 30) {
last = target;
rafId = requestAnimationFrame(center);
}
};
rafId = requestAnimationFrame(center);
} else {
// Long timeline: Virtuoso positions itself on the target via
// `initialTopMostItemIndex` (it owns the shared scroller, so driving
// scrollTop here would fight it). We only apply the highlight tint.
didHighlightRef.current = highlightCommentId;
}
setHighlightedId(highlightCommentId);
const fade = window.setTimeout(() => setHighlightedId(null), 2500);
return () => {
cancelAnimationFrame(rafId);
if (rafId) cancelAnimationFrame(rafId);
clearTimeout(fade);
};
}, [highlightCommentId, items, targetIdx, scrollContainerEl, replyToRoot, expandedResolved, timelineView, toggleResolvedExpand]);
}, [highlightCommentId, items, targetIdx, scrollContainerEl, replyToRoot, expandedResolved, timelineView, toggleResolvedExpand, flatTimeline]);
// Cmd-F / Ctrl-F on a virtualized timeline only searches what's mounted in
// the viewport — off-screen comments are invisible to browser find-in-page.
@@ -1239,6 +1262,25 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
}, [id, items.length, t]);
const descEditorRef = useRef<ContentEditorRef>(null);
// Defer the two heavy Tiptap editors (description + bottom comment composer)
// past first paint so a fresh ProseMirror EditorView is never created inside
// the synchronous commit of an issue switch. Until they hydrate, the
// description shows as read-only markdown and the composer shows a static
// placeholder — both near-zero cost. The render-phase reset in
// useDeferredMount re-arms this on every issueId change, including the inbox
// pane (now reconciled in place, no remount) and the full-page route.
const { ready: descEditorReady, mountNow: mountDescEditor } = useDeferredMount(id);
const { ready: commentComposerReady } = useDeferredMount(id);
// Only steal focus into the editor when the user actively clicked the
// read-only placeholder — never on the automatic post-paint mount, which
// would yank focus to the description on every issue switch.
const descFocusOnMountRef = useRef(false);
useEffect(() => {
if (descEditorReady && descFocusOnMountRef.current) {
descFocusOnMountRef.current = false;
descEditorRef.current?.focus();
}
}, [descEditorReady]);
const { isDragOver: descDragOver, dropZoneProps: descDropZoneProps } = useFileDropZone({
onDrop: (files) => files.forEach((f) => descEditorRef.current?.uploadFile(f)),
});
@@ -1938,6 +1980,30 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
)}
<div {...descDropZoneProps} className="relative mt-5 rounded-lg">
{!descEditorReady ? (
// Read-only stand-in shown for the one frame before the editor
// hydrates (or until the user clicks in). Keeps the heavy Tiptap
// mount off the synchronous issue-switch commit; a mousedown
// upgrades it immediately so a quick click-to-edit still lands.
<div
className="min-h-[1.5rem] cursor-text"
onMouseDown={() => {
descFocusOnMountRef.current = true;
mountDescEditor();
}}
>
{issue.description?.trim() ? (
<ReadonlyContent
content={issue.description}
attachments={descEditorAttachments}
/>
) : (
<p className="text-sm text-muted-foreground">
{t(($) => $.detail.desc_placeholder)}
</p>
)}
</div>
) : (
<ContentEditor
ref={descEditorRef}
key={id}
@@ -1975,6 +2041,7 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
currentIssueId={id}
attachments={descEditorAttachments}
/>
)}
<div className="flex items-center gap-1 mt-3">
<ReactionBar
@@ -2163,46 +2230,21 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
{timelineLoading && timelineView.groups.length === 0 ? (
<TimelineSkeleton />
) : (
// Two render modes:
// - `highlightCommentId` set (came from inbox deep-link) →
// render flat. Every comment mounts, every height is real,
// the target id is in the DOM the instant the useEffect
// above runs `scrollIntoView`. No virtualization estimate
// errors, no spacer reflow drift. Pays cold-mount cost
// proportional to items.length (markdown + lowlight per
// comment), which is acceptable in the deep-link case —
// the user has explicit intent to land on a specific item.
// - otherwise → Virtuoso. Browsing mode, virtualization
// wins on first-paint perf for long timelines.
//
// The split is deliberate: virtualization and "land precisely
// on a target" have fundamentally opposed contracts (estimated
// heights vs real heights). Trying to satisfy both in one
// path is what produced the bug history this PR closes.
!highlightCommentId ? (
!scrollContainerEl ? (
// Skeleton while the callback ref populates so the gap
// between IssueDetail mount and Virtuoso mount doesn't
// flash empty.
<TimelineSkeleton />
) : (
<div className="mt-4">
<Virtuoso
key={`${wsId}:${id}`}
customScrollParent={scrollContainerEl}
data={items}
increaseViewportBy={{ top: 800, bottom: 800 }}
computeItemKey={(_i, item) => `${item.kind}:${item.id}`}
skipAnimationFrameInResizeObserver
// followOutput intentionally NOT set. Virtuoso treats
// it as a sticky "is at bottom" flag and resets
// scrollTop to maxScrollTop on every height-change
// tick — issue-detail is document-shaped, not chat.
itemContent={renderItem}
/>
</div>
)
) : (
// Three render modes:
// - SHORT highlighted timeline (`flatTimeline`) → render flat.
// Every comment mounts, every height is real, the target id
// is in the DOM the instant the deep-link effect above runs.
// No virtualization estimate errors, no spacer reflow drift.
// Cheap because the list is short.
// - LONG highlighted timeline → Virtuoso positioned on the
// target via `initialTopMostItemIndex`. Avoids synchronously
// mounting a long comment list (markdown + lowlight per
// CommentCard), which is the multi-second freeze on entering
// a comment-heavy issue from the inbox. Landing is best-effort
// (estimated heights) rather than pixel-perfect — the right
// trade for not freezing the page.
// - no highlight → Virtuoso browsing mode.
flatTimeline ? (
<div className="mt-4">
{items.map((item, i) => (
<Fragment key={`${item.kind}:${item.id}`}>
@@ -2210,17 +2252,53 @@ export function IssueDetail({ issueId, onDelete, onDone, defaultSidebarOpen = tr
</Fragment>
))}
</div>
) : !scrollContainerEl ? (
// Skeleton while the callback ref populates so the gap between
// IssueDetail mount and Virtuoso mount doesn't flash empty.
<TimelineSkeleton />
) : (
<div className="mt-4">
<Virtuoso
key={`${wsId}:${id}`}
customScrollParent={scrollContainerEl}
data={items}
{...(highlightCommentId && targetIdx >= 0
? { initialTopMostItemIndex: { index: targetIdx, align: "center" as const } }
: {})}
increaseViewportBy={{ top: 800, bottom: 800 }}
computeItemKey={(_i, item) => `${item.kind}:${item.id}`}
skipAnimationFrameInResizeObserver
// followOutput intentionally NOT set. Virtuoso treats
// it as a sticky "is at bottom" flag and resets
// scrollTop to maxScrollTop on every height-change
// tick — issue-detail is document-shaped, not chat.
itemContent={renderItem}
/>
</div>
)
)}
{/* Bottom comment input — no avatar, full width */}
<div className="mt-4">
{/* key={id}: web's /issues/[id] route doesn't remount on
issueId change, so without an explicit key the editor
keeps the previous issue's in-memory content and the
next keystroke would flush it into the new issue's
draft key. */}
<CommentInput key={id} issueId={id} onSubmit={submitComment} />
{/* Deferred one frame past paint (commentComposerReady) so the
composer's Tiptap editor isn't created inside the synchronous
issue-switch commit; a static placeholder of similar height
holds the spot meanwhile (it usually sits below the fold on
comment-heavy issues anyway). No manual expand control — it
hydrates on its own.
key={id}: web's /issues/[id] route doesn't remount on issueId
change, so without an explicit key the editor keeps the
previous issue's in-memory content and the next keystroke
would flush it into the new issue's draft key. */}
{commentComposerReady ? (
<CommentInput key={id} issueId={id} onSubmit={submitComment} />
) : (
<div className="relative flex flex-col rounded-lg bg-card pb-8 ring-1 ring-border">
<div className="px-3 py-2 text-sm text-muted-foreground">
{t(($) => $.comment.leave_comment_placeholder)}
</div>
</div>
)}
</div>
</div>
</div>

View File

@@ -0,0 +1,100 @@
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
import { act, render, renderHook } from "@testing-library/react";
import { useDeferredMount } from "./use-deferred-mount";
// Drive requestAnimationFrame deterministically: queue callbacks and flush
// them on demand so the test controls exactly when the deferred mount lands.
let rafCallbacks: FrameRequestCallback[] = [];
beforeEach(() => {
rafCallbacks = [];
vi.stubGlobal("requestAnimationFrame", (cb: FrameRequestCallback) => {
rafCallbacks.push(cb);
return rafCallbacks.length;
});
vi.stubGlobal("cancelAnimationFrame", (id: number) => {
rafCallbacks[id - 1] = () => {};
});
});
afterEach(() => {
vi.unstubAllGlobals();
});
function flushRaf() {
const pending = rafCallbacks;
rafCallbacks = [];
act(() => {
pending.forEach((cb) => cb(0));
});
}
describe("useDeferredMount", () => {
it("starts not-ready and flips ready after the next animation frame", () => {
const { result } = renderHook(() => useDeferredMount());
expect(result.current.ready).toBe(false);
flushRaf();
expect(result.current.ready).toBe(true);
});
it("mountNow forces an immediate ready without waiting for the frame", () => {
const { result } = renderHook(() => useDeferredMount());
expect(result.current.ready).toBe(false);
act(() => {
result.current.mountNow();
});
expect(result.current.ready).toBe(true);
});
it("re-arms the deferral when resetKey changes", () => {
const { result, rerender } = renderHook(
({ key }: { key: string }) => useDeferredMount(key),
{ initialProps: { key: "a" } },
);
flushRaf();
expect(result.current.ready).toBe(true);
rerender({ key: "b" });
expect(result.current.ready).toBe(false);
flushRaf();
expect(result.current.ready).toBe(true);
});
it("never reports ready for the first render after a key change (no sync mount-then-unmount)", () => {
// Captures the `ready` value on EVERY render, including the synchronous
// render(s) React runs while a key change is reconciled — the window
// renderHook's settled `result.current` hides. An effect-based reset would
// leave this first post-change render ready=true (heavy child mounts, then
// unmounts a tick later); the render-phase reset must keep it false.
const readyLog: boolean[] = [];
function Probe({ k }: { k: string }) {
const { ready } = useDeferredMount(k);
readyLog.push(ready);
return <span>{ready ? "ready" : "pending"}</span>;
}
const { rerender } = render(<Probe k="a" />);
flushRaf();
// Now settled ready=true for key "a".
readyLog.length = 0;
act(() => {
rerender(<Probe k="b" />);
});
// Every render between the key change and the next frame must be pending.
expect(readyLog.length).toBeGreaterThan(0);
expect(readyLog.some((r) => r === true)).toBe(false);
flushRaf();
expect(readyLog.at(-1)).toBe(true);
});
});

View File

@@ -0,0 +1,56 @@
"use client";
import { useCallback, useEffect, useState } from "react";
/**
* Defers mounting a heavy child until just after the first paint of the
* surrounding tree. Returns `ready: false` on the initial commit (and on the
* first commit after `resetKey` changes) and flips to `true` on the next
* animation frame.
*
* The inbox detail pane remounts the whole `IssueDetail` subtree on every issue
* switch (it is keyed by `issue_id`). Creating the description's Tiptap editor
* inside that synchronous commit is one of the costliest pieces of work on the
* critical switch path — a fresh ProseMirror `EditorView` with ~20 extensions.
* Gating it on this hook keeps the editor out of the switch commit, so the new
* issue's content paints first and the editor hydrates a frame later. UX is
* unchanged; only the timing moves.
*
* `resetKey` re-arms the deferral when it changes. The reset is derived during
* render (the tracked key lives in state and is reconciled in the render body),
* NOT in an effect — so the very first render after a key change already
* reports `ready: false`. That matters on routes where the host component is
* NOT remounted on key change (e.g. the full-page issue route): an
* effect-based reset would let the heavy child mount synchronously on that
* first render and then immediately unmount, which is the opposite of the goal.
*
* `mountNow` forces an immediate mount, for when the user interacts with the
* placeholder before the deferred frame lands.
*/
export function useDeferredMount(resetKey?: unknown): {
ready: boolean;
mountNow: () => void;
} {
const [ready, setReady] = useState(false);
const [trackedKey, setTrackedKey] = useState(resetKey);
// Reconcile during render (React's "adjust state when a prop changes"
// pattern): on a key change, reset the tracked state for subsequent renders.
const keyChanged = trackedKey !== resetKey;
if (keyChanged) {
setTrackedKey(resetKey);
setReady(false);
}
useEffect(() => {
const raf = requestAnimationFrame(() => setReady(true));
return () => cancelAnimationFrame(raf);
}, [resetKey]);
const mountNow = useCallback(() => setReady(true), []);
// On the render where the key just changed, `ready` still holds the previous
// key's value. Derive the returned value from the comparison so even that
// pass reports false — the heavy child is never mountable for a stale key.
return { ready: keyChanged ? false : ready, mountNow };
}