External/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-08-18 06:50:31 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(desktop): restore anchor on in-viewport reflow when scrolled up

Browse Source 
The ResizeObserver in useAnchoredScroll only re-pinned the scroller when
the user was at-bottom — when scrolled up reading older history and a row
above the reading row reflowed (link-card decode, async embed expand, late
font load, markdown that expands), the anchor row shifted on them and
nothing restored it. The PR description claimed image and embed loads all
flow through the anchor, but on a careful read only NIP-92 imeta images
are actually covered (their dim is reserved before decode, so no resize
fires). Every other in-viewport content growth fell into the gap.

Extract the anchor-restoration primitive — find the row (with the
nearest-newer fallback already used for prepends), measure its current
top, scrollBy the delta — into a shared restoreAnchorToMessage helper,
and call it from both the layout effect and the ResizeObserver. One
primitive serves the React-driven path (post-commit, on messages /
spinner change) and the non-React-driven path (image decode, embed
expand, font load), preserving the single-owner invariant. A messagesRef
is mirrored from the layout effect so the observer reads the same list
the DOM was last rendered from without resubscribing on every commit.

E2E coverage: scroll-history e2e adds an in-viewport reflow case that
seeds a scrollable channel, scrolls to a mid position, captures the
top-crossing row's offset, programmatically grows a row above the anchor
via style.minHeight, and asserts the anchor's offset is unchanged within
2px after the observer fires. Confirmed the assertion catches the
pre-fix behavior (80px drift) by reverting the implementation and
re-running.

tsc, biome (764 files), 998/998 unit, scroll-history e2e 7/7 green.

