mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(timeline): gate first-load bottom pin until scroll margin is measured
Kills the first-load out-of-place flash. On `isLoading: true→false` in
one commit, two sibling layout effects race: the init bottom pin
(scrollToBottom → scrollToIndex align:end) and the scrollMargin
re-measure (which only mounts once the list outer ref mounts). The pin
can fire against a STALE `scrollMargin: 0`, landing scrollMargin px short
of true bottom — rows paint at translateY(start - 0) while the list
actually sits scrollMargin lower. That's the flash. The margin then lands
and the settle pin re-anchors, catching up.
Fix: gate the init pin on a measured margin.
- useVirtualScrollMargin now returns { value, measured }. `measured` flips
true only after a real measurement against a mounted list — a `> 0`
test won't do, since a legitimate margin can be 0.
- The init pin bails on `isLoading || !scrollMarginReady` WITHOUT marking
hasInitialized, so it re-fires once the margin lands and pins against a
trustworthy offset. scrollMarginReady added to the effect deps.
- MessageTimeline threads scrollMargin.value into the virtualizer + list
prop and scrollMargin.measured into the scroll hook.
Adds a DOM regression test: 0 end-pins while the margin is unmeasured,
exactly 1 once it lands.
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
59464fd425
commit
d39fb399f0
@@ -247,7 +247,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({
|
||||
overscan,
|
||||
// Account for the sentinel/spinner/intro above the list inside the same
|
||||
// scroll container, so item offsets line up with where they actually paint.
|
||||
scrollMargin,
|
||||
scrollMargin: scrollMargin.value,
|
||||
});
|
||||
|
||||
const {
|
||||
@@ -263,6 +263,11 @@ export const MessageTimeline = React.memo(function MessageTimeline({
|
||||
rows,
|
||||
scrollContainerRef,
|
||||
virtualizer,
|
||||
// The init bottom pin must wait until the list's scroll margin is measured;
|
||||
// pinning against the pre-mount stale `0` lands `scrollMargin` px short of
|
||||
// true bottom and paints the rows out of place for a beat (the first-load
|
||||
// flash) before re-anchoring. `measured` gates that first pin.
|
||||
scrollMarginReady: scrollMargin.measured,
|
||||
targetMessageId,
|
||||
onTargetReached,
|
||||
searchActiveMessageId,
|
||||
@@ -540,7 +545,7 @@ export const MessageTimeline = React.memo(function MessageTimeline({
|
||||
entries={entries}
|
||||
renderEntry={renderEntry}
|
||||
rows={rows}
|
||||
scrollMargin={scrollMargin}
|
||||
scrollMargin={scrollMargin.value}
|
||||
virtualizer={virtualizer}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -15,14 +15,29 @@ import * as React from "react";
|
||||
* We re-measure whenever the above-content can change height (intro mount/
|
||||
* unmount, spinner toggle) AND via a ResizeObserver on the scroll container, so
|
||||
* the margin stays correct as content streams in.
|
||||
*
|
||||
* Returns both the margin and a `measured` flag. The flag matters because a
|
||||
* legitimate margin can be `0` (nothing above the list), so callers that must
|
||||
* not act on a STALE pre-mount margin — e.g. the first-load bottom pin — can't
|
||||
* just test `margin > 0`. `measured` flips true only after the list has mounted
|
||||
* and we've taken a real measurement, so the init pin can wait for a trustworthy
|
||||
* offset instead of pinning against the pre-mount `0` and flashing out of place.
|
||||
*/
|
||||
export type VirtualScrollMargin = {
|
||||
/** The list's measured offset within the scroll container (px). */
|
||||
value: number;
|
||||
/** True once a real measurement has been taken (list was mounted). */
|
||||
measured: boolean;
|
||||
};
|
||||
|
||||
export function useVirtualScrollMargin(
|
||||
scrollContainerRef: React.RefObject<HTMLDivElement | null>,
|
||||
listOuterRef: React.RefObject<HTMLDivElement | null>,
|
||||
// Re-measure triggers — values whose change can shift the list's offset.
|
||||
deps: ReadonlyArray<unknown>,
|
||||
): number {
|
||||
): VirtualScrollMargin {
|
||||
const [scrollMargin, setScrollMargin] = React.useState(0);
|
||||
const [measured, setMeasured] = React.useState(false);
|
||||
|
||||
React.useLayoutEffect(() => {
|
||||
const container = scrollContainerRef.current;
|
||||
@@ -45,6 +60,9 @@ export function useVirtualScrollMargin(
|
||||
c.scrollTop,
|
||||
);
|
||||
setScrollMargin((current) => (current === next ? current : next));
|
||||
// We've taken a real measurement against a mounted list — the margin is
|
||||
// now trustworthy for the init pin (even if its value is 0).
|
||||
setMeasured((current) => (current ? current : true));
|
||||
};
|
||||
|
||||
measure();
|
||||
@@ -60,5 +78,5 @@ export function useVirtualScrollMargin(
|
||||
// deps drive intentional re-measures (intro/spinner/list visibility).
|
||||
}, [scrollContainerRef, listOuterRef, ...deps]);
|
||||
|
||||
return scrollMargin;
|
||||
return { value: scrollMargin, measured };
|
||||
}
|
||||
|
||||
@@ -72,6 +72,7 @@ test("on init with no deep-link target, scrolls to the last row (sticky bottom)"
|
||||
rows,
|
||||
scrollContainerRef: makeContainerRef(true),
|
||||
virtualizer,
|
||||
scrollMarginReady: true,
|
||||
}),
|
||||
);
|
||||
|
||||
@@ -96,6 +97,7 @@ test("a new latest message while pinned autoscrolls; accent uses smooth", () =>
|
||||
rows,
|
||||
scrollContainerRef: makeContainerRef(true),
|
||||
virtualizer,
|
||||
scrollMarginReady: true,
|
||||
}),
|
||||
{ initialProps: { messages: initial, rows: rows1 } },
|
||||
);
|
||||
@@ -132,6 +134,7 @@ test("a deep-link target scrolls to that message's flat row and centers it", ()
|
||||
rows,
|
||||
scrollContainerRef: makeContainerRef(false),
|
||||
virtualizer,
|
||||
scrollMarginReady: true,
|
||||
targetMessageId: "b",
|
||||
onTargetReached,
|
||||
}),
|
||||
@@ -165,6 +168,7 @@ test("first-load settle: re-pins to bottom as total size grows while pinned", ()
|
||||
// Pinned at bottom (default scroll metrics read as at-bottom).
|
||||
scrollContainerRef: makeContainerRef(true),
|
||||
virtualizer: harness.virtualizer,
|
||||
scrollMarginReady: true,
|
||||
// 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>),
|
||||
@@ -212,6 +216,7 @@ test("first-load settle: does NOT re-pin after the user scrolls away", () => {
|
||||
rows,
|
||||
scrollContainerRef: containerRef,
|
||||
virtualizer: harness.virtualizer,
|
||||
scrollMarginReady: true,
|
||||
...({ totalSizeTick } as Record<string, never>),
|
||||
}),
|
||||
{ initialProps: { totalSizeTick: 0 } },
|
||||
@@ -239,3 +244,52 @@ test("first-load settle: does NOT re-pin after the user scrolls away", () => {
|
||||
"must not re-pin to bottom once the user has scrolled away",
|
||||
);
|
||||
});
|
||||
|
||||
test("init pin waits for scrollMarginReady, then pins once the margin lands", () => {
|
||||
// The first-load flash (step 5): the init bottom pin and the scrollMargin
|
||||
// re-measure are sibling layout effects racing in the same `isLoading→false`
|
||||
// commit. If the pin fires while the margin is still the stale pre-mount `0`,
|
||||
// it lands `scrollMargin` px short of true bottom and paints rows out of
|
||||
// place before re-anchoring. Gate: while `scrollMarginReady` is false, the
|
||||
// init pin must NOT fire; once it flips true, it pins exactly once.
|
||||
const messages = [message({ id: "a" }), message({ id: "b" })];
|
||||
const rows = buildVirtualTimelineRows(messages);
|
||||
const { scrollToIndex, virtualizer } = makeVirtualizer();
|
||||
|
||||
const { rerender } = renderHook(
|
||||
({ scrollMarginReady }: { scrollMarginReady: boolean }) =>
|
||||
useVirtualTimelineScroll({
|
||||
channelId: "c1",
|
||||
isLoading: false,
|
||||
messages,
|
||||
rows,
|
||||
scrollContainerRef: makeContainerRef(true),
|
||||
virtualizer,
|
||||
scrollMarginReady,
|
||||
}),
|
||||
{ initialProps: { scrollMarginReady: false } },
|
||||
);
|
||||
|
||||
// Margin not yet measured — the init pin must hold.
|
||||
assert.equal(
|
||||
scrollToIndex.mock.calls.filter((c) => c.arguments[1]?.align === "end")
|
||||
.length,
|
||||
0,
|
||||
"init pin must not fire before the scroll margin is measured",
|
||||
);
|
||||
|
||||
// Margin lands — the init pin fires now, against a trustworthy offset.
|
||||
act(() => {
|
||||
rerender({ scrollMarginReady: true });
|
||||
});
|
||||
|
||||
const endPins = scrollToIndex.mock.calls.filter(
|
||||
(c) => c.arguments[1]?.align === "end",
|
||||
);
|
||||
assert.equal(
|
||||
endPins.length,
|
||||
1,
|
||||
"init pin must fire exactly once after the margin is measured",
|
||||
);
|
||||
assert.equal(endPins[0]?.arguments[0], rows.length - 1);
|
||||
});
|
||||
|
||||
@@ -21,6 +21,13 @@ type UseVirtualTimelineScrollOptions = {
|
||||
rows: VirtualTimelineRow[];
|
||||
scrollContainerRef: React.RefObject<HTMLDivElement | null>;
|
||||
virtualizer: Virtualizer<HTMLDivElement, Element>;
|
||||
/**
|
||||
* True once the list's `scrollMargin` has been measured against a mounted
|
||||
* list. The first-load bottom pin waits on this: pinning while the margin is
|
||||
* still the pre-mount stale `0` lands `scrollMargin` px short of true bottom
|
||||
* and paints the rows out of place for a beat before re-anchoring.
|
||||
*/
|
||||
scrollMarginReady: boolean;
|
||||
targetMessageId?: string | null;
|
||||
onTargetReached?: (messageId: string) => void;
|
||||
/** The currently active find-in-channel match, drives scroll-to-row. */
|
||||
@@ -50,6 +57,7 @@ export function useVirtualTimelineScroll({
|
||||
rows,
|
||||
scrollContainerRef,
|
||||
virtualizer,
|
||||
scrollMarginReady,
|
||||
targetMessageId,
|
||||
onTargetReached,
|
||||
searchActiveMessageId,
|
||||
@@ -128,7 +136,14 @@ export function useVirtualTimelineScroll({
|
||||
// autoscroll if pinned or accented, otherwise bump the "N new messages" pill.
|
||||
React.useLayoutEffect(() => {
|
||||
if (!hasInitializedRef.current) {
|
||||
if (isLoading) {
|
||||
// Wait for the first paint to settle: `isLoading` clearing means the rows
|
||||
// are mounting, but the list's `scrollMargin` is measured in a SIBLING
|
||||
// layout effect that races this one in the same commit. Pinning before
|
||||
// the margin lands anchors against the stale pre-mount `0`, landing
|
||||
// `scrollMargin` px short of true bottom — the out-of-place first-load
|
||||
// flash. Hold the init pin (without marking initialized) until the margin
|
||||
// is measured, so the very first pin lands against a trustworthy offset.
|
||||
if (isLoading || !scrollMarginReady) {
|
||||
return;
|
||||
}
|
||||
if (!targetMessageId) {
|
||||
@@ -174,6 +189,7 @@ export function useVirtualTimelineScroll({
|
||||
latestMessage,
|
||||
latestMessageKey,
|
||||
messages.length,
|
||||
scrollMarginReady,
|
||||
scrollToBottom,
|
||||
targetMessageId,
|
||||
]);
|
||||
|
||||
Reference in New Issue
Block a user