mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(timeline): hold first-load viewport at the bottom while the document settles
Root cause: the init pin was one-shot. On first load the virtualizer paints
with ESTIMATED row heights and the deferred snapshot streams in over several
commits, so the true bottom keeps moving AFTER the single init scrollToBottom.
Nothing re-pinned: the new-message branch only fires on a new latest-message
key (the bottom message is already present during initial fill), so the viewport
ended up anchored where the estimated bottom was and the rest filled in below as
heights resolved — exactly the 'doesn't stay attached to the bottom' tho saw.
Fix: a settle-pin layout effect re-anchors to the bottom whenever the
virtualizer's getTotalSize() changes (estimate→measured heights, streaming rows,
container resize) — but ONLY while still pinned (stickToBottomRef) and not
chasing a deep-link, so a user who scrolled up is never yanked back down. Guarded
on a real size change (lastPinnedTotalSizeRef) so the pin's own scroll-induced
re-render can't loop; scrollToBottom records the pinned size to avoid a
redundant double-fire.
Distinct from the mid-scroll prepend retention (a8b21597) and layered cleanly on
the scrollMargin work + existing sticky-bottom autoscroll — no regression to
either. Main timeline only; thread pane untouched. Two DOM tests lock in: re-pin
on size growth while pinned, and NO re-pin after the user scrolls away.
Co-authored-by: Taylor Ho <taylorkmho@gmail.com>
Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
co-authored by
Taylor Ho
parent
a8b2159722
commit
6f6d2320ed
@@ -28,18 +28,23 @@ function message(
|
||||
}
|
||||
|
||||
// Records scrollToIndex calls so we can assert the virtualizer is driven
|
||||
// correctly. The hook never reads layout off the virtualizer directly.
|
||||
function makeVirtualizer() {
|
||||
// correctly. `getTotalSize` is controllable so we can simulate the document
|
||||
// growing during first-load settle (estimate→measured heights, streaming rows).
|
||||
function makeVirtualizer(initialTotalSize = 100) {
|
||||
let totalSize = initialTotalSize;
|
||||
const scrollToIndex =
|
||||
mock.fn<
|
||||
(index: number, opts?: { align?: string; behavior?: string }) => void
|
||||
>();
|
||||
return {
|
||||
scrollToIndex,
|
||||
virtualizer: { scrollToIndex } as unknown as Virtualizer<
|
||||
HTMLDivElement,
|
||||
Element
|
||||
>,
|
||||
setTotalSize(next: number) {
|
||||
totalSize = next;
|
||||
},
|
||||
virtualizer: {
|
||||
scrollToIndex,
|
||||
getTotalSize: () => totalSize,
|
||||
} as unknown as Virtualizer<HTMLDivElement, Element>,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -141,3 +146,96 @@ test("a deep-link target scrolls to that message's flat row and centers it", ()
|
||||
assert.equal(onTargetReached.mock.calls.length, 1);
|
||||
assert.equal(onTargetReached.mock.calls[0].arguments[0], "b");
|
||||
});
|
||||
|
||||
test("first-load settle: re-pins to bottom as total size grows while pinned", () => {
|
||||
// First load: rows paint with estimated heights, then measured heights /
|
||||
// streaming rows grow getTotalSize(). While pinned, each growth re-anchors to
|
||||
// the bottom so the viewport holds at the newest message.
|
||||
const messages = [message({ id: "a" }), message({ id: "b" })];
|
||||
const rows = buildVirtualTimelineRows(messages);
|
||||
const harness = makeVirtualizer(100);
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ totalSizeTick }: { totalSizeTick: number }) =>
|
||||
useVirtualTimelineScroll({
|
||||
channelId: "c1",
|
||||
isLoading: false,
|
||||
messages,
|
||||
rows,
|
||||
// Pinned at bottom (default scroll metrics read as at-bottom).
|
||||
scrollContainerRef: makeContainerRef(true),
|
||||
virtualizer: harness.virtualizer,
|
||||
// totalSizeTick is unused by the hook — it just forces a re-render after
|
||||
// we bump the fake's total size, mirroring react-virtual's onChange.
|
||||
...({ totalSizeTick } as Record<string, never>),
|
||||
}),
|
||||
{ initialProps: { totalSizeTick: 0 } },
|
||||
);
|
||||
|
||||
const endPinsBefore = harness.scrollToIndex.mock.calls.filter(
|
||||
(c) => c.arguments[1]?.align === "end",
|
||||
).length;
|
||||
|
||||
// Document grows (measured heights settle / more rows stream in).
|
||||
act(() => {
|
||||
harness.setTotalSize(900);
|
||||
rerender({ totalSizeTick: 1 });
|
||||
});
|
||||
|
||||
const endPinsAfter = harness.scrollToIndex.mock.calls.filter(
|
||||
(c) => c.arguments[1]?.align === "end",
|
||||
).length;
|
||||
assert.ok(
|
||||
endPinsAfter > endPinsBefore,
|
||||
"expected a re-pin to bottom when total size grew while pinned",
|
||||
);
|
||||
const lastEnd = harness.scrollToIndex.mock.calls
|
||||
.filter((c) => c.arguments[1]?.align === "end")
|
||||
.at(-1);
|
||||
assert.equal(lastEnd?.arguments[0], rows.length - 1);
|
||||
});
|
||||
|
||||
test("first-load settle: does NOT re-pin after the user scrolls away", () => {
|
||||
const messages = [message({ id: "a" }), message({ id: "b" })];
|
||||
const rows = buildVirtualTimelineRows(messages);
|
||||
const harness = makeVirtualizer(100);
|
||||
// Container reads as NOT at bottom — the initial syncScrollState/scroll state
|
||||
// leaves stickToBottom false, so the settle effect must not yank back down.
|
||||
const containerRef = makeContainerRef(false);
|
||||
|
||||
const { result, rerender } = renderHook(
|
||||
({ totalSizeTick }: { totalSizeTick: number }) =>
|
||||
useVirtualTimelineScroll({
|
||||
channelId: "c1",
|
||||
isLoading: false,
|
||||
messages,
|
||||
rows,
|
||||
scrollContainerRef: containerRef,
|
||||
virtualizer: harness.virtualizer,
|
||||
...({ totalSizeTick } as Record<string, never>),
|
||||
}),
|
||||
{ initialProps: { totalSizeTick: 0 } },
|
||||
);
|
||||
|
||||
// User has scrolled up — reflect that through syncScrollState.
|
||||
act(() => {
|
||||
result.current.syncScrollState();
|
||||
});
|
||||
const endPinsBefore = harness.scrollToIndex.mock.calls.filter(
|
||||
(c) => c.arguments[1]?.align === "end",
|
||||
).length;
|
||||
|
||||
act(() => {
|
||||
harness.setTotalSize(900);
|
||||
rerender({ totalSizeTick: 1 });
|
||||
});
|
||||
|
||||
const endPinsAfter = harness.scrollToIndex.mock.calls.filter(
|
||||
(c) => c.arguments[1]?.align === "end",
|
||||
).length;
|
||||
assert.equal(
|
||||
endPinsAfter,
|
||||
endPinsBefore,
|
||||
"must not re-pin to bottom once the user has scrolled away",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -60,6 +60,10 @@ export function useVirtualTimelineScroll({
|
||||
const previousMessageCountRef = React.useRef(0);
|
||||
const handledTargetMessageIdRef = React.useRef<string | null>(null);
|
||||
const handledSearchActiveIdRef = React.useRef<string | null>(null);
|
||||
// Total virtual size at the last time we pinned to bottom — lets the
|
||||
// settle-pin effect below re-anchor only when the size actually changed,
|
||||
// instead of looping on its own scroll-induced re-renders.
|
||||
const lastPinnedTotalSizeRef = React.useRef(-1);
|
||||
|
||||
const [isAtBottom, setIsAtBottom] = React.useState(true);
|
||||
const [newMessageCount, setNewMessageCount] = React.useState(0);
|
||||
@@ -78,6 +82,9 @@ export function useVirtualTimelineScroll({
|
||||
setNewMessageCount(0);
|
||||
setIsAtBottom(true);
|
||||
virtualizer.scrollToIndex(lastRowIndex, { align: "end", behavior });
|
||||
// Mark the current size as pinned so the settle effect doesn't redundantly
|
||||
// re-fire for this same commit.
|
||||
lastPinnedTotalSizeRef.current = virtualizer.getTotalSize();
|
||||
},
|
||||
[lastRowIndex, virtualizer],
|
||||
);
|
||||
@@ -91,6 +98,7 @@ export function useVirtualTimelineScroll({
|
||||
previousMessageCountRef.current = 0;
|
||||
handledTargetMessageIdRef.current = null;
|
||||
handledSearchActiveIdRef.current = null;
|
||||
lastPinnedTotalSizeRef.current = -1;
|
||||
setIsAtBottom(true);
|
||||
setNewMessageCount(0);
|
||||
setHighlightedMessageId(null);
|
||||
@@ -170,6 +178,33 @@ export function useVirtualTimelineScroll({
|
||||
targetMessageId,
|
||||
]);
|
||||
|
||||
// Keep pinned to the bottom while the document settles. On first load the
|
||||
// virtualizer paints with ESTIMATED row heights and the deferred snapshot
|
||||
// streams in over several commits, so the true bottom keeps moving after the
|
||||
// one-shot init pin above. As `getTotalSize()` grows (estimate→measured
|
||||
// heights, more rows, container resize), re-anchor to the bottom — but ONLY
|
||||
// while still pinned and not chasing a deep-link, so a user who scrolled up is
|
||||
// never yanked back down. Guarded on a real size change so the pin's own
|
||||
// scroll-induced re-render can't loop. This is what makes first-load "land and
|
||||
// hold at the bottom" instead of anchoring up top as content fills in.
|
||||
const totalSize = virtualizer.getTotalSize();
|
||||
React.useLayoutEffect(() => {
|
||||
if (
|
||||
isLoading ||
|
||||
targetMessageId ||
|
||||
!hasInitializedRef.current ||
|
||||
!stickToBottomRef.current ||
|
||||
lastRowIndex < 0
|
||||
) {
|
||||
return;
|
||||
}
|
||||
if (totalSize === lastPinnedTotalSizeRef.current) {
|
||||
return;
|
||||
}
|
||||
lastPinnedTotalSizeRef.current = totalSize;
|
||||
virtualizer.scrollToIndex(lastRowIndex, { align: "end" });
|
||||
}, [totalSize, isLoading, targetMessageId, lastRowIndex, virtualizer]);
|
||||
|
||||
// Deep-link jump-to-message. Drives the virtualizer to mount and center the
|
||||
// target row, replacing the bespoke querySelector + scrollIntoView path that
|
||||
// breaks under virtualization (the row may be unmounted).
|
||||
|
||||
Reference in New Issue
Block a user