Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion apps/app/src/components/thread/timeline/ThreadTimelineRows.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,8 @@ export interface ThreadTimelineRowsProps {
isLoadingOlderTimelineRows?: boolean;
onLoadOlderRows?: () => Promise<void> | void;
timelineRows: TimelineRow[];
/** Outline destination kept mounted while timeline windowing is enabled. */
timelineNavigationTargetRowId?: string | null;
threadId?: string;
threadRuntimeDisplayStatus: ThreadRuntimeDisplayStatus;
/** Omit for standalone initial-unread rendering, pass false for live updates. */
Expand Down Expand Up @@ -280,6 +282,7 @@ interface TimelineRowsListProps {
compactActivityIntents: boolean;
hasOlderTimelineRows?: boolean;
isLoadingOlderTimelineRows?: boolean;
navigationTargetRowId?: string | null;
onLoadOlderRows?: () => Promise<void> | void;
rows: readonly ThreadTimelineViewRow[];
scopeActive: boolean;
Expand Down Expand Up @@ -2055,6 +2058,7 @@ function TimelineRowsList({
compactActivityIntents,
hasOlderTimelineRows,
isLoadingOlderTimelineRows,
navigationTargetRowId,
onLoadOlderRows,
rows,
scopeActive,
Expand Down Expand Up @@ -2114,8 +2118,23 @@ function TimelineRowsList({
if (spacing === "top-level" && scrollRestoreRowId !== null) {
keys.add(scrollRestoreRowId);
}
if (
spacing === "top-level" &&
timelineWindowingEnabled &&
navigationTargetRowId != null
) {
keys.add(navigationTargetRowId);
}
return keys;
}, [items, rows, scrollRestoreRowId, spacing, stableSearchExpandedRowIds]);
}, [
items,
rows,
scrollRestoreRowId,
spacing,
stableSearchExpandedRowIds,
navigationTargetRowId,
timelineWindowingEnabled,
]);
const getWindowingScrollElement =
detailScrollRoot?.getScrollElement ??
bottomAnchor?.getScrollElement ??
Expand Down Expand Up @@ -2486,6 +2505,9 @@ function ThreadTimelineRowsForTimelineView(props: ThreadTimelineRowsProps) {
isLoadingOlderTimelineRows={
props.isLoadingOlderTimelineRows
}
navigationTargetRowId={
props.timelineNavigationTargetRowId
}
onLoadOlderRows={props.onLoadOlderRows}
rows={rows}
scopeActive={scopeActive}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ describe("ThreadTimelineRows windowing experiment", () => {
});
});

it("keeps an offscreen search target realized and reveals it", async () => {
it("keeps offscreen search and outline targets realized", async () => {
const scrollElement = document.createElement("div");
scrollElement.setAttribute("data-test-main-scroll", "");
Object.defineProperty(scrollElement, "clientHeight", { value: 800 });
Expand Down Expand Up @@ -214,6 +214,7 @@ describe("ThreadTimelineRows windowing experiment", () => {
<ThreadTimelineRows
threadId="thr_large_search"
timelineRows={rows}
timelineNavigationTargetRowId="search-message-40"
timelineWindowingEnabled
threadRuntimeDisplayStatus="idle"
workspaceRootPath={undefined}
Expand All @@ -229,6 +230,11 @@ describe("ThreadTimelineRows windowing experiment", () => {

expect(target?.dataset.timelineWindowedRealized).toBe("true");
expect(target?.textContent).toContain("Search message 20");
const outlineTarget = view.container.querySelector<HTMLElement>(
'[data-timeline-row-id="search-message-40"]',
);
expect(outlineTarget?.dataset.timelineWindowedRealized).toBe("true");
expect(outlineTarget?.textContent).toContain("Search message 40");
await waitFor(() =>
expect(scrollElementIntoView).toHaveBeenCalledWith({
element: target,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ export interface ThreadTimelineSurfaceProps {
stoppingAnchorAt?: number;
timelineErrorClassName?: string;
timelineRows: TimelineRow[];
/** Outline destination kept mounted while timeline windowing is enabled. */
timelineNavigationTargetRowId?: string | null;
threadId: string;
threadRuntimeDisplayStatus: ThreadRuntimeDisplayStatus;
unreadDividerAutoScroll?: boolean;
Expand Down Expand Up @@ -170,6 +172,7 @@ export function ThreadTimelineSurface({
stoppingAnchorAt = 0,
timelineErrorClassName = "mt-6 text-destructive",
timelineRows,
timelineNavigationTargetRowId,
threadId,
threadRuntimeDisplayStatus,
unreadDividerAutoScroll,
Expand Down Expand Up @@ -242,6 +245,7 @@ export function ThreadTimelineSurface({
isLoadingOlderTimelineRows={isLoadingOlderTimelineRows}
onLoadOlderRows={onLoadOlderRows}
timelineRows={timelineRowsWithPendingStop}
timelineNavigationTargetRowId={timelineNavigationTargetRowId}
timelineWindowingEnabled={timelineWindowingEnabled}
threadId={threadId}
threadRuntimeDisplayStatus={threadRuntimeDisplayStatus}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
TimelineWindowedItems,
type TimelineWindowedItemRenderState,
} from "./TimelineWindowedItems.js";
import { TimelineWindowedItemsLoader } from "./TimelineWindowedItemsLoader.js";

const ITEM_KEYS = Array.from({ length: 100 }, (_, index) => `row-${index}`);

Expand Down Expand Up @@ -134,6 +135,32 @@ afterEach(() => {
});

describe("TimelineWindowedItems", () => {
it("seeds exact heights while the lazy windowing implementation loads", () => {
const measurements = new Map<string, number>();

render(
<TimelineWindowedItemsLoader
enabled
estimateItemHeight={() => 100}
gap={0}
getScrollElement={() => scrollElement}
itemKeys={ITEM_KEYS}
measurements={measurements}
renderItem={(index, state) => (
<div
key={ITEM_KEYS[index]}
ref={state.itemRef}
data-index={state.itemIndex}
/>
)}
/>,
{ container: scrollElement },
);

expect(measurements.get("row-0")).toBe(32);
expect(measurements.get("row-99")).toBe(32);
});

it("keeps the control path fully mounted when the experiment is off", () => {
renderWindowedItems({ enabled: false });

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
} from "react";

const DEFAULT_WINDOWING_MIN_ITEM_COUNT = 20;
const MAX_CONTROL_PATH_MEASUREMENTS = 2_000;
const NOOP_ITEM_REF = () => {};

export interface TimelineWindowingScrollRoot {
Expand Down Expand Up @@ -53,13 +54,28 @@ const LazyTimelineWindowedItems = lazy(async () => {

function TimelineWindowedItemsControl({
itemKeys,
measurements,
renderItem,
}: TimelineWindowedItemsProps) {
return itemKeys.map((_key, index) =>
captureMeasurements = false,
}: TimelineWindowedItemsProps & { captureMeasurements?: boolean }) {
return itemKeys.map((key, index) =>
renderItem(index, {
isRealized: true,
itemIndex: undefined,
itemRef: NOOP_ITEM_REF,
itemIndex: captureMeasurements ? index : undefined,
itemRef: captureMeasurements
? (element) => {
if (element === null) return;
const height = element.getBoundingClientRect().height;
if (height <= 0) return;
measurements.delete(key);
measurements.set(key, height);
while (measurements.size > MAX_CONTROL_PATH_MEASUREMENTS) {
const oldestKey = measurements.keys().next().value;
if (oldestKey === undefined) break;
measurements.delete(oldestKey);
}
}
: NOOP_ITEM_REF,
itemStyle: undefined,
windowingEnabled: false,
}),
Expand All @@ -75,7 +91,9 @@ export function TimelineWindowedItemsLoader(props: TimelineWindowedItemsProps) {
(props.minItemCount ?? DEFAULT_WINDOWING_MIN_ITEM_COUNT);
if (!configured) return <TimelineWindowedItemsControl {...props} />;
return (
<Suspense fallback={<TimelineWindowedItemsControl {...props} />}>
<Suspense
fallback={<TimelineWindowedItemsControl {...props} captureMeasurements />}
>
<LazyTimelineWindowedItems {...props} />
</Suspense>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ function TocHost({
hostPaddingX = 0,
hostWidth = 1_200,
loadOlderTimelineRows = () => {},
onNavigateToRow,
threadId = "thr_toc_test",
timelineRows,
}: {
Expand All @@ -113,6 +114,7 @@ function TocHost({
hostPaddingX?: number;
hostWidth?: number;
loadOlderTimelineRows?: () => void | Promise<void>;
onNavigateToRow?: (rowId: string) => void;
threadId?: string;
timelineRows: readonly TimelineRow[];
}) {
Expand All @@ -136,6 +138,7 @@ function TocHost({
timelineRows={timelineRows}
hasOlderTimelineRows={hasOlderTimelineRows}
loadOlderTimelineRows={loadOlderTimelineRows}
onNavigateToRow={onNavigateToRow}
/>
</div>
);
Expand Down Expand Up @@ -834,6 +837,7 @@ describe("ThreadTableOfContents", () => {
it("scrolls straight to a message already loaded in the window", async () => {
scrollElement.appendChild(timelineRowElement("u2"));
const loadOlder = vi.fn();
const onNavigateToRow = vi.fn();
setOutline([
{
id: "u1",
Expand All @@ -860,12 +864,14 @@ describe("ThreadTableOfContents", () => {
timelineRows={[]}
hasOlderTimelineRows
loadOlderTimelineRows={loadOlder}
onNavigateToRow={onNavigateToRow}
/>,
);
openTocPanel();
fireEvent.click(await screen.findByText("Loaded question"));

await waitFor(() => expect(scrollElementIntoView).toHaveBeenCalledTimes(1));
expect(onNavigateToRow).toHaveBeenCalledWith("u2");
expect(loadOlder).not.toHaveBeenCalled();
});

Expand Down
6 changes: 5 additions & 1 deletion apps/app/src/components/thread/toc/ThreadTableOfContents.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ interface ThreadTableOfContentsProps {
hasOlderTimelineRows: boolean;
/** Loads the next older timeline page; awaited while jumping to an unloaded row. */
loadOlderTimelineRows: () => void | Promise<void>;
/** Lets timeline windowing mount an offscreen destination before scrolling. */
onNavigateToRow?: (rowId: string) => void;
}

// Matches `@container scroll-overlay (min-width: 56rem)` in app.css.
Expand Down Expand Up @@ -532,6 +534,7 @@ export function ThreadTableOfContents({
timelineRows,
hasOlderTimelineRows,
loadOlderTimelineRows,
onNavigateToRow,
}: ThreadTableOfContentsProps) {
const bottomAnchor = useBottomAnchoredScroll();
const [rootElement, setRootElement] = useState<HTMLElement | null>(null);
Expand Down Expand Up @@ -668,6 +671,7 @@ export function ThreadTableOfContents({
options: { block: "start", inline: "nearest" },
});
};
onNavigateToRow?.(id);

let row = findTimelineRowElement(getScrollElement(), id);
if (row) {
Expand Down Expand Up @@ -714,7 +718,7 @@ export function ThreadTableOfContents({
setPendingJumpId(null);
}
},
[bottomAnchor],
[bottomAnchor, onNavigateToRow],
);

if (userItems.length < TOC_MIN_USER_MESSAGES) {
Expand Down
30 changes: 25 additions & 5 deletions apps/app/src/views/thread-detail/ThreadTimelinePane.test.tsx
Original file line number Diff line number Diff line change
@@ -1,26 +1,39 @@
// @vitest-environment jsdom

import { cleanup, render, screen } from "@testing-library/react";
import { cleanup, fireEvent, render, screen } from "@testing-library/react";
import { afterEach, expect, it, vi } from "vitest";
import type { ThreadTimelineSurfaceProps } from "@/components/thread/timeline/ThreadTimelineSurface";

vi.mock("@/components/thread/timeline/ThreadTimelineSurface", () => ({
ThreadTimelineSurface: (props: ThreadTimelineSurfaceProps) => (
<div data-testid="timeline">
{props.onOpenPluginPanel === undefined ? "missing" : "available"}
<span data-testid="plugin-panel-opener">
{props.onOpenPluginPanel === undefined ? "missing" : "available"}
</span>
<span data-testid="navigation-target">
{props.timelineNavigationTargetRowId ?? "none"}
</span>
</div>
),
}));

vi.mock("@/components/thread/toc/ThreadTableOfContents", () => ({
ThreadTableOfContents: () => null,
ThreadTableOfContents: ({
onNavigateToRow,
}: {
onNavigateToRow?: (rowId: string) => void;
}) => (
<button type="button" onClick={() => onNavigateToRow?.("row-target")}>
Jump to row
</button>
),
}));

const { ThreadTimelinePane } = await import("./ThreadTimelinePane");

afterEach(cleanup);

it("forwards the plugin-panel opener to rendered message directives", () => {
it("forwards pane callbacks to the timeline and conversation outline", () => {
render(
<ThreadTimelinePane
activeThinking={null}
Expand All @@ -46,5 +59,12 @@ it("forwards the plugin-panel opener to rendered message directives", () => {
/>,
);

expect(screen.getByTestId("timeline").textContent).toBe("available");
expect(screen.getByTestId("plugin-panel-opener").textContent).toBe(
"available",
);
expect(screen.getByTestId("navigation-target").textContent).toBe("none");
fireEvent.click(screen.getByRole("button", { name: "Jump to row" }));
expect(screen.getByTestId("navigation-target").textContent).toBe(
"row-target",
);
});
7 changes: 5 additions & 2 deletions apps/app/src/views/thread-detail/ThreadTimelinePane.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ReactNode } from "react";
import { useState, type ReactNode } from "react";
import type { ThreadTimelineUnreadDividerPlacement } from "@/components/thread/timeline";
import type { PromptMentionLinkResolver } from "@/components/promptbox/editor/prompt-mention-link";
import { EmbeddedThreadChat } from "@/components/thread/embedded-chat";
Expand All @@ -22,6 +22,8 @@ export function ThreadTimelinePane({
footer,
...surface
}: ThreadTimelinePaneProps) {
const [timelineNavigationTargetRowId, setTimelineNavigationTargetRowId] =
useState<string | null>(null);
return (
<EmbeddedThreadChat
variant="hosted-footer"
Expand All @@ -32,9 +34,10 @@ export function ThreadTimelinePane({
timelineRows={surface.timelineRows}
hasOlderTimelineRows={surface.hasOlderTimelineRows}
loadOlderTimelineRows={surface.onLoadOlderRows}
onNavigateToRow={setTimelineNavigationTargetRowId}
/>
}
surface={surface}
surface={{ ...surface, timelineNavigationTargetRowId }}
/>
);
}
Loading