mirror of
https://github.com/block/buzz.git
synced 2026-10-29 21:41:33 +00:00
fix(desktop): always flash a linked message in the channel timeline (#8660)
## Problem When you follow a link to a message, the channel timeline should scroll to it and flash the row for about 2 seconds. On `main` the flash often does not appear: | How you reach the message | Flash on `main` | |---|---| | Same channel, row already on screen | yes | | Same channel, row loaded but off screen | **no** | | Same channel, row in older history not loaded yet | yes | | From another channel, row near the bottom | **no** | | From another channel, row further up | **no**, and the timeline stays at the bottom | The main timeline is always virtualized. `useAnchoredScroll` had three ways to scroll to a target there. Only one of them, "the row is already fully on screen", fired the flash. In the other cases the scroll returned without the flash, and the follow-up target effect marked the target handled without checking the result, so the flash never fired. Opening a link from another channel had a second problem. The row could look on screen while the channel's first bottom pin was still in flight in Virtua. The shortcut used a direct DOM `scrollTo`, which does not cancel that pin, so Virtua kept going to the bottom and took the target with it. Top-level message links also open the thread panel, and that resize settled the timeline at the bottom again. A third problem also exists on `main`. Centering a row near the top of the loaded history loads an older page. Opening a channel also starts a refresh of the newest channel window, and that refresh then drops the older pages from the head of the list. A head trim was not passed to Virtua as a `shift`, so the rows above the reader vanished with no compensation and the view fell to the bottom about 0.3–0.8 s after the flash started. The deep-link e2e test stopped checking the flash in #5306, so CI did not catch this. ## Change - In virtualized mode, every target scroll now goes through Virtua's `scrollToIndex` (centered). That replaces any bottom pin still in flight. Smooth scrolling is kept only when the row is already in the viewport. - Every target scroll that Virtua accepts now fires the flash. The highlight is keyed by message id, so a row that Virtua mounts after the scroll still shows it. - The scroll returns `true` once Virtua accepts the target and `false` while the target is not loaded yet. The follow-up target effect and the search retry now just read that value. This removes the "phase 1" special case and the duplicate DOM lookup. - A target scroll also clears the virtualizer's at-bottom state, so a resize right after it (for example the thread panel opening) does not pin back to the bottom. - Only an accepted target scroll retires bottom intent. It also cancels the hook's queued second mount-to-bottom frame. A target that is not loaded yet leaves the opening bottom pin alone while it retries. - `didPrependVirtualizedTimeline` now also admits a pure head trim (the new keys are an exact suffix of the previous keys), so Virtua's `shift` keeps the reading row in place when a window refresh drops older pages. The oldest loaded day has no divider while more history exists, so a head trim stays an exact suffix even when it cuts inside a day. The non-virtualized path (thread panel) is unchanged. ## Screenshots Before and after, from another channel, for a link to a row further up: https://github.com/block/buzz/pull/8660#issuecomment-6481877304 ## Testing Evidence at head `653d89766`: - New `desktop/tests/e2e/message-target-highlight.spec.ts`: a 5-row table with the cases above. Each case asserts that the target row is on screen **and** has the flash class at the same time, and that the row is still on screen 2.2 s later, after the flash ends. 15/15 passed (3 runs each). - On unfixed `main`, 3 of the 5 cases fail the flash check. - With only the head-trim change reverted, "same channel, loaded off screen" and "other channel, further up" fail the 2.2 s check. - `useAnchoredScroll.lifecycle.test.mjs`: an on-screen virtual target scrolls through the virtualizer, makes no direct DOM write, retires bottom intent and does not re-pin later. New: a target that is not loaded yet keeps the bottom pin. It fails with the old cancel order. - `virtualizedTimelineItems.test.mjs`: new rows for a head trim inside the oldest day and at its boundary (admitted, with the size cache aligned), and for a trim plus append and a middle removal (rejected). - `pnpm test` (6837 + 161 pass), `pnpm check` (tsc via pre-push, biome, px-text, pubkey truncation). - These e2e specs, run together: message-target-highlight, scroll-history, virtualization, timeline-no-shift, unread-pill, overscroll-boundary, dm-history-live-regression, live-broadcast-reply-timeline, message-copy-link, thread-focus-mode, thread-follow, thread-reply-anchor-roleplay, thread-unread, channels, messaging. 257 passed, 0 failed. Later commits at `b8e172672` add no production behavior. They move one helper above `computeAnchor`'s doc comment, and add a lifecycle test (from Cerveau's probe): a target that loads before the queued second mount frame is not scrolled back to the bottom. That test fails without the mount-frame cancellation. Pre-push `pnpm test` passed at `b8e172672`. Adversarial review by Auto (virtualizer and callers) and Cerveau (lifecycle, tests and thread panel). Their must-fix findings are fixed above, and both approved `653d89766`. Follow-up, not in this PR: after a jump to a row at the very bottom, centering is clamped, so no scroll event fires and the at-bottom state stays false until the next scroll. `main` has the same gap for `isAtBottom`. Not tested yet in the native app. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Signed-off-by: Logan Johnson <loganj@squareup.com> Co-authored-by: Larry <627498bd4bd1f281a16431e3c6cce3b5c25b6692798c78672298aefbf2f8f8b5@buzz.block.builderlab.xyz> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Cerveau <fc91c5708d48aecea993d9219505dc841bc7b0222b7ed325b8edfd026bfb43b5@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -116,6 +116,7 @@ export default defineConfig({
|
||||
"**/reminder-click-repro.spec.ts",
|
||||
"**/virtualization.spec.ts",
|
||||
"**/scroll-history.spec.ts",
|
||||
"**/message-target-highlight.spec.ts",
|
||||
"**/channel-dense-second-reach.spec.ts",
|
||||
"**/channel-window-mock-paging.spec.ts",
|
||||
"**/channel-head-restart.spec.ts",
|
||||
|
||||
@@ -206,6 +206,45 @@ test("naive counterexample stays rejected: same key cannot precede prepended row
|
||||
);
|
||||
});
|
||||
|
||||
test("head trim from a window refresh is admitted as a shift", () => {
|
||||
const previousKeys = keysOf([
|
||||
group("day-Z", DAY_Z, ["z1", "z2"]),
|
||||
group("day-A", DAY_A, ["a1", "a2", "a3"]),
|
||||
]);
|
||||
// Trimmed inside the oldest day, and exactly at its proven boundary: the
|
||||
// surviving oldest day loses its divider, so the result is a clean suffix.
|
||||
for (const keys of [
|
||||
keysOf([group("day-A", DAY_A, ["a2", "a3"])]),
|
||||
keysOf([group("day-A", DAY_A, ["a1", "a2", "a3"])]),
|
||||
]) {
|
||||
assert.equal(didPrependVirtualizedTimeline(previousKeys, keys), true);
|
||||
const delta = previousKeys.length - keys.length;
|
||||
keys.forEach((key, index) => {
|
||||
assert.equal(previousKeys[index + delta], key);
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
test("shrinks that are not a pure head trim are not shifted", () => {
|
||||
const previousKeys = keysOf([group("day-A", DAY_A, ["a1", "a2", "a3"])]);
|
||||
// Trimmed from the head while a new message appended.
|
||||
assert.equal(
|
||||
didPrependVirtualizedTimeline(
|
||||
previousKeys,
|
||||
keysOf([group("day-A", DAY_A, ["a3", "a4"])]),
|
||||
),
|
||||
false,
|
||||
);
|
||||
// A message removed from the middle.
|
||||
assert.equal(
|
||||
didPrependVirtualizedTimeline(
|
||||
previousKeys,
|
||||
keysOf([group("day-A", DAY_A, ["a1", "a3"])]),
|
||||
),
|
||||
false,
|
||||
);
|
||||
});
|
||||
|
||||
test("virtualized rows preserve their heterogeneous height estimates", () => {
|
||||
const short = messageItem("short");
|
||||
short.entry.message.body = "hello";
|
||||
|
||||
@@ -107,11 +107,18 @@ export function didPrependVirtualizedTimeline(
|
||||
previousKeys: readonly string[],
|
||||
keys: readonly string[],
|
||||
): boolean {
|
||||
const prependedCount = keys.length - previousKeys.length;
|
||||
const delta = keys.length - previousKeys.length;
|
||||
// Virtua shifts its positional size cache by the length delta, so enable
|
||||
// `shift` only when every previous slot is the exact suffix it expects.
|
||||
if (delta > 0) {
|
||||
return previousKeys.every((key, index) => key === keys[index + delta]);
|
||||
}
|
||||
// A channel-window refresh drops older pages from the start of the list.
|
||||
// Without `shift` the rows above the reader vanish uncompensated and carry
|
||||
// the reading position away, so a pure head trim shifts too.
|
||||
return (
|
||||
prependedCount > 0 &&
|
||||
previousKeys.every((key, index) => key === keys[index + prependedCount])
|
||||
delta < 0 &&
|
||||
keys.length > 0 &&
|
||||
keys.every((key, index) => key === previousKeys[index - delta])
|
||||
);
|
||||
}
|
||||
|
||||
@@ -498,8 +498,6 @@ const MessageTimelineBase = React.forwardRef<
|
||||
[prepareForOwnMessage, scrollToBottom, timelineVirtualizerApi],
|
||||
);
|
||||
|
||||
// Jump-to-message is purely DOM-based now: all loaded rows are mounted, so
|
||||
// `scrollToMessage` always finds the target row. No virtualizer convergence.
|
||||
const jumpToMessage = React.useCallback(
|
||||
(messageId: string, options?: { behavior?: ScrollBehavior }) => {
|
||||
return scrollToMessage(messageId, { highlight: true, ...options });
|
||||
@@ -542,8 +540,8 @@ const MessageTimelineBase = React.forwardRef<
|
||||
|
||||
// Scroll to the active search match when it changes. `jumpToMessage` updates
|
||||
// the scroll anchor (so the post-commit restore won't yank the view back off
|
||||
// the match) and, when virtualized, converges on the target through the index
|
||||
// model — the row may be windowed out of the DOM.
|
||||
// the match) and, when virtualized, scrolls through the index model — the row
|
||||
// may be windowed out of the DOM.
|
||||
const prevSearchActiveRef = React.useRef<string | null>(null);
|
||||
const pendingSearchTargetRef = React.useRef<string | null>(null);
|
||||
React.useEffect(() => {
|
||||
@@ -565,22 +563,10 @@ const MessageTimelineBase = React.forwardRef<
|
||||
}
|
||||
}, [jumpToMessage, searchActiveMessageId, showTimelineSkeleton]);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: deferredMessages and virtualizerRenderVersion are intentional retry triggers — a search hit may be spliced into messages asynchronously, and in virtualized mode a phase-1 index jump only realizes the row; retry when the rendered range changes so the DOM-visible path can center and highlight it.
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: deferredMessages and virtualizerRenderVersion are intentional retry triggers — a search hit may be spliced into messages asynchronously, so retry until the timeline can scroll to it.
|
||||
React.useEffect(() => {
|
||||
const target = pendingSearchTargetRef.current;
|
||||
if (!target || showTimelineSkeleton) return;
|
||||
if (
|
||||
useTimelineVirtualizer &&
|
||||
!activeScrollContainerRef.current?.querySelector(
|
||||
`[data-message-id="${CSS.escape(target)}"]`,
|
||||
)
|
||||
) {
|
||||
// Phase 1: ask the virtualizer to realize the match's index. The retry effect
|
||||
// runs again on range change and the DOM-visible path does the actual
|
||||
// center + highlight once the row exists.
|
||||
void jumpToMessage(target, { behavior: "auto" });
|
||||
return;
|
||||
}
|
||||
if (jumpToMessage(target, { behavior: "auto" })) {
|
||||
pendingSearchTargetRef.current = null;
|
||||
}
|
||||
|
||||
@@ -683,11 +683,14 @@ function VirtualizedTimelineRows({
|
||||
settleAtBottom();
|
||||
},
|
||||
settleAtBottom,
|
||||
scrollToMessage(messageId) {
|
||||
cancelBottomSettle();
|
||||
scrollToMessage(messageId, options) {
|
||||
const list = listRef.current;
|
||||
const index = messageItemIndexByIdRef.current.get(messageId);
|
||||
if (index === undefined) return false;
|
||||
listRef.current?.scrollToIndex(index, { align: "center" });
|
||||
if (!list || index === undefined) return false;
|
||||
list.scrollToIndex(index, {
|
||||
align: "center",
|
||||
smooth: options?.behavior === "smooth",
|
||||
});
|
||||
return true;
|
||||
},
|
||||
};
|
||||
|
||||
@@ -281,7 +281,10 @@ function VirtualTargetHarness({ refs }) {
|
||||
scrollContainerRef: refs.scroller,
|
||||
virtualCancelBottomIntent: bottomApi.cancel,
|
||||
virtualizerOwnsPrependAnchoring: true,
|
||||
virtualScrollToMessage: () => true,
|
||||
virtualScrollToMessage: (messageId, options) => {
|
||||
refs.targetScrolls.push({ messageId, options });
|
||||
return refs.targetResolves;
|
||||
},
|
||||
});
|
||||
React.useLayoutEffect(() => {
|
||||
if (didRun.current) return;
|
||||
@@ -522,7 +525,7 @@ test("user interaction releases and retires a pending pinned target", async () =
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
test("mounted virtual target retires bottom intent before direct centering", async () => {
|
||||
async function renderVirtualTarget({ targetResolves }) {
|
||||
const resizeObservers = [];
|
||||
globalThis.ResizeObserver = class {
|
||||
constructor(callback) {
|
||||
@@ -571,7 +574,9 @@ test("mounted virtual target retires bottom intent before direct centering", asy
|
||||
},
|
||||
},
|
||||
scroller: { current: scroller },
|
||||
targetResolves,
|
||||
targetResult: { current: null },
|
||||
targetScrolls: [],
|
||||
};
|
||||
const root = createRoot(document.createElement("div"));
|
||||
await act(async () => {
|
||||
@@ -579,21 +584,113 @@ test("mounted virtual target retires bottom intent before direct centering", asy
|
||||
});
|
||||
|
||||
assert.deepEqual(bottomWrites, [{ index: 4, options: { align: "end" } }]);
|
||||
assert.equal(refs.targetResult.current, true);
|
||||
assert.equal(row.getBoundingClientRect().top, 180);
|
||||
const bottomGeometryObserver = resizeObservers.find(
|
||||
(observer) =>
|
||||
observer.targets.includes(content) && observer.targets.includes(scroller),
|
||||
);
|
||||
assert.ok(bottomGeometryObserver);
|
||||
bottomGeometryObserver.callback();
|
||||
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
|
||||
const settleBottomGeometry = async () => {
|
||||
const bottomGeometryObserver = resizeObservers.find(
|
||||
(observer) =>
|
||||
observer.targets.includes(content) &&
|
||||
observer.targets.includes(scroller),
|
||||
);
|
||||
assert.ok(bottomGeometryObserver);
|
||||
bottomGeometryObserver.callback();
|
||||
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
|
||||
};
|
||||
return { bottomWrites, refs, root, scroller, settleBottomGeometry };
|
||||
}
|
||||
|
||||
test("mounted virtual target retires bottom intent and scrolls through the virtualizer", async () => {
|
||||
const { bottomWrites, refs, root, scroller, settleBottomGeometry } =
|
||||
await renderVirtualTarget({ targetResolves: true });
|
||||
|
||||
assert.equal(refs.targetResult.current, true);
|
||||
// Even an on-screen row goes through the virtualizer: a direct DOM scroll
|
||||
// would not supersede a bottom pin the virtualizer still has in flight.
|
||||
assert.deepEqual(refs.targetScrolls, [
|
||||
{ messageId: "selected", options: { behavior: "auto" } },
|
||||
]);
|
||||
assert.equal(scroller.scrollTop, 0, "no direct DOM centering write");
|
||||
await settleBottomGeometry();
|
||||
|
||||
assert.equal(
|
||||
row.getBoundingClientRect().top,
|
||||
180,
|
||||
"target remains centered after later virtual geometry activity",
|
||||
);
|
||||
assert.equal(bottomWrites.length, 1, "geometry cannot re-pin to bottom");
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
test("unresolved virtual target keeps the bottom pin", async () => {
|
||||
const { bottomWrites, refs, root, settleBottomGeometry } =
|
||||
await renderVirtualTarget({ targetResolves: false });
|
||||
|
||||
// A target that is not loaded yet retries on later commits. Until it
|
||||
// resolves, the channel must keep following its floor.
|
||||
assert.equal(refs.targetResult.current, false);
|
||||
await settleBottomGeometry();
|
||||
|
||||
assert.equal(bottomWrites.length, 2, "geometry still re-pins to bottom");
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
test("target arriving before the second mount frame supersedes the bottom pin", async () => {
|
||||
const nodes = makePinnedCenterNodes();
|
||||
nodes.container.querySelector = () => null;
|
||||
const frames = new Map();
|
||||
let nextFrameId = 0;
|
||||
const previousRaf = globalThis.requestAnimationFrame;
|
||||
const previousCancelRaf = globalThis.cancelAnimationFrame;
|
||||
globalThis.requestAnimationFrame = (callback) => {
|
||||
nextFrameId += 1;
|
||||
frames.set(nextFrameId, callback);
|
||||
return nextFrameId;
|
||||
};
|
||||
globalThis.cancelAnimationFrame = (id) => frames.delete(id);
|
||||
const writes = [];
|
||||
let targetLoaded = false;
|
||||
const scrollContainerRef = { current: nodes.container };
|
||||
const contentRef = { current: nodes.content };
|
||||
function RaceHarness({ messages }) {
|
||||
useAnchoredScroll({
|
||||
channelId: "conversation",
|
||||
contentRef,
|
||||
isLoading: false,
|
||||
messages,
|
||||
scrollContainerRef,
|
||||
targetMessageId: "selected",
|
||||
virtualizerOwnsPrependAnchoring: true,
|
||||
virtualScrollToBottom: () => writes.push("bottom"),
|
||||
virtualScrollToMessage: () => {
|
||||
if (!targetLoaded) return false;
|
||||
writes.push("target");
|
||||
return true;
|
||||
},
|
||||
});
|
||||
return null;
|
||||
}
|
||||
const root = createRoot(document.createElement("div"));
|
||||
try {
|
||||
// The target is not loaded on mount, so the hook pins to bottom now and
|
||||
// queues a second pin for the next frame.
|
||||
await act(async () =>
|
||||
root.render(
|
||||
React.createElement(RaceHarness, { messages: [{ id: "tail" }] }),
|
||||
),
|
||||
);
|
||||
// The target lands before that frame runs.
|
||||
targetLoaded = true;
|
||||
await act(async () =>
|
||||
root.render(
|
||||
React.createElement(RaceHarness, {
|
||||
messages: [{ id: "selected" }, { id: "tail" }],
|
||||
}),
|
||||
),
|
||||
);
|
||||
await act(async () => {
|
||||
for (const [id, callback] of [...frames]) {
|
||||
frames.delete(id);
|
||||
callback();
|
||||
}
|
||||
});
|
||||
|
||||
assert.equal(writes.at(-1), "target", JSON.stringify(writes));
|
||||
} finally {
|
||||
await act(async () => root.unmount());
|
||||
globalThis.requestAnimationFrame = previousRaf;
|
||||
globalThis.cancelAnimationFrame = previousCancelRaf;
|
||||
}
|
||||
});
|
||||
|
||||
@@ -101,6 +101,12 @@ function isAtBottomNow(
|
||||
);
|
||||
}
|
||||
|
||||
function isInViewport(el: HTMLElement, container: HTMLElement): boolean {
|
||||
const rect = el.getBoundingClientRect();
|
||||
const containerRect = container.getBoundingClientRect();
|
||||
return rect.top >= containerRect.top && rect.bottom <= containerRect.bottom;
|
||||
}
|
||||
|
||||
/**
|
||||
* Pick an anchor for the current scroll position.
|
||||
*
|
||||
@@ -210,6 +216,12 @@ export function useAnchoredScroll({
|
||||
const programmaticScrollRafRef = React.useRef<number | null>(null);
|
||||
const targetSettleRafRef = React.useRef<number | null>(null);
|
||||
|
||||
const cancelMountBottomPin = React.useCallback(() => {
|
||||
if (mountPinRafIdRef.current === null) return;
|
||||
cancelAnimationFrame(mountPinRafIdRef.current);
|
||||
mountPinRafIdRef.current = null;
|
||||
}, []);
|
||||
|
||||
// Reset everything when the channel changes — the layout effect that runs
|
||||
// immediately after this reset is responsible for either jumping to bottom
|
||||
// or to the target message for the new channel.
|
||||
@@ -439,48 +451,37 @@ export function useAnchoredScroll({
|
||||
`[data-message-id="${messageId}"]`,
|
||||
);
|
||||
if (virtualizerOwnsPrependAnchoring && virtualScrollToMessage) {
|
||||
// Target navigation owns the viewport before any movement strategy is
|
||||
// chosen. The already-mounted fast path centers the DOM node directly
|
||||
// and would otherwise leave durable bottom intent armed.
|
||||
virtualCancelBottomIntent?.();
|
||||
if (el) {
|
||||
const rect = el.getBoundingClientRect();
|
||||
const containerRect = container.getBoundingClientRect();
|
||||
const isInViewport =
|
||||
rect.top >= containerRect.top &&
|
||||
rect.bottom <= containerRect.bottom;
|
||||
if (!isInViewport) {
|
||||
if (!virtualScrollToMessage(messageId, { behavior: "auto" })) {
|
||||
return false;
|
||||
}
|
||||
anchorRef.current = { kind: "message", messageId, topOffset: 0 };
|
||||
setIsAtBottom(false);
|
||||
return false;
|
||||
}
|
||||
const centeredTop = (container.clientHeight - rect.height) / 2;
|
||||
container.scrollTo({
|
||||
top: Math.max(
|
||||
0,
|
||||
container.scrollTop +
|
||||
(rect.top - containerRect.top) -
|
||||
centeredTop,
|
||||
),
|
||||
behavior: options.behavior ?? "auto",
|
||||
});
|
||||
} else if (
|
||||
// Virtua owns the viewport, so every target goes through its index
|
||||
// scroll. A direct DOM scroll would not supersede a bottom pin Virtua
|
||||
// still has in flight from opening the channel, and that pin would
|
||||
// carry the target back off screen.
|
||||
if (
|
||||
!virtualScrollToMessage(messageId, {
|
||||
behavior: options.behavior ?? "auto",
|
||||
// Smooth scrolling across unmeasured rows lands short of the
|
||||
// target, so only an already-visible row animates.
|
||||
behavior:
|
||||
el && isInViewport(el, container)
|
||||
? (options.behavior ?? "auto")
|
||||
: "auto",
|
||||
})
|
||||
) {
|
||||
// A target that is not loaded yet leaves the bottom pin alone.
|
||||
return false;
|
||||
}
|
||||
// An accepted target supersedes every pending bottom pin.
|
||||
virtualCancelBottomIntent?.();
|
||||
cancelMountBottomPin();
|
||||
anchorRef.current = { kind: "message", messageId, topOffset: 0 };
|
||||
virtualizerAtBottomRef.current = false;
|
||||
setIsAtBottom(false);
|
||||
if (el && options.highlight) highlightMessage(messageId);
|
||||
return el !== null;
|
||||
// The highlight is keyed by id, so a row Virtua mounts after this
|
||||
// scroll still renders it.
|
||||
if (options.highlight) highlightMessage(messageId);
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!el) return false;
|
||||
cancelMountBottomPin();
|
||||
|
||||
const rect = el.getBoundingClientRect();
|
||||
const containerRect = container.getBoundingClientRect();
|
||||
@@ -535,6 +536,7 @@ export function useAnchoredScroll({
|
||||
return true;
|
||||
},
|
||||
[
|
||||
cancelMountBottomPin,
|
||||
highlightMessage,
|
||||
pinTargetCentered,
|
||||
scrollContainerRef,
|
||||
@@ -893,32 +895,17 @@ export function useAnchoredScroll({
|
||||
if (!hasInitializedRef.current) return; // initial-mount path will handle.
|
||||
|
||||
void virtualizerRenderVersion;
|
||||
const container = scrollContainerRef.current;
|
||||
if (!container) return;
|
||||
const el = container.querySelector<HTMLElement>(
|
||||
`[data-message-id="${targetMessageId}"]`,
|
||||
);
|
||||
if (!el && virtualizerOwnsPrependAnchoring) {
|
||||
if (
|
||||
scrollToMessageImperative(targetMessageId, {
|
||||
highlight: highlightTargetMessage,
|
||||
})
|
||||
) {
|
||||
handledTargetIdRef.current = targetMessageId;
|
||||
onTargetReached?.(targetMessageId);
|
||||
}
|
||||
return;
|
||||
}
|
||||
if (!el) {
|
||||
// Row not in the DOM yet. A cold deep-link target is fetched by id and
|
||||
// spliced into `messages` a render or two later; this effect re-runs on
|
||||
// each `messages` commit and retries until the row exists.
|
||||
if (
|
||||
!scrollToMessageImperative(targetMessageId, {
|
||||
highlight: highlightTargetMessage,
|
||||
})
|
||||
) {
|
||||
// Not loaded yet. A cold deep-link target is fetched by id and spliced
|
||||
// into `messages` a render or two later; this effect re-runs on each
|
||||
// `messages` commit and retries until the row can be reached.
|
||||
return;
|
||||
}
|
||||
handledTargetIdRef.current = targetMessageId;
|
||||
scrollToMessageImperative(targetMessageId, {
|
||||
highlight: highlightTargetMessage,
|
||||
});
|
||||
onTargetReached?.(targetMessageId);
|
||||
}, [
|
||||
highlightTargetMessage,
|
||||
@@ -926,10 +913,8 @@ export function useAnchoredScroll({
|
||||
messages,
|
||||
onTargetReached,
|
||||
releasePinnedCenter,
|
||||
scrollContainerRef,
|
||||
scrollToMessageImperative,
|
||||
targetMessageId,
|
||||
virtualizerOwnsPrependAnchoring,
|
||||
virtualizerRenderVersion,
|
||||
]);
|
||||
|
||||
|
||||
181
desktop/tests/e2e/message-target-highlight.spec.ts
Normal file
181
desktop/tests/e2e/message-target-highlight.spec.ts
Normal file
@@ -0,0 +1,181 @@
|
||||
import { expect, type Page, test } from "@playwright/test";
|
||||
|
||||
import { installMockBridge } from "../helpers/bridge";
|
||||
|
||||
// A message target (deep link, link click, jump) must flash wherever the row
|
||||
// starts relative to the viewport. The virtualized timeline scrolls by
|
||||
// different strategies for each starting position; the flash must not depend
|
||||
// on which one ran.
|
||||
|
||||
const HIGHLIGHT_CLASS = "route-target-highlight-fade";
|
||||
|
||||
type TargetCase = {
|
||||
name: string;
|
||||
/** Navigate from another channel, as a link click from elsewhere does. */
|
||||
fromOtherChannel: boolean;
|
||||
prependCount: number;
|
||||
pick: (ids: { mounted: string[]; prepended: string[] }) => string;
|
||||
};
|
||||
|
||||
const CASES: TargetCase[] = [
|
||||
{
|
||||
name: "same channel, row on screen",
|
||||
fromOtherChannel: false,
|
||||
prependCount: 50,
|
||||
pick: ({ mounted }) => mounted[mounted.length - 3],
|
||||
},
|
||||
{
|
||||
name: "same channel, row loaded off screen",
|
||||
fromOtherChannel: false,
|
||||
prependCount: 50,
|
||||
pick: ({ mounted }) => mounted[2],
|
||||
},
|
||||
{
|
||||
name: "same channel, row in unloaded older history",
|
||||
fromOtherChannel: false,
|
||||
prependCount: 600,
|
||||
pick: ({ prepended }) => prepended[Math.floor(prepended.length / 8)],
|
||||
},
|
||||
{
|
||||
name: "other channel, row near the bottom",
|
||||
fromOtherChannel: true,
|
||||
prependCount: 50,
|
||||
pick: ({ mounted }) => mounted[mounted.length - 3],
|
||||
},
|
||||
{
|
||||
name: "other channel, row further up",
|
||||
fromOtherChannel: true,
|
||||
prependCount: 50,
|
||||
pick: ({ mounted }) => mounted[2],
|
||||
},
|
||||
];
|
||||
|
||||
async function seedGeneral(page: Page, prependCount: number) {
|
||||
await installMockBridge(page);
|
||||
await page.goto("/");
|
||||
await page.waitForFunction(
|
||||
() =>
|
||||
typeof window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__ === "function" &&
|
||||
typeof window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__ === "function",
|
||||
);
|
||||
return page.evaluate((count) => {
|
||||
for (let index = 0; index < 40; index += 1) {
|
||||
window.__BUZZ_E2E_EMIT_MOCK_MESSAGE__?.({
|
||||
channelName: "general",
|
||||
content: `visible current ${index}\nsecond line ${index}`,
|
||||
createdAt: 1_700_000_000 + index,
|
||||
});
|
||||
}
|
||||
const events = window.__BUZZ_E2E_PREPEND_MOCK_HISTORY__?.({
|
||||
channelName: "general",
|
||||
count,
|
||||
lineCount: 3,
|
||||
});
|
||||
return (events ?? []).map((event) => event.id);
|
||||
}, prependCount);
|
||||
}
|
||||
|
||||
for (const targetCase of CASES) {
|
||||
test(`message target flashes: ${targetCase.name}`, async ({ page }) => {
|
||||
const prepended = await seedGeneral(page, targetCase.prependCount);
|
||||
|
||||
await page.getByTestId("channel-general").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("general");
|
||||
const timeline = page.getByTestId("message-timeline");
|
||||
await expect(timeline).toContainText("visible current 39");
|
||||
// The second bottom-pin pass runs one frame after mount; let it land so
|
||||
// "on screen" and "off screen" are measured against the settled view.
|
||||
await page.evaluate(
|
||||
() =>
|
||||
new Promise((resolve) =>
|
||||
requestAnimationFrame(() => requestAnimationFrame(resolve)),
|
||||
),
|
||||
);
|
||||
const mounted = await timeline.evaluate((el) =>
|
||||
[...el.querySelectorAll<HTMLElement>("[data-message-id]")].map(
|
||||
(row) => row.dataset.messageId ?? "",
|
||||
),
|
||||
);
|
||||
const targetId = targetCase.pick({ mounted, prepended });
|
||||
expect(targetId).toBeTruthy();
|
||||
const generalPath = await page.evaluate(
|
||||
() => (window.location.hash.replace(/^#/, "") || "/").split("?")[0],
|
||||
);
|
||||
|
||||
if (targetCase.fromOtherChannel) {
|
||||
await page.getByTestId("channel-random").click();
|
||||
await expect(page.getByTestId("chat-title")).toHaveText("random");
|
||||
}
|
||||
|
||||
// The app uses hash history, so the target lives in the hash fragment.
|
||||
// Watch from the same task that navigates so a short flash is not missed.
|
||||
const flashedOnScreen = await page.evaluate(
|
||||
([targetId, generalPath, highlightClass]) =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
const deadline = performance.now() + 3_000;
|
||||
const tick = () => {
|
||||
const timelineEl = document.querySelector(
|
||||
'[data-testid="message-timeline"]',
|
||||
);
|
||||
const row = timelineEl?.querySelector<HTMLElement>(
|
||||
`[data-message-id="${CSS.escape(targetId)}"]`,
|
||||
);
|
||||
const flashing =
|
||||
row?.closest(`[class*="${highlightClass}"]`) ??
|
||||
row?.querySelector(`[class*="${highlightClass}"]`);
|
||||
if (timelineEl && row && flashing) {
|
||||
const viewport = timelineEl.getBoundingClientRect();
|
||||
const rect = row.getBoundingClientRect();
|
||||
if (rect.bottom > viewport.top && rect.top < viewport.bottom) {
|
||||
resolve(true);
|
||||
return;
|
||||
}
|
||||
}
|
||||
if (performance.now() > deadline) {
|
||||
resolve(false);
|
||||
return;
|
||||
}
|
||||
requestAnimationFrame(tick);
|
||||
};
|
||||
const params = new URLSearchParams({ messageId: targetId });
|
||||
window.history.pushState(
|
||||
{},
|
||||
"",
|
||||
`${window.location.pathname}#${generalPath}?${params.toString()}`,
|
||||
);
|
||||
window.dispatchEvent(new HashChangeEvent("hashchange"));
|
||||
window.dispatchEvent(new PopStateEvent("popstate"));
|
||||
tick();
|
||||
}),
|
||||
[targetId, generalPath, HIGHLIGHT_CLASS] as const,
|
||||
);
|
||||
|
||||
expect(flashedOnScreen).toBe(true);
|
||||
|
||||
// The target must still be on screen after the flash ends. Late layout
|
||||
// passes (the thread panel opening, history loads, bottom settles) must not
|
||||
// carry it away.
|
||||
const stillOnScreen = await page.evaluate(
|
||||
(targetId) =>
|
||||
new Promise<boolean>((resolve) => {
|
||||
setTimeout(() => {
|
||||
const timelineEl = document.querySelector(
|
||||
'[data-testid="message-timeline"]',
|
||||
);
|
||||
const row = timelineEl?.querySelector<HTMLElement>(
|
||||
`[data-message-id="${CSS.escape(targetId)}"]`,
|
||||
);
|
||||
if (!timelineEl || !row) {
|
||||
resolve(false);
|
||||
return;
|
||||
}
|
||||
const viewport = timelineEl.getBoundingClientRect();
|
||||
const rect = row.getBoundingClientRect();
|
||||
resolve(rect.bottom > viewport.top && rect.top < viewport.bottom);
|
||||
}, 2_200);
|
||||
}),
|
||||
targetId,
|
||||
);
|
||||
expect(stillOnScreen).toBe(true);
|
||||
});
|
||||
}
|
||||
Reference in New Issue
Block a user