Co-authored-by: Tyler Longwell <tlongwell@squareup.com>
Signed-off-by: Tyler Longwell <tlongwell@squareup.com>
This commit is contained in:
npub17jjz49l9jjmhhk7cac63j8yt9z555n9cw8vk7v5jz4vzw4ppld5qgj57cc
2026-06-18 16:39:26 -04:00
co-authored by Tyler Longwell
parent 5a42d3706a
commit 78ac6caf19
2 changed files with 216 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files
desktop/src/features/messages/ui/useAnchoredScroll.ts
+107 -51
View File
@@ -140,6 +140,71 @@ function findNearestNewerMessageId(
return null;
}
/**
* Restore a message-kind anchor's on-screen offset after a layout shift.
*
* Finds the anchor row (or the nearest newer rendered row if the anchor
* itself was removed), measures its current top-relative offset, and
* `scrollBy(0, delta)` if the offset has drifted. Returns the new anchor
* state the caller should write back:
* - `{ kind: "message", ... }` — anchor (or its fallback) is in the DOM
* and now sits at its previous offset.
* - `{ kind: "at-bottom" }` — anchor and all newer rendered rows are gone;
* caller should pin to the bottom and update at-bottom state.
*
* `scrollBy` is intentional over `scrollTop = ...`: relative adjustment
* composes with the browser's own scroll anchoring and doesn't fight a
* smooth-scroll in flight. Same rationale as the layout-effect restore.
*
* Used by both the post-commit layout effect (prepend / append / spinner
* toggle / etc.) and the ResizeObserver (in-viewport reflow from image
* decode, embed expansion, font load). Keeping them on one primitive
* preserves the single-owner invariant of the hook.
*/
function restoreAnchorToMessage(
container: HTMLDivElement,
messages: TimelineMessage[],
anchor: Extract<AnchorState, { kind: "message" }>,
): AnchorState {
let anchorEl = container.querySelector<HTMLElement>(
`[data-message-id="${anchor.messageId}"]`,
);
let usedAnchor: AnchorState = anchor;
if (!anchorEl) {
const fallbackId = findNearestNewerMessageId(
container,
messages,
anchor.messageId,
);
if (fallbackId) {
anchorEl = container.querySelector<HTMLElement>(
`[data-message-id="${fallbackId}"]`,
);
if (anchorEl) {
usedAnchor = {
kind: "message",
messageId: fallbackId,
topOffset: anchor.topOffset,
};
}
}
}
if (!anchorEl) {
// Anchor message and all subsequent rendered messages are gone.
container.scrollTo({ top: container.scrollHeight, behavior: "auto" });
return { kind: "at-bottom" };
}
const containerTop = container.getBoundingClientRect().top;
const currentTop = anchorEl.getBoundingClientRect().top - containerTop;
const delta = currentTop - usedAnchor.topOffset;
if (Math.abs(delta) > 0.5) {
container.scrollBy(0, delta);
}
return usedAnchor;
}
export function useAnchoredScroll({
scrollContainerRef,
contentRef,
@@ -157,6 +222,11 @@ export function useAnchoredScroll({
// both on scroll (commit-time read) and in the layout effect (post-render
// restoration). useState would force re-renders we don't want.
const anchorRef = React.useRef<AnchorState>({ kind: "at-bottom" });
// Latest `messages` mirrored to a ref so the ResizeObserver effect can read
// the current list without re-subscribing the observer on every commit
// (which would also drop any in-flight resize callbacks). Kept fresh by a
// layout effect below so the read is consistent with what's in the DOM.
const messagesRef = React.useRef<TimelineMessage[]>(messages);
const [isAtBottom, setIsAtBottom] = React.useState(true);
const [newMessageCount, setNewMessageCount] = React.useState(0);
const [highlightedMessageId, setHighlightedMessageId] = React.useState<
@@ -285,6 +355,12 @@ export function useAnchoredScroll({
const container = scrollContainerRef.current;
if (!container) return;
// Mirror the current messages list into the ref read by the
// ResizeObserver's restore path. Must happen before any early return so
// a non-React layout shift sees the same array the next restoration
// would use.
messagesRef.current = messages;
// First render after a reset (channel switch or initial mount): jump
// to the requested target message, or to the bottom by default.
if (!hasInitializedRef.current) {
@@ -342,49 +418,13 @@ export function useAnchoredScroll({
container.scrollTo({ top: container.scrollHeight, behavior: "auto" });
if (newLatestArrived) setNewMessageCount(0);
} else {
// Anchored to a specific message. Find it; if it's gone, fall back to
// the nearest newer rendered message; if that's also gone, give up
// and pin to bottom.
let anchorEl = container.querySelector<HTMLElement>(
`[data-message-id="${anchor.messageId}"]`,
);
let usedAnchor: AnchorState = anchor;
if (!anchorEl) {
const fallbackId = findNearestNewerMessageId(
container,
messages,
anchor.messageId,
);
if (fallbackId) {
anchorEl = container.querySelector<HTMLElement>(
`[data-message-id="${fallbackId}"]`,
);
if (anchorEl) {
usedAnchor = {
kind: "message",
messageId: fallbackId,
topOffset: anchor.topOffset,
};
}
}
}
if (anchorEl) {
const containerTop = container.getBoundingClientRect().top;
const currentTop = anchorEl.getBoundingClientRect().top - containerTop;
const delta = currentTop - usedAnchor.topOffset;
if (Math.abs(delta) > 0.5) {
// `scrollBy` is intentional over `scrollTop = ...`: relative
// adjustment composes with whatever the browser's own scroll
// anchoring did, and it doesn't fight a smooth-scroll in flight.
container.scrollBy(0, delta);
}
anchorRef.current = usedAnchor;
} else {
// Anchor message and all subsequent rendered messages are gone.
// Last-resort fallback so the user doesn't end up stranded.
container.scrollTo({ top: container.scrollHeight, behavior: "auto" });
anchorRef.current = { kind: "at-bottom" };
// Anchored to a specific message. The shared helper finds it (with a
// nearest-newer fallback if the row was removed) and corrects the
// offset via `scrollBy`. If both the anchor and all newer rendered
// rows are gone, it pins to the bottom and returns `at-bottom`.
const restored = restoreAnchorToMessage(container, messages, anchor);
anchorRef.current = restored;
if (restored.kind === "at-bottom") {
setIsAtBottom(true);
}
@@ -473,11 +513,16 @@ export function useAnchoredScroll({
]);
// ---------------------------------------------------------------------------
// Content resize: when fonts load late, an image decodes, or an embed
// expands, the row positions shift. A ResizeObserver fires a synthetic
// scroll so the anchor is recomputed; the layout effect on the next
// render will then restore. We deliberately do NOT call scrollTo here —
// that would fight the anchor restoration.
// Content resize: when fonts load late, an image decodes, an embed expands,
// or any in-viewport reflow happens that React isn't driving (so the
// layout-effect doesn't fire), the anchor row's on-screen offset drifts.
//
// When stuck-to-bottom we re-pin to bottom. When anchored to a message we
// call the same restore primitive the layout effect uses, so an in-viewport
// reflow above the reader's eye shifts back into place. Without this,
// anything that resizes without changing `messages` (link-card decode,
// async embed expand, late font load, markdown that expands) silently
// pushes the reading row around.
// ---------------------------------------------------------------------------
React.useEffect(() => {
const content = contentRef.current;
@@ -485,10 +530,21 @@ export function useAnchoredScroll({
const observer = new ResizeObserver(() => {
const container = scrollContainerRef.current;
if (!container) return;
// If we're stuck-to-bottom, just stay there. Otherwise let the next
// layout effect handle restoration via the existing anchor.
if (anchorRef.current.kind === "at-bottom") {
const anchor = anchorRef.current;
if (anchor.kind === "at-bottom") {
container.scrollTo({ top: container.scrollHeight, behavior: "auto" });
return;
}
// Use the same restore primitive as the layout effect so the
// single-owner model holds across non-React-driven layout shifts.
const restored = restoreAnchorToMessage(
container,
messagesRef.current,
anchor,
);
anchorRef.current = restored;
if (restored.kind === "at-bottom") {
setIsAtBottom(true);
}
});
observer.observe(content);
desktop/tests/e2e/scroll-history.spec.ts
+109
View File
@@ -939,3 +939,112 @@ test("composer expansion does not push bottom row out of viewport", async ({
if (!after.found) return;
expect(after.gapAboveComposer).toBeGreaterThanOrEqual(-4);
});
// Criterion 8: in-viewport content resize while scrolled up preserves the
// anchor row's position.
//
// The hook owns scroll position via `useAnchoredScroll`. When the user is
// scrolled up reading older history and a row *above* their reading row
// reflows (image decode without reserved dim metadata, link-card load,
// async embed expand, late font load, markdown that expands), the rows
// below shift on them. Before this test landed, the ResizeObserver only
// re-pinned when stuck-to-bottom; the scrolled-up case had no correction.
//
// The fix: the ResizeObserver calls the same anchor-restore primitive as
// the post-commit layout effect when anchored to a message. This test
// reproduces the scenario without touching React state — it directly
// grows a DOM row's height via a style override, which is exactly the
// kind of layout shift that previously had no correction (the messages
// array is unchanged, so the React layout effect never runs).
//
// Black-box assertions: the anchor row's top within the timeline must be
// unchanged (within 2px) before and after the synthetic above-anchor
// height growth.
test("in-viewport reflow above the anchor row does not push it down", async ({
page,
}) => {
await installMockBridge(page);
await page.goto("/");
await page.waitForFunction(
() => typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function",
);
// Seed enough rows that the timeline becomes scrollable with several
// rows above whatever we anchor to.
await page.evaluate(() => {
for (let index = 0; index < 60; index += 1) {
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
channelName: "general",
content: `resize-anchor row ${index}\nsecond line ${index}\nthird line ${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("resize-anchor row 59");
// Wait until the timeline is genuinely scrollable.
await page.waitForFunction(() => {
const element = document.querySelector(
'[data-testid="message-timeline"]',
) as HTMLDivElement | null;
return element && element.scrollHeight > element.clientHeight + 800;
});
// Scroll to a middle position so we have rows on both sides of the anchor.
await timeline.evaluate((element) => {
const t = element as HTMLDivElement;
t.scrollTop = Math.floor(t.scrollHeight / 2);
t.dispatchEvent(new Event("scroll", { bubbles: true }));
});
await page.waitForTimeout(50);
// Capture the anchor row (top-crossing) and its baseline top within
// the timeline. This is the row the user is reading.
const baseline = await getFirstVisibleMessage(page);
expect(baseline).not.toBeNull();
if (!baseline) return;
// Find a rendered row *above* the anchor and grow its height. This
// mimics an in-viewport reflow (image decode, embed expansion) that
// does NOT change the messages array, so the React layout effect
// would not fire. The ResizeObserver is the only path that can
// correct the resulting shift.
const growthApplied = await timeline.evaluate((element, anchorId) => {
const t = element as HTMLDivElement;
const rows = Array.from(
t.querySelectorAll<HTMLElement>("[data-message-id]"),
);
const anchorIndex = rows.findIndex(
(row) => row.dataset.messageId === anchorId,
);
if (anchorIndex <= 0) return false;
// Pick a row a few above the anchor so the growth is clearly above
// the reader's eye, not at it.
const target = rows[Math.max(0, anchorIndex - 3)];
if (!target) return false;
// 80px is well above the 0.5px noise floor in restoreAnchorToMessage
// and large enough to be a visible jump if uncorrected.
const currentHeight = target.getBoundingClientRect().height;
target.style.minHeight = `${currentHeight + 80}px`;
return true;
}, baseline.id);
expect(growthApplied).toBe(true);
// ResizeObserver callbacks run asynchronously after layout. Poll the
// anchor row's position; it must converge back to (or stay at) its
// baseline top within ~2px.
await expect
.poll(
async () => {
const current = await getMessagePosition(page, baseline.id);
return current
? Math.abs(current.top - baseline.top)
: Number.POSITIVE_INFINITY;
},
{ timeout: 3_000 },
)
.toBeLessThanOrEqual(2);
});