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: 13 additions & 11 deletions desktop/src/features/messages/lib/rowHeightEstimate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -165,18 +165,20 @@ const SYSTEM_GROUP_HEIGHT = 80;
* near its true height instead of snapping the scroll position. `auto` keeps
* refining once the row paints.
*/
export function estimateTimelineItemHeight(item: TimelineItem): number {
return item.kind === "message"
? estimateRowHeight(item.entry.message, {
isContinuation: item.isContinuation,
}) + (item.isFollowedByContinuation ? 0 : MESSAGE_ITEM_BOTTOM_PADDING)
: item.kind === "system"
? estimateRowHeight(item.entry.message)
: item.kind === "system-group"
? SYSTEM_GROUP_HEIGHT
: DIVIDER_HEIGHT;
}

export function timelineRowReserveStyle(
item: TimelineItem,
): React.CSSProperties {
const height =
item.kind === "message"
? estimateRowHeight(item.entry.message, {
isContinuation: item.isContinuation,
}) + (item.isFollowedByContinuation ? 0 : MESSAGE_ITEM_BOTTOM_PADDING)
: item.kind === "system"
? estimateRowHeight(item.entry.message)
: item.kind === "system-group"
? SYSTEM_GROUP_HEIGHT
: DIVIDER_HEIGHT;
return { containIntrinsicSize: `auto ${height}px` };
return { containIntrinsicSize: `auto ${estimateTimelineItemHeight(item)}px` };
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import test from "node:test";
import {
buildVirtualizedItems,
didPrependVirtualizedTimeline,
estimateVirtualizedTimelineItemHeight,
virtualizedItemKey,
} from "./virtualizedTimelineItems.ts";

Expand Down Expand Up @@ -204,3 +205,23 @@ test("naive counterexample stays rejected: same key cannot precede prepended row
false,
);
});

test("virtualized rows preserve their heterogeneous height estimates", () => {
const short = messageItem("short");
short.entry.message.body = "hello";
const tall = messageItem("tall");
tall.entry.message.body = Array.from(
{ length: 20 },
(_, index) => `line ${index}`,
).join("\n");
const items = buildVirtualizedItems(
[{ key: "day-A", headingTimestamp: DAY_A, items: [short, tall] }],
undefined,
true,
);
const estimates = items.map(estimateVirtualizedTimelineItemHeight);

assert.equal(estimates[0], 32);
assert.ok(estimates[2] > estimates[1] + 200);
assert.equal(estimates.at(-1), 96);
});
10 changes: 10 additions & 0 deletions desktop/src/features/messages/lib/virtualizedTimelineItems.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import type * as React from "react";

import { estimateTimelineItemHeight } from "./rowHeightEstimate";
import {
getTimelineItemKey,
type TimelineDayGroup,
Expand All @@ -26,6 +27,15 @@ export type VirtualizedTimelineItem =
item: TimelineNonDayItem;
};

export function estimateVirtualizedTimelineItemHeight(
item: VirtualizedTimelineItem,
): number {
if (item.kind === "bottom-spacer") return 96;
if (item.kind === "leading-content") return 60;
if (item.kind === "day-divider") return 32;
return estimateTimelineItemHeight(item.item);
}

