Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions desktop/src/features/channels/ui/ChannelPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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,
Expand Down Expand Up @@ -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}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import test from "node:test";
import {
findTopVisibleThreadMessageId,
getResolvedThreadTargets,
getScopedLayoutScrollTargetId,
} from "./useThreadViewModeSwitch.ts";

function row(id, top, bottom) {
Expand Down Expand Up @@ -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(
Expand Down
73 changes: 60 additions & 13 deletions desktop/src/features/channels/ui/useThreadViewModeSwitch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,21 +31,49 @@ 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;
};

/** 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) => {
Expand All @@ -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(() => {
Expand All @@ -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,
Expand Down
4 changes: 4 additions & 0 deletions desktop/src/features/messages/ui/MessageThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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: (
Expand Down Expand Up @@ -207,6 +208,7 @@ export function MessageThreadPanel({
onMarkRead,
onExpandReplies,
onScrollTargetResolved,
onScrollTargetSettled,
onSelectReplyTarget,
onSend,
onToggleReaction,
Expand Down Expand Up @@ -488,6 +490,8 @@ export function MessageThreadPanel({
messages: threadMessages,
highlightTargetMessage: scrollTargetHighlights,
onTargetReached: onScrollTargetResolved,
onTargetSettled: onScrollTargetSettled,
pinTargetCentered: !scrollTargetHighlights,
scrollContainerRef: threadBodyRef,
targetMessageId: scrollTargetId,
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ function makePinnedCenterNodes() {
listener(event);
},
getBoundingClientRect() {
return { top: 0 };
return { bottom: this.clientHeight, top: 0 };
},
querySelector() {
return row;
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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 {
Expand Down
Loading
Loading