mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): preserve thread anchor through layout reflow (#3212)
## Summary - keep a thread presentation-switch anchor pinned while focus/split width reflow settles - retire the temporary anchor only after resize correction and a following paint confirm the row is visible - preserve the existing external-target resolution behavior and viewport E2E contract ## Root cause The focus and split wrappers intentionally retain the same thread surface, but switching wrappers also changes the message column width. `useAnchoredScroll` centered the captured message once and immediately cleared the one-shot layout target. A later text reflow could then move that message outside the viewport with no remaining target to correct it. ## Verification - `pnpm check` - `pnpm typecheck` - `pnpm test` — 3,699 passed - `pnpm build:e2e` - `pnpm exec playwright test tests/e2e/thread-focus-mode.spec.ts --project=smoke --repeat-each=10` — 20 passed - push hook: branch-skew, Desktop check, and Desktop full unit suite passed --------- Signed-off-by: Wes <wesbillman@users.noreply.github.com> Co-authored-by: Carl <c7ebe626f000404285d3686e1dc74cc07cc60a9754a150041ba132e14bd3e2ec@buzz.block.builderlab.xyz>
This commit is contained in:
@@ -514,8 +514,6 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
const isOverlay = useIsThreadPanelOverlay();
|
||||
const useSplitAuxiliaryPane = !isSinglePanelView && !isOverlay;
|
||||
const threadViewMode = useThreadViewMode();
|
||||
// Focus mode only replaces the wide split thread pane; narrow threads and
|
||||
// other auxiliary panels keep their existing presentation.
|
||||
const useFocusThreadDrawer =
|
||||
threadViewMode === "focus" &&
|
||||
useSplitAuxiliaryPane &&
|
||||
@@ -526,6 +524,7 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
);
|
||||
const { changeThreadViewMode, layoutScrollTargetId, resolveScrollTarget } =
|
||||
useThreadViewModeSwitch({
|
||||
activeThreadHeadId: threadHeadMessage?.id ?? null,
|
||||
externalScrollTargetId: threadScrollTargetId,
|
||||
onExternalTargetResolved: onThreadScrollTargetResolved,
|
||||
onModeChange: markExitComplete,
|
||||
@@ -881,7 +880,8 @@ export const ChannelPane = React.memo(function ChannelPane({
|
||||
onExpandReplies={onExpandThreadReplies}
|
||||
onSelectReplyTarget={onSelectThreadReplyTarget}
|
||||
onSend={onSendThreadReply}
|
||||
onScrollTargetResolved={resolveScrollTarget}
|
||||
onScrollTargetResolved={() => resolveScrollTarget()}
|
||||
onScrollTargetSettled={resolveScrollTarget}
|
||||
onToggleReaction={onToggleReaction}
|
||||
onUnfollowThread={onUnfollowThread}
|
||||
profiles={profiles}
|
||||
|
||||
@@ -4,6 +4,7 @@ import test from "node:test";
|
||||
import {
|
||||
findTopVisibleThreadMessageId,
|
||||
getResolvedThreadTargets,
|
||||
getScopedLayoutScrollTargetId,
|
||||
} from "./useThreadViewModeSwitch.ts";
|
||||
|
||||
function row(id, top, bottom) {
|
||||
@@ -44,6 +45,55 @@ test("resolves both sources when a layout anchor matches the external target", (
|
||||
);
|
||||
});
|
||||
|
||||
test("does not resolve a layout target that was never captured", () => {
|
||||
assert.deepEqual(
|
||||
getResolvedThreadTargets({
|
||||
externalTargetId: "reply-b",
|
||||
layoutTargetId: null,
|
||||
}),
|
||||
{ resolveExternal: true, resolveLayout: false },
|
||||
);
|
||||
assert.deepEqual(
|
||||
getResolvedThreadTargets({
|
||||
externalTargetId: null,
|
||||
layoutTargetId: null,
|
||||
}),
|
||||
{ resolveExternal: true, resolveLayout: false },
|
||||
);
|
||||
});
|
||||
|
||||
test("drops a captured layout target when the active thread closes or changes", () => {
|
||||
const captured = { messageId: "reply-a", threadHeadId: "thread-a" };
|
||||
|
||||
assert.equal(
|
||||
getScopedLayoutScrollTargetId({
|
||||
activeThreadHeadId: "thread-a",
|
||||
layoutTarget: captured,
|
||||
}),
|
||||
"reply-a",
|
||||
);
|
||||
assert.equal(
|
||||
getScopedLayoutScrollTargetId({
|
||||
activeThreadHeadId: null,
|
||||
layoutTarget: captured,
|
||||
}),
|
||||
null,
|
||||
);
|
||||
const replacementLayoutTargetId = getScopedLayoutScrollTargetId({
|
||||
activeThreadHeadId: "thread-b",
|
||||
layoutTarget: captured,
|
||||
});
|
||||
assert.equal(replacementLayoutTargetId, null);
|
||||
assert.deepEqual(
|
||||
getResolvedThreadTargets({
|
||||
externalTargetId: "reply-b",
|
||||
layoutTargetId: replacementLayoutTargetId,
|
||||
}),
|
||||
{ resolveExternal: true, resolveLayout: false },
|
||||
"the stale anchor does not mask the replacement thread target",
|
||||
);
|
||||
});
|
||||
|
||||
test("returns null without a mounted thread body or visible message", () => {
|
||||
assert.equal(findTopVisibleThreadMessageId(null), null);
|
||||
assert.equal(
|
||||
|
||||
@@ -31,7 +31,25 @@ export function getResolvedThreadTargets({
|
||||
};
|
||||
}
|
||||
|
||||
type LayoutScrollTarget = {
|
||||
messageId: string;
|
||||
threadHeadId: string;
|
||||
};
|
||||
|
||||
export function getScopedLayoutScrollTargetId({
|
||||
activeThreadHeadId,
|
||||
layoutTarget,
|
||||
}: {
|
||||
activeThreadHeadId: string | null;
|
||||
layoutTarget: LayoutScrollTarget | null;
|
||||
}): string | null {
|
||||
return layoutTarget?.threadHeadId === activeThreadHeadId
|
||||
? layoutTarget.messageId
|
||||
: null;
|
||||
}
|
||||
|
||||
type ThreadViewModeSwitchOptions = {
|
||||
activeThreadHeadId: string | null;
|
||||
externalScrollTargetId: string | null;
|
||||
onExternalTargetResolved: () => void;
|
||||
onModeChange?: (mode: ThreadViewMode) => void;
|
||||
@@ -39,13 +57,23 @@ type ThreadViewModeSwitchOptions = {
|
||||
|
||||
/** Preserves the reply being read while the thread changes presentation. */
|
||||
export function useThreadViewModeSwitch({
|
||||
activeThreadHeadId,
|
||||
externalScrollTargetId,
|
||||
onExternalTargetResolved,
|
||||
onModeChange,
|
||||
}: ThreadViewModeSwitchOptions) {
|
||||
const [layoutScrollTargetId, setLayoutScrollTargetId] = React.useState<
|
||||
string | null
|
||||
>(null);
|
||||
const [layoutScrollTarget, setLayoutScrollTarget] =
|
||||
React.useState<LayoutScrollTarget | null>(null);
|
||||
const layoutScrollTargetId = getScopedLayoutScrollTargetId({
|
||||
activeThreadHeadId,
|
||||
layoutTarget: layoutScrollTarget,
|
||||
});
|
||||
|
||||
React.useEffect(() => {
|
||||
setLayoutScrollTarget((current) =>
|
||||
current?.threadHeadId === activeThreadHeadId ? current : null,
|
||||
);
|
||||
}, [activeThreadHeadId]);
|
||||
|
||||
const changeThreadViewMode = React.useCallback(
|
||||
(mode: ThreadViewMode, restoreFocus: boolean) => {
|
||||
@@ -54,7 +82,11 @@ export function useThreadViewModeSwitch({
|
||||
);
|
||||
const anchorId = findTopVisibleThreadMessageId(body);
|
||||
|
||||
setLayoutScrollTargetId(anchorId);
|
||||
setLayoutScrollTarget(
|
||||
anchorId && activeThreadHeadId
|
||||
? { messageId: anchorId, threadHeadId: activeThreadHeadId }
|
||||
: null,
|
||||
);
|
||||
onModeChange?.(mode);
|
||||
setThreadViewMode(mode);
|
||||
requestAnimationFrame(() => {
|
||||
@@ -69,17 +101,32 @@ export function useThreadViewModeSwitch({
|
||||
});
|
||||
});
|
||||
},
|
||||
[onModeChange],
|
||||
[activeThreadHeadId, onModeChange],
|
||||
);
|
||||
|
||||
const resolveScrollTarget = React.useCallback(() => {
|
||||
const resolution = getResolvedThreadTargets({
|
||||
externalTargetId: externalScrollTargetId,
|
||||
layoutTargetId: layoutScrollTargetId,
|
||||
});
|
||||
if (resolution.resolveLayout) setLayoutScrollTargetId(null);
|
||||
if (resolution.resolveExternal) onExternalTargetResolved();
|
||||
}, [externalScrollTargetId, layoutScrollTargetId, onExternalTargetResolved]);
|
||||
const resolveScrollTarget = React.useCallback(
|
||||
(settledMessageId?: string) => {
|
||||
const resolution = getResolvedThreadTargets({
|
||||
externalTargetId: externalScrollTargetId,
|
||||
layoutTargetId: layoutScrollTargetId,
|
||||
});
|
||||
if (resolution.resolveExternal) onExternalTargetResolved();
|
||||
if (settledMessageId) {
|
||||
setLayoutScrollTarget((current) =>
|
||||
current?.threadHeadId === activeThreadHeadId &&
|
||||
current.messageId === settledMessageId
|
||||
? null
|
||||
: current,
|
||||
);
|
||||
}
|
||||
},
|
||||
[
|
||||
activeThreadHeadId,
|
||||
externalScrollTargetId,
|
||||
layoutScrollTargetId,
|
||||
onExternalTargetResolved,
|
||||
],
|
||||
);
|
||||
|
||||
return {
|
||||
changeThreadViewMode,
|
||||
|
||||
@@ -79,6 +79,7 @@ type MessageThreadPanelProps = ThreadPanelLayoutProps & {
|
||||
onMarkRead?: (message: TimelineMessage) => void;
|
||||
onExpandReplies: (message: TimelineMessage) => void;
|
||||
onScrollTargetResolved: () => void;
|
||||
onScrollTargetSettled?: (messageId: string) => void;
|
||||
scrollTargetHighlights?: boolean;
|
||||
onSelectReplyTarget: (message: TimelineMessage) => void;
|
||||
onSend: (
|
||||
@@ -207,6 +208,7 @@ export function MessageThreadPanel({
|
||||
onMarkRead,
|
||||
onExpandReplies,
|
||||
onScrollTargetResolved,
|
||||
onScrollTargetSettled,
|
||||
onSelectReplyTarget,
|
||||
onSend,
|
||||
onToggleReaction,
|
||||
@@ -488,6 +490,8 @@ export function MessageThreadPanel({
|
||||
messages: threadMessages,
|
||||
highlightTargetMessage: scrollTargetHighlights,
|
||||
onTargetReached: onScrollTargetResolved,
|
||||
onTargetSettled: onScrollTargetSettled,
|
||||
pinTargetCentered: !scrollTargetHighlights,
|
||||
scrollContainerRef: threadBodyRef,
|
||||
targetMessageId: scrollTargetId,
|
||||
});
|
||||
|
||||
@@ -173,7 +173,7 @@ function makePinnedCenterNodes() {
|
||||
listener(event);
|
||||
},
|
||||
getBoundingClientRect() {
|
||||
return { top: 0 };
|
||||
return { bottom: this.clientHeight, top: 0 };
|
||||
},
|
||||
querySelector() {
|
||||
return row;
|
||||
@@ -232,12 +232,13 @@ function makePinnedCenterNodes() {
|
||||
};
|
||||
}
|
||||
|
||||
function Harness({ channelId, refs }) {
|
||||
function Harness({ channelId, onTargetSettled, refs }) {
|
||||
useAnchoredScroll({
|
||||
channelId,
|
||||
contentRef: refs.content,
|
||||
isLoading: false,
|
||||
messages: [{ id: "selected" }],
|
||||
onTargetSettled,
|
||||
pinTargetCentered: true,
|
||||
scrollContainerRef: refs.container,
|
||||
targetMessageId: "selected",
|
||||
@@ -312,6 +313,68 @@ test("channel change attaches pinned-center observers after refs mount", async (
|
||||
});
|
||||
});
|
||||
|
||||
test("pinned target settles only after resize correction and a paint frame", async () => {
|
||||
const refs = {
|
||||
container: { current: null },
|
||||
content: { current: null },
|
||||
};
|
||||
const root = createRoot(document.createElement("div"));
|
||||
const settled = [];
|
||||
const nodes = makePinnedCenterNodes();
|
||||
refs.container.current = nodes.container;
|
||||
refs.content.current = nodes.content;
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(Harness, {
|
||||
channelId: "conversation",
|
||||
onTargetSettled: (messageId) => settled.push(messageId),
|
||||
refs,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
assert.deepEqual(settled, [], "initial target reach is not settled");
|
||||
nodes.moveSelectedRowBy(96);
|
||||
nodes.resizeObservers[0].callback();
|
||||
assert.deepEqual(settled, [], "resize callback waits for the paint frame");
|
||||
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
|
||||
|
||||
assert.deepEqual(settled, ["selected"]);
|
||||
assert.deepEqual(nodes.container.scrollWrites, [96]);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
test("user interaction releases and retires a pending pinned target", async () => {
|
||||
const refs = {
|
||||
container: { current: null },
|
||||
content: { current: null },
|
||||
};
|
||||
const root = createRoot(document.createElement("div"));
|
||||
const settled = [];
|
||||
const nodes = makePinnedCenterNodes();
|
||||
refs.container.current = nodes.container;
|
||||
refs.content.current = nodes.content;
|
||||
|
||||
await act(async () => {
|
||||
root.render(
|
||||
React.createElement(Harness, {
|
||||
channelId: "conversation",
|
||||
onTargetSettled: (messageId) => settled.push(messageId),
|
||||
refs,
|
||||
}),
|
||||
);
|
||||
});
|
||||
await act(async () => nodes.container.dispatchEvent({ type: "wheel" }));
|
||||
nodes.moveSelectedRowBy(96);
|
||||
nodes.resizeObservers[0].callback();
|
||||
await act(async () => new Promise((resolve) => setTimeout(resolve, 0)));
|
||||
|
||||
assert.deepEqual(settled, ["selected"]);
|
||||
assert.deepEqual(nodes.container.scrollWrites, []);
|
||||
await act(async () => root.unmount());
|
||||
});
|
||||
|
||||
test("mounted virtual target retires bottom intent before direct centering", async () => {
|
||||
const resizeObservers = [];
|
||||
globalThis.ResizeObserver = class {
|
||||
|
||||
@@ -109,6 +109,8 @@ type UseAnchoredScrollOptions = {
|
||||
/** Keeps a targeted message centered until the user deliberately scrolls. */
|
||||
pinTargetCentered?: boolean;
|
||||
onTargetReached?: (messageId: string) => void;
|
||||
/** Reports a pinned target after resize correction and one paint frame. */
|
||||
onTargetSettled?: (messageId: string) => void;
|
||||
virtualCancelBottomIntent?: () => void;
|
||||
virtualScrollToMessage?: (
|
||||
messageId: string,
|
||||
@@ -230,6 +232,7 @@ export function useAnchoredScroll({
|
||||
highlightTargetMessage = true,
|
||||
pinTargetCentered = false,
|
||||
onTargetReached,
|
||||
onTargetSettled,
|
||||
virtualCancelBottomIntent,
|
||||
virtualScrollToMessage,
|
||||
virtualScrollToBottom,
|
||||
@@ -278,6 +281,7 @@ export function useAnchoredScroll({
|
||||
const programmaticScrollTopRef = React.useRef<number | null>(null);
|
||||
const isWritingScrollRef = React.useRef(false);
|
||||
const programmaticScrollRafRef = React.useRef<number | null>(null);
|
||||
const targetSettleRafRef = React.useRef<number | null>(null);
|
||||
|
||||
// Reset everything when the channel changes — the layout effect that runs
|
||||
// immediately after this reset is responsible for either jumping to bottom
|
||||
@@ -303,6 +307,10 @@ export function useAnchoredScroll({
|
||||
cancelAnimationFrame(programmaticScrollRafRef.current);
|
||||
programmaticScrollRafRef.current = null;
|
||||
}
|
||||
if (targetSettleRafRef.current !== null) {
|
||||
cancelAnimationFrame(targetSettleRafRef.current);
|
||||
targetSettleRafRef.current = null;
|
||||
}
|
||||
if (highlightTimeoutRef.current !== null) {
|
||||
window.clearTimeout(highlightTimeoutRef.current);
|
||||
highlightTimeoutRef.current = null;
|
||||
@@ -382,6 +390,40 @@ export function useAnchoredScroll({
|
||||
if (atBottom) setNewMessageCount(0);
|
||||
}, [scrollContainerRef]);
|
||||
|
||||
const schedulePinnedTargetSettle = React.useCallback(
|
||||
(messageId: string) => {
|
||||
if (!onTargetSettled) return;
|
||||
if (targetSettleRafRef.current !== null) {
|
||||
cancelAnimationFrame(targetSettleRafRef.current);
|
||||
}
|
||||
targetSettleRafRef.current = requestAnimationFrame(() => {
|
||||
targetSettleRafRef.current = null;
|
||||
const container = scrollContainerRef.current;
|
||||
const anchor = anchorRef.current;
|
||||
if (
|
||||
!container ||
|
||||
anchor.kind !== "pinned-center" ||
|
||||
anchor.messageId !== messageId
|
||||
) {
|
||||
return;
|
||||
}
|
||||
const row = container.querySelector<HTMLElement>(
|
||||
`[data-message-id="${CSS.escape(messageId)}"]`,
|
||||
);
|
||||
if (!row) return;
|
||||
const rowRect = row.getBoundingClientRect();
|
||||
const containerRect = container.getBoundingClientRect();
|
||||
if (
|
||||
rowRect.bottom > containerRect.top &&
|
||||
rowRect.top < containerRect.bottom
|
||||
) {
|
||||
onTargetSettled(messageId);
|
||||
}
|
||||
});
|
||||
},
|
||||
[onTargetSettled, scrollContainerRef],
|
||||
);
|
||||
|
||||
const scrollToBottomImperative = React.useCallback(
|
||||
(behavior: ScrollBehavior = "auto") => {
|
||||
const container = scrollContainerRef.current;
|
||||
@@ -752,11 +794,11 @@ export function useAnchoredScroll({
|
||||
isLoading,
|
||||
messages,
|
||||
onTargetReached,
|
||||
repinPinnedCenter,
|
||||
scrollContainerRef,
|
||||
scrollToBottomImperative,
|
||||
scrollToMessageImperative,
|
||||
targetMessageId,
|
||||
repinPinnedCenter,
|
||||
virtualScrollToBottom,
|
||||
virtualSettleAtBottom,
|
||||
virtualizerOwnsPrependAnchoring,
|
||||
@@ -779,6 +821,7 @@ export function useAnchoredScroll({
|
||||
if (!container) return;
|
||||
if (anchorRef.current.kind === "pinned-center") {
|
||||
repinPinnedCenter();
|
||||
schedulePinnedTargetSettle(anchorRef.current.messageId);
|
||||
} else if (
|
||||
anchorRef.current.kind === "at-bottom" &&
|
||||
!virtualizerOwnsPrependAnchoring
|
||||
@@ -787,24 +830,43 @@ export function useAnchoredScroll({
|
||||
}
|
||||
});
|
||||
observer.observe(content);
|
||||
return () => observer.disconnect();
|
||||
return () => {
|
||||
observer.disconnect();
|
||||
if (targetSettleRafRef.current !== null) {
|
||||
cancelAnimationFrame(targetSettleRafRef.current);
|
||||
targetSettleRafRef.current = null;
|
||||
}
|
||||
};
|
||||
}, [
|
||||
channelId,
|
||||
contentRef,
|
||||
repinPinnedCenter,
|
||||
schedulePinnedTargetSettle,
|
||||
scrollContainerRef,
|
||||
virtualizerOwnsPrependAnchoring,
|
||||
]);
|
||||
|
||||
// Pinned centers survive our own corrections but release as soon as the
|
||||
// reader deliberately takes control of the scroll position.
|
||||
// reader deliberately takes control of the scroll position or the caller
|
||||
// retires the temporary target after layout settlement.
|
||||
React.useEffect(() => {
|
||||
if (!pinTargetCentered) releasePinnedCenter();
|
||||
}, [pinTargetCentered, releasePinnedCenter]);
|
||||
|
||||
// biome-ignore lint/correctness/useExhaustiveDependencies: channelId deliberately re-subscribes after a keyed or conditional scroll-container mount replaces ref.current.
|
||||
React.useEffect(() => {
|
||||
if (!pinTargetCentered) return;
|
||||
const container = scrollContainerRef.current;
|
||||
if (!container) return;
|
||||
|
||||
const handleUserInteraction = () => releasePinnedCenter();
|
||||
const handleUserInteraction = () => {
|
||||
const pinnedMessageId =
|
||||
anchorRef.current.kind === "pinned-center"
|
||||
? anchorRef.current.messageId
|
||||
: null;
|
||||
releasePinnedCenter();
|
||||
if (pinnedMessageId) onTargetSettled?.(pinnedMessageId);
|
||||
};
|
||||
container.addEventListener("wheel", handleUserInteraction, {
|
||||
passive: true,
|
||||
});
|
||||
@@ -817,7 +879,13 @@ export function useAnchoredScroll({
|
||||
container.removeEventListener("touchstart", handleUserInteraction);
|
||||
container.removeEventListener("keydown", handleUserInteraction);
|
||||
};
|
||||
}, [channelId, pinTargetCentered, releasePinnedCenter, scrollContainerRef]);
|
||||
}, [
|
||||
channelId,
|
||||
onTargetSettled,
|
||||
pinTargetCentered,
|
||||
releasePinnedCenter,
|
||||
scrollContainerRef,
|
||||
]);
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Target message handling (deep link, jump-to-reply, etc.). Distinct from
|
||||
@@ -896,6 +964,9 @@ export function useAnchoredScroll({
|
||||
if (programmaticScrollRafRef.current !== null) {
|
||||
cancelAnimationFrame(programmaticScrollRafRef.current);
|
||||
}
|
||||
if (targetSettleRafRef.current !== null) {
|
||||
cancelAnimationFrame(targetSettleRafRef.current);
|
||||
}
|
||||
};
|
||||
}, []);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user