export function virtualizedItemKey(item: VirtualizedTimelineItem): string {
if (item.kind === "bottom-spacer") return "bottom-spacer";
if (item.kind === "leading-content") return "leading-content";
Expand Down
154 changes: 102 additions & 52 deletions desktop/src/features/messages/ui/TimelineMessageList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import {
import {
buildVirtualizedItems,
didPrependVirtualizedTimeline,
estimateVirtualizedTimelineItemHeight,
type VirtualizedTimelineItem,
virtualizedItemKey,
} from "@/features/messages/lib/virtualizedTimelineItems";
import { THREAD_REPLY_ROW_MARGIN_INLINE_REM } from "@/features/messages/lib/threadTreeLayout";
Expand Down Expand Up @@ -370,6 +372,33 @@ type VirtualizedTimelineRowsProps = {
renderItem: (item: TimelineNonDayItem) => React.ReactNode;
};

type VirtualizedTimelineItemShellProps = {
children: React.ReactNode;
index: number;
ref?: React.LegacyRef<HTMLDivElement>;
style: React.CSSProperties;
};

const PreserveVirtualizedItemVisibilityContext = React.createContext(false);

function VirtualizedTimelineItemShell({
children,
ref,
style,
}: VirtualizedTimelineItemShellProps) {
const preserveVisibility = React.useContext(
PreserveVirtualizedItemVisibilityContext,
);
return (
<div
ref={ref}
style={preserveVisibility ? style : { ...style, visibility: undefined }}
>
{children}
</div>
);
}

function VirtualizedTimelineRows({
dayGroups,
historyExhausted,
Expand All @@ -391,6 +420,20 @@ function VirtualizedTimelineRows({
typeof window === "undefined" ? 1_000 : window.innerHeight,
);
const hasInitialPositionedRef = React.useRef(false);
const estimateCallCountRef = React.useRef(0);
const estimateItemSize = React.useCallback(
(item: VirtualizedTimelineItem) => {
estimateCallCountRef.current += 1;
const scroller = hostRef.current?.firstElementChild;
if (scroller instanceof HTMLDivElement) {
scroller.dataset.virtuaEstimateCallCount = String(
estimateCallCountRef.current,
);
}
return estimateVirtualizedTimelineItemHeight(item);
},
[],
);
const items = React.useMemo(
() => buildVirtualizedItems(dayGroups, leadingContent, historyExhausted),
[dayGroups, historyExhausted, leadingContent],
Expand Down Expand Up @@ -555,6 +598,9 @@ function VirtualizedTimelineRows({
if (element) {
element.dataset.buzzConversationScroll = "true";
element.dataset.testid = "message-timeline";
element.dataset.virtuaEstimateCallCount = String(
estimateCallCountRef.current,
);
}
onVirtualizerScrollerChange?.(element);
return () => onVirtualizerScrollerChange?.(null);
Expand Down Expand Up @@ -637,62 +683,66 @@ function VirtualizedTimelineRows({

return (
<div className="h-full min-h-0 w-full" ref={hostRef}>
<VList
ref={listRef}
className="h-full min-h-0 w-full overflow-y-auto overflow-x-hidden overscroll-contain px-2 pt-[var(--channel-top-chrome-height,4.5rem)]"
data={items}
bufferSize={offscreenBufferSize}
keepMounted={retainedIndices}
style={{ overflowAnchor: "none" }}
shift={isPrepend}
onScroll={handleScroll}
onScrollEnd={handleScrollEnd}
>
{(item) => {
if (item.kind === "bottom-spacer") {
return (
<div
aria-hidden
className="h-[var(--composer-overlay-height,6rem)]"
key={virtualizedItemKey(item)}
/>
);
}
if (item.kind === "leading-content") {
return <div key={virtualizedItemKey(item)}>{item.content}</div>;
}
if (item.kind === "day-divider") {
const dayLabel = formatDayHeading(item.headingTimestamp);
<PreserveVirtualizedItemVisibilityContext value={isPrepend}>
<VList
ref={listRef}
className="h-full min-h-0 w-full overflow-y-auto overflow-x-hidden overscroll-contain px-2 pt-[var(--channel-top-chrome-height,4.5rem)]"
data={items}
item={VirtualizedTimelineItemShell}
itemSize={estimateItemSize}
bufferSize={offscreenBufferSize}
keepMounted={retainedIndices}
style={{ overflowAnchor: "none" }}
shift={isPrepend}
onScroll={handleScroll}
onScrollEnd={handleScrollEnd}
>
{(item) => {
if (item.kind === "bottom-spacer") {
return (
<div
aria-hidden
className="h-[var(--composer-overlay-height,6rem)]"
key={virtualizedItemKey(item)}
/>
);
}
if (item.kind === "leading-content") {
return <div key={virtualizedItemKey(item)}>{item.content}</div>;
}
if (item.kind === "day-divider") {
const dayLabel = formatDayHeading(item.headingTimestamp);
return (
<div
// The sticky pill needs travel room, but its containing block
// is this item wrapper. The trailing spacer extends the content
// box by 4rem while the matching negative margin keeps the
// measured layout height at exactly the divider's height, so
// row spacing and Virtua's size cache are unaffected. Both the
// spacer and the pill are pointer-events-none, and the later
// (absolutely positioned) row siblings paint above the spacer.
className="relative -mb-16 flex flex-col before:absolute before:inset-x-0 before:top-4 before:h-px before:bg-border/35 before:content-['']"
data-day-label={dayLabel}
data-testid="message-timeline-day-group"
key={virtualizedItemKey(item)}
>
<DayDivider label={dayLabel} />
<div aria-hidden className="pointer-events-none h-16" />
</div>
);
}
return (
<div
// The sticky pill needs travel room, but its containing block
// is this item wrapper. The trailing spacer extends the content
// box by 4rem while the matching negative margin keeps the
// measured layout height at exactly the divider's height, so
// row spacing and Virtua's size cache are unaffected. Both the
// spacer and the pill are pointer-events-none, and the later
// (absolutely positioned) row siblings paint above the spacer.
className="relative -mb-16 flex flex-col before:absolute before:inset-x-0 before:top-4 before:h-px before:bg-border/35 before:content-['']"
data-day-label={dayLabel}
data-testid="message-timeline-day-group"
<TimelineRowShell
item={item.item}
key={virtualizedItemKey(item)}
useContentVisibility={false}
>
<DayDivider label={dayLabel} />
<div aria-hidden className="pointer-events-none h-16" />
</div>
{renderItem(item.item)}
</TimelineRowShell>
);
}
return (
<TimelineRowShell
item={item.item}
key={virtualizedItemKey(item)}
useContentVisibility={false}
>
{renderItem(item.item)}
</TimelineRowShell>
);
}}
</VList>
}}
</VList>
</PreserveVirtualizedItemVisibilityContext>
</div>
);
}
Expand Down
75 changes: 75 additions & 0 deletions desktop/tests/e2e/timeline-no-shift.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,56 @@ function expectAnchorOrderUnchanged(
expect(after.rowsFromAnchor).toEqual(before.rowsFromAnchor);
}

test("timeline does not recompute row estimates during ordinary scroll", async ({
page,
}) => {
await installMockBridge(page);
await page.goto("/");
await waitForMockTimelineBridge(page);
await page.evaluate(() => {
for (let index = 0; index < 120; index += 1) {
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
channelName: "general",
content: `estimate memo row ${index}\nsecond line ${index}`,
createdAt: 1_700_500_000 + index,
});
}
});

await page.getByTestId("channel-general").click();
await expect(page.getByTestId("chat-title")).toHaveText("general");
const timeline = page.getByTestId("message-timeline");
await expect(timeline).toContainText("estimate memo row 119");
await page.waitForFunction(() => {
const element = document.querySelector<HTMLDivElement>(
'[data-testid="message-timeline"]',
);
return element && element.scrollHeight > element.clientHeight * 3;
});

const estimateCallsBefore = await timeline.evaluate((element) =>
Number((element as HTMLDivElement).dataset.virtuaEstimateCallCount ?? "0"),
);
expect(estimateCallsBefore).toBeGreaterThan(0);

await timeline.evaluate(async (element) => {
const scroller = element as HTMLDivElement;
const maxOffset = scroller.scrollHeight - scroller.clientHeight;
for (const fraction of [0.75, 0.5, 0.25, 0.6, 0.4]) {
scroller.scrollTop = maxOffset * fraction;
scroller.dispatchEvent(new Event("scroll", { bubbles: true }));
await new Promise<void>((resolve) =>
requestAnimationFrame(() => resolve()),
);
}
});

const estimateCallsAfter = await timeline.evaluate((element) =>
Number((element as HTMLDivElement).dataset.virtuaEstimateCallCount ?? "0"),
);
expect(estimateCallsAfter).toBe(estimateCallsBefore);
});

test("timeline reserves mixed-media rows before fast scrollback", async ({
page,
}, testInfo) => {
Expand Down Expand Up @@ -450,6 +500,15 @@ test("timeline prepend plus late row reflow keeps the reading row stable", async
const before = await snapshotAnchor(timeline);
expect(before.anchorId).not.toBe("");
expect(before.oldestOlderIndex).not.toBeNull();
await timeline.evaluate((element, anchorId) => {
const anchor = element.querySelector<HTMLElement>(
`[data-message-id="${CSS.escape(anchorId)}"]`,
);
if (!anchor) throw new Error("prepend mount-identity anchor missing");
(
window as typeof window & { __PREPEND_MOUNT_IDENTITY__?: HTMLElement }
).__PREPEND_MOUNT_IDENTITY__ = anchor;
}, before.anchorId);
await startAnchorDriftSampler(timeline, before.anchorId, before.anchorTop);

await expect
Expand All @@ -469,6 +528,22 @@ test("timeline prepend plus late row reflow keeps the reading row stable", async

const afterPrepend = await snapshotAnchor(timeline);
expect(afterPrepend.anchorId).toBe(before.anchorId);
expect(
await timeline.evaluate((element, anchorId) => {
const anchor = element.querySelector<HTMLElement>(
`[data-message-id="${CSS.escape(anchorId)}"]`,
);
return (
anchor ===
(
window as typeof window & {
__PREPEND_MOUNT_IDENTITY__?: HTMLElement;
}
).__PREPEND_MOUNT_IDENTITY__
);
}, before.anchorId),
"prepend must preserve the mounted anchor DOM node",
).toBe(true);
expect(
Math.abs(afterPrepend.anchorTop - before.anchorTop),
// First-pass prepended rows realize from content-visibility estimates to
Expand Down
Loading