Studio: keep chat in place when composer attachments resize it (#6070)
* Studio: keep chat in place when composer attachments resize it Attaching or removing a file in the chat composer could yank the whole conversation to the bottom, and the grown composer covered the end of the chat with no way to scroll it back into view. Root cause: the Viewport composes refs with an identity that changes on re-render, so React re-runs our scroll ref on unrelated renders and the autoscroll hook treated every rebind as a fresh mount, pinning to the bottom. On top of that the viewport reserved a fixed 160px under the last message regardless of composer size. - Treat same-element ref rebinds as no-ops in the autoscroll hook; only a genuinely new viewport element pins and resets detach state - Size the bottom spacer from the measured composer height plus a 24px gap so the chat can always be scrolled above the composer - On composer growth, detach from the bottom instead of auto-scrolling; the user scrolls down to reveal the covered lines - On composer shrink, defer the spacer shrink until it cannot clamp scrollTop, then release it invisibly on scroll or on bottom-pinning moments (run start, thread switch, thread load) * Studio: release deferred composer spacer when a run owns the bottom Sending with attachments cleared the chips after thread.runStart had already fired, so the spacer shrink was deferred while the user sat pinned at the bottom, leaving a permanent extra gap above the composer. Apply shrinks immediately while a run is active or within 1s of run start; the run-start pin owns the bottom then, so the clamp is the intended glide. Caught by a cross-engine Playwright pass (Chromium, Firefox, WebKit) over the pre and post builds. * Studio: track the viewport element in state so listeners survive remounts The deferred-shrink scroll listener was attached once against a ref, but the keyed overlay provider remounts the viewport subtree on thread switches, leaving the listener bound to the unmounted element. Removing an attachment near the bottom in the new thread then left the oversized spacer stuck until a run started. Track the viewport element in state so the listener and the clamp math follow the new element. Reproduced and verified with a thread-switch scenario on Chromium, Firefox and WebKit; full matrix re-run green. * Studio: release deferred composer spacer shrink when at the bottom (#6070) --------- Co-authored-by: shimmyshimmer <michael@unsloth.ai> Co-authored-by: Daniel Han <danielhanchen@gmail.com>
This commit is contained in:
Michael Han
GitHub
parent
da1c5b4b94
commit
cf97faed9f
2 changed files with 271 additions and 28 deletions
|
|
@ -119,6 +119,7 @@ import {
|
|||
useCallback,
|
||||
useContext,
|
||||
useEffect,
|
||||
useLayoutEffect,
|
||||
useRef,
|
||||
useState,
|
||||
} from "react";
|
||||
|
|
@ -127,6 +128,18 @@ import {
|
|||
// composer), so the composer can show its "Drop files here" affordance.
|
||||
const PageDragContext = createContext(false);
|
||||
|
||||
// Gap (px) between the last message and the floating composer. The bottom
|
||||
// spacer tracks composer height plus this gap so the chat can always be
|
||||
// scrolled fully above the composer.
|
||||
const COMPOSER_SCROLL_GAP_PX = 24;
|
||||
// The scroll-to-bottom footer sits 10px below the spacer top.
|
||||
const FOOTER_GAP_BELOW_SPACER_PX = 10;
|
||||
// Composer shrinks this soon after a run start (send clears the chips)
|
||||
// apply immediately: the run-start pin owns the bottom, so the clamp is
|
||||
// the intended glide. Covers instant responses where isRunning is
|
||||
// already false by the time the dock resize is observed.
|
||||
const RUN_SHRINK_WINDOW_MS = 1000;
|
||||
|
||||
export const Thread: FC<{
|
||||
hideComposer?: boolean;
|
||||
hideWelcome?: boolean;
|
||||
|
|
@ -144,12 +157,156 @@ export const Thread: FC<{
|
|||
);
|
||||
const activeThreadId = useChatRuntimeStore((s) => s.activeThreadId);
|
||||
const threadId = targetThreadId ?? activeThreadId ?? null;
|
||||
const aui = useAui();
|
||||
|
||||
// Measured height of the floating composer dock (null until measured).
|
||||
// Drives the bottom spacer and the scroll-to-bottom footer offset.
|
||||
const [composerHeight, setComposerHeight] = useState<number | null>(null);
|
||||
const footerBottomPx =
|
||||
composerHeight == null
|
||||
? null
|
||||
: composerHeight + COMPOSER_SCROLL_GAP_PX - FOOTER_GAP_BELOW_SPACER_PX;
|
||||
|
||||
// The viewport element is owned by the autoscroll hook; mirror it
|
||||
// locally for the spacer clamp math below. State, not a ref: the keyed
|
||||
// provider below remounts the viewport on thread switches, and the
|
||||
// scroll listener effect must re-attach to the new element.
|
||||
const [viewportEl, setViewportEl] = useState<HTMLElement | null>(null);
|
||||
const composedViewportRef = useCallback(
|
||||
(node: HTMLElement | null) => {
|
||||
setViewportEl(node);
|
||||
viewportRef(node);
|
||||
},
|
||||
[viewportRef],
|
||||
);
|
||||
|
||||
// Bottom spacer sizing. Invariant: the chat never moves on its own when
|
||||
// the composer resizes.
|
||||
// - Grow (attachment added, multiline input): grow the spacer at once.
|
||||
// Growth below the scroll position is invisible and only adds room.
|
||||
// - Shrink (attachment removed): shrinking scrollHeight near the bottom
|
||||
// would clamp scrollTop and yank the chat down. Defer the shrink until
|
||||
// it is invisible (user scrolled up) or a bottom-pinning moment.
|
||||
// Applied imperatively so a remounted spacer can be sized from refs even
|
||||
// when composerHeight did not change (e.g. thread switch).
|
||||
const spacerElRef = useRef<HTMLDivElement | null>(null);
|
||||
const desiredSpacerPxRef = useRef<number | null>(null);
|
||||
const appliedSpacerPxRef = useRef<number | null>(null);
|
||||
|
||||
const applySpacerPx = useCallback((px: number) => {
|
||||
appliedSpacerPxRef.current = px;
|
||||
const node = spacerElRef.current;
|
||||
if (node) {
|
||||
node.style.height = `${px}px`;
|
||||
}
|
||||
}, []);
|
||||
|
||||
// Release any deferred shrink; used at moments that pin to the bottom
|
||||
// anyway, where the clamp is the intended motion.
|
||||
const releaseSpacerExcess = useCallback(() => {
|
||||
const desired = desiredSpacerPxRef.current;
|
||||
const applied = appliedSpacerPxRef.current;
|
||||
if (desired != null && applied != null && applied > desired) {
|
||||
applySpacerPx(desired);
|
||||
}
|
||||
}, [applySpacerPx]);
|
||||
|
||||
const spacerRef = useCallback(
|
||||
(node: HTMLDivElement | null) => {
|
||||
spacerElRef.current = node;
|
||||
// Fresh mounts (thread switch, first message) start at the desired
|
||||
// size; deferral state from a previous mount is moot.
|
||||
const desired = desiredSpacerPxRef.current;
|
||||
if (node && desired != null) {
|
||||
applySpacerPx(desired);
|
||||
}
|
||||
},
|
||||
[applySpacerPx],
|
||||
);
|
||||
|
||||
const prevComposerHeightRef = useRef<number | null>(null);
|
||||
// Set on thread.runStart; see RUN_SHRINK_WINDOW_MS.
|
||||
const runStartAtRef = useRef(0);
|
||||
useLayoutEffect(() => {
|
||||
const prev = prevComposerHeightRef.current;
|
||||
prevComposerHeightRef.current = composerHeight;
|
||||
if (composerHeight == null || hideComposer) {
|
||||
desiredSpacerPxRef.current = null;
|
||||
appliedSpacerPxRef.current = null;
|
||||
spacerElRef.current?.style.removeProperty("height");
|
||||
return;
|
||||
}
|
||||
const desired = composerHeight + COMPOSER_SCROLL_GAP_PX;
|
||||
desiredSpacerPxRef.current = desired;
|
||||
const applied = appliedSpacerPxRef.current;
|
||||
if (applied == null || desired >= applied) {
|
||||
applySpacerPx(desired);
|
||||
} else {
|
||||
const distance = viewportEl
|
||||
? viewportEl.scrollHeight - viewportEl.scrollTop - viewportEl.clientHeight
|
||||
: Number.POSITIVE_INFINITY;
|
||||
const runOwnsBottom =
|
||||
aui.thread().getState().isRunning ||
|
||||
performance.now() - runStartAtRef.current < RUN_SHRINK_WINDOW_MS;
|
||||
// At the bottom the shrink only drops blank spacer, so apply it now
|
||||
// instead of stranding dead space until the next pin.
|
||||
if (
|
||||
runOwnsBottom ||
|
||||
distance >= applied - desired ||
|
||||
autoScrollContext.getIsAtBottom()
|
||||
) {
|
||||
applySpacerPx(desired);
|
||||
}
|
||||
// else: deferred; released on scroll or a bottom-pinning event.
|
||||
}
|
||||
if (prev != null && composerHeight > prev) {
|
||||
// The chat is now above the new bottom. Detach as if the user had
|
||||
// scrolled up so no later signal re-pins and shoves the chat up.
|
||||
// Scrolling back down re-attaches; explicit pins still work.
|
||||
// Mid-run growth comes from tool-status rows, not the user, and
|
||||
// detaching then would break streaming autoscroll, so skip it.
|
||||
if (!aui.thread().getState().isRunning) {
|
||||
autoScrollContext.detachFromBottom();
|
||||
}
|
||||
}
|
||||
}, [composerHeight, hideComposer, autoScrollContext, aui, applySpacerPx, viewportEl]);
|
||||
|
||||
// Drop deferred spacer excess as soon as the user has scrolled far
|
||||
// enough above the bottom that the shrink cannot clamp scrollTop.
|
||||
// Keyed on viewportEl so the listener follows viewport remounts.
|
||||
useEffect(() => {
|
||||
const el = viewportEl;
|
||||
if (!el) {
|
||||
return;
|
||||
}
|
||||
const onScroll = () => {
|
||||
const desired = desiredSpacerPxRef.current;
|
||||
const applied = appliedSpacerPxRef.current;
|
||||
if (desired == null || applied == null || applied <= desired) {
|
||||
return;
|
||||
}
|
||||
const distance = el.scrollHeight - el.scrollTop - el.clientHeight;
|
||||
if (distance >= applied - desired) {
|
||||
applySpacerPx(desired);
|
||||
}
|
||||
};
|
||||
el.addEventListener("scroll", onScroll, { passive: true });
|
||||
return () => el.removeEventListener("scroll", onScroll);
|
||||
}, [viewportEl, applySpacerPx]);
|
||||
|
||||
// These pin to the bottom, so releasing the excess here is invisible.
|
||||
// runStart also opens the shrink window for the send-clears-chips case.
|
||||
useAuiEvent("thread.runStart", () => {
|
||||
runStartAtRef.current = performance.now();
|
||||
releaseSpacerExcess();
|
||||
});
|
||||
useAuiEvent("thread.initialize", releaseSpacerExcess);
|
||||
useAuiEvent("threadListItem.switchedTo", releaseSpacerExcess);
|
||||
|
||||
// Page-wide drag-and-drop: dropping a file anywhere on the chat page (not
|
||||
// just on the composer) attaches it and shows the composer drop affordance.
|
||||
// The composer's own dropzone still handles drops on the box itself; its
|
||||
// handler calls preventDefault, so the page handler skips them (no double-add).
|
||||
const aui = useAui();
|
||||
const [pageDragging, setPageDragging] = useState(false);
|
||||
const dragDepth = useRef(0);
|
||||
const hasFiles = (e: ReactDragEvent) =>
|
||||
|
|
@ -208,7 +365,7 @@ export const Thread: FC<{
|
|||
>
|
||||
<IntentAwareScrollProvider value={autoScrollContext}>
|
||||
<ThreadPrimitive.Viewport
|
||||
ref={viewportRef}
|
||||
ref={composedViewportRef}
|
||||
autoScroll={false}
|
||||
scrollToBottomOnRunStart={false}
|
||||
scrollToBottomOnInitialize={false}
|
||||
|
|
@ -240,7 +397,15 @@ export const Thread: FC<{
|
|||
sticky footer and feel cramped. */}
|
||||
<AuiIf condition={({ thread }) => hideWelcome || !thread.isEmpty}>
|
||||
<div
|
||||
className={cn("shrink-0", hideComposer ? "h-16" : "h-40")}
|
||||
ref={spacerRef}
|
||||
className={cn(
|
||||
"shrink-0",
|
||||
hideComposer
|
||||
? "h-16"
|
||||
: composerHeight == null
|
||||
? "h-40"
|
||||
: undefined,
|
||||
)}
|
||||
aria-hidden={true}
|
||||
/>
|
||||
</AuiIf>
|
||||
|
|
@ -250,21 +415,34 @@ export const Thread: FC<{
|
|||
className={cn(
|
||||
"aui-thread-viewport-footer pointer-events-none sticky z-20 flex w-full justify-center bg-transparent",
|
||||
// 150px (was 140px) to add a small gap above the composer
|
||||
hideComposer ? "bottom-3" : "bottom-[150px]",
|
||||
hideComposer
|
||||
? "bottom-3"
|
||||
: footerBottomPx == null
|
||||
? "bottom-[150px]"
|
||||
: undefined,
|
||||
)}
|
||||
style={
|
||||
!hideComposer && footerBottomPx != null
|
||||
? { bottom: footerBottomPx }
|
||||
: undefined
|
||||
}
|
||||
>
|
||||
<ThreadScrollToBottom />
|
||||
</ThreadPrimitive.ViewportFooter>
|
||||
</AuiIf>
|
||||
</ThreadPrimitive.Viewport>
|
||||
|
||||
<GeneratedImageViewportOverlay hideComposer={hideComposer} />
|
||||
<GeneratedImageViewportOverlay
|
||||
hideComposer={hideComposer}
|
||||
bottomOffsetPx={footerBottomPx}
|
||||
/>
|
||||
|
||||
{!hideComposer && (
|
||||
<AuiIf condition={({ thread }) => hideWelcome || !thread.isEmpty}>
|
||||
<ThreadComposerDock
|
||||
disabled={isComposerAttachPending}
|
||||
threadId={threadId}
|
||||
onHeightChange={setComposerHeight}
|
||||
/>
|
||||
</AuiIf>
|
||||
)}
|
||||
|
|
@ -275,9 +453,10 @@ export const Thread: FC<{
|
|||
);
|
||||
};
|
||||
|
||||
const GeneratedImageViewportOverlay: FC<{ hideComposer?: boolean }> = ({
|
||||
hideComposer,
|
||||
}) => {
|
||||
const GeneratedImageViewportOverlay: FC<{
|
||||
hideComposer?: boolean;
|
||||
bottomOffsetPx?: number | null;
|
||||
}> = ({ hideComposer, bottomOffsetPx }) => {
|
||||
const { overlay, closeOverlay } = useGeneratedImageOverlay();
|
||||
|
||||
useEffect(() => {
|
||||
|
|
@ -302,8 +481,17 @@ const GeneratedImageViewportOverlay: FC<{ hideComposer?: boolean }> = ({
|
|||
<section
|
||||
className={cn(
|
||||
"pointer-events-none absolute inset-x-5 top-[48px] flex flex-col items-center",
|
||||
hideComposer ? "bottom-4" : "bottom-[150px]",
|
||||
hideComposer
|
||||
? "bottom-4"
|
||||
: bottomOffsetPx == null
|
||||
? "bottom-[150px]"
|
||||
: undefined,
|
||||
)}
|
||||
style={
|
||||
!hideComposer && bottomOffsetPx != null
|
||||
? { bottom: bottomOffsetPx }
|
||||
: undefined
|
||||
}
|
||||
aria-label="Generated image preview"
|
||||
>
|
||||
<div className="pointer-events-auto relative flex min-h-0 w-full max-w-[1100px] flex-1 flex-col items-center justify-center gap-3 rounded-3xl bg-muted/10 p-3 ring-1 ring-border/20">
|
||||
|
|
@ -370,11 +558,29 @@ const GeneratedImageViewportOverlay: FC<{ hideComposer?: boolean }> = ({
|
|||
const ThreadComposerDock: FC<{
|
||||
disabled?: boolean;
|
||||
threadId?: string | null;
|
||||
}> = ({ disabled, threadId }) => {
|
||||
onHeightChange?: (height: number | null) => void;
|
||||
}> = ({ disabled, threadId, onHeightChange }) => {
|
||||
const { overlay } = useGeneratedImageOverlay();
|
||||
|
||||
// Report the dock's rendered height so the viewport can reserve matching
|
||||
// scroll space when attachments or multiline input grow the composer.
|
||||
const dockRef = useRef<HTMLDivElement | null>(null);
|
||||
useEffect(() => {
|
||||
const el = dockRef.current;
|
||||
if (!el || !onHeightChange) return;
|
||||
const measure = () => onHeightChange(el.offsetHeight);
|
||||
measure();
|
||||
const resizeObserver = new ResizeObserver(measure);
|
||||
resizeObserver.observe(el);
|
||||
return () => {
|
||||
resizeObserver.disconnect();
|
||||
onHeightChange(null);
|
||||
};
|
||||
}, [onHeightChange]);
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={dockRef}
|
||||
className={cn(
|
||||
"aui-thread-composer-dock pointer-events-none absolute bottom-0 left-0 right-0 md:right-[10px]",
|
||||
overlay ? "z-40" : "z-20",
|
||||
|
|
|
|||
|
|
@ -77,6 +77,14 @@ type AutoScrollContextValue = {
|
|||
scrollToBottom: ScrollToBottom;
|
||||
getIsAtBottom: () => boolean;
|
||||
subscribe: (listener: () => void) => () => void;
|
||||
/**
|
||||
* Mark the user as detached from the bottom, as if they had scrolled
|
||||
* up. Called when the composer grows and the bottom spacer grows with
|
||||
* it: the chat is then above the new bottom, and observer-driven pins
|
||||
* must not shove it up. Scrolling back to the bottom re-attaches;
|
||||
* explicit pins (run start, scroll-to-bottom button) still work.
|
||||
*/
|
||||
detachFromBottom: () => void;
|
||||
};
|
||||
|
||||
const noopContext: AutoScrollContextValue = {
|
||||
|
|
@ -87,6 +95,9 @@ const noopContext: AutoScrollContextValue = {
|
|||
subscribe: () => () => {
|
||||
/* no-op */
|
||||
},
|
||||
detachFromBottom: () => {
|
||||
/* no viewport mounted */
|
||||
},
|
||||
};
|
||||
|
||||
const AutoScrollContext = createContext<AutoScrollContextValue>(noopContext);
|
||||
|
|
@ -129,6 +140,9 @@ export function useIntentAwareAutoScroll(): {
|
|||
const scrollImplRef = useRef<ScrollToBottom>(() => {
|
||||
/* no viewport mounted */
|
||||
});
|
||||
const detachImplRef = useRef<() => void>(() => {
|
||||
/* no viewport mounted */
|
||||
});
|
||||
|
||||
const getIsAtBottom = useCallback(() => isAtBottomRef.current, []);
|
||||
|
||||
|
|
@ -153,8 +167,12 @@ export function useIntentAwareAutoScroll(): {
|
|||
scrollImplRef.current(behavior);
|
||||
}, []);
|
||||
|
||||
const detachFromBottom = useCallback(() => {
|
||||
detachImplRef.current();
|
||||
}, []);
|
||||
|
||||
const attach = useCallback(
|
||||
(el: HTMLElement) => {
|
||||
(el: HTMLElement, isRebind: boolean) => {
|
||||
let rafId: number | null = null;
|
||||
let lastScrollTop = el.scrollTop;
|
||||
let lastClientWidth = el.clientWidth;
|
||||
|
|
@ -286,6 +304,13 @@ export function useIntentAwareAutoScroll(): {
|
|||
requestTick();
|
||||
};
|
||||
|
||||
// Programmatic detach (see detachFromBottom). Same effect as the
|
||||
// user scrolling up; the tick refresh updates isAtBottom.
|
||||
detachImplRef.current = () => {
|
||||
detach();
|
||||
requestTick();
|
||||
};
|
||||
|
||||
const onWheel = (e: WheelEvent) => {
|
||||
if (
|
||||
e.deltaY < 0 &&
|
||||
|
|
@ -476,21 +501,24 @@ export function useIntentAwareAutoScroll(): {
|
|||
const mutationObserver = new MutationObserver(onLayoutChange);
|
||||
const onViewportResize = onLayoutChange;
|
||||
|
||||
// Fresh attach always starts pinned. `userDetachedRef` survives
|
||||
// ref rebinds (it's hook-scoped), so if the viewport element is
|
||||
// ever unmounted and remounted without an AUI lifecycle event
|
||||
// (e.g. a parent layout refactor that remounts the viewport),
|
||||
// a prior detach would silently disable auto-follow for the
|
||||
// rest of the session.
|
||||
userDetachedRef.current = false;
|
||||
// Fresh attach (a new viewport element) always starts pinned.
|
||||
// Rebinds to the SAME element must not pin or reset detach state:
|
||||
// the Viewport composes refs with an identity that changes on
|
||||
// re-render, so React re-runs the ref (null, then same element)
|
||||
// on unrelated renders such as composer resizes. Pinning here
|
||||
// would yank the chat to the bottom on every such render. The
|
||||
// observers below are re-installed either way.
|
||||
if (!isRebind) {
|
||||
userDetachedRef.current = false;
|
||||
|
||||
// Pin to bottom when the ref first attaches. Covers the case
|
||||
// where `thread.initialize` fires before the ref is bound.
|
||||
extendFollow();
|
||||
if (el.scrollHeight > el.clientHeight) {
|
||||
el.scrollTo({ top: el.scrollHeight, behavior: "instant" });
|
||||
// Pin to bottom when the ref first attaches. Covers the case
|
||||
// where `thread.initialize` fires before the ref is bound.
|
||||
extendFollow();
|
||||
if (el.scrollHeight > el.clientHeight) {
|
||||
el.scrollTo({ top: el.scrollHeight, behavior: "instant" });
|
||||
}
|
||||
setIsAtBottom(true);
|
||||
}
|
||||
setIsAtBottom(true);
|
||||
requestTick();
|
||||
|
||||
// Observe the border box, not the content box. The stabilizer
|
||||
|
|
@ -544,6 +572,9 @@ export function useIntentAwareAutoScroll(): {
|
|||
scrollImplRef.current = () => {
|
||||
/* no viewport mounted */
|
||||
};
|
||||
detachImplRef.current = () => {
|
||||
/* no viewport mounted */
|
||||
};
|
||||
};
|
||||
},
|
||||
[setIsAtBottom],
|
||||
|
|
@ -562,6 +593,7 @@ export function useIntentAwareAutoScroll(): {
|
|||
useAuiEvent("thread.initialize", () => pinToBottom("instant"));
|
||||
useAuiEvent("threadListItem.switchedTo", () => pinToBottom("instant"));
|
||||
|
||||
const lastElRef = useRef<HTMLElement | null>(null);
|
||||
const ref = useCallback<RefCallback<HTMLElement>>(
|
||||
(el) => {
|
||||
if (cleanupRef.current) {
|
||||
|
|
@ -569,15 +601,20 @@ export function useIntentAwareAutoScroll(): {
|
|||
cleanupRef.current = null;
|
||||
}
|
||||
if (el) {
|
||||
cleanupRef.current = attach(el);
|
||||
// Same-element rebind vs a genuinely new element, see attach().
|
||||
const isRebind = lastElRef.current === el;
|
||||
lastElRef.current = el;
|
||||
cleanupRef.current = attach(el, isRebind);
|
||||
}
|
||||
// On null, keep lastElRef so a rebind to the same element is
|
||||
// recognized; a real remount binds a different element anyway.
|
||||
},
|
||||
[attach],
|
||||
);
|
||||
|
||||
const context = useMemo<AutoScrollContextValue>(
|
||||
() => ({ scrollToBottom, getIsAtBottom, subscribe }),
|
||||
[scrollToBottom, getIsAtBottom, subscribe],
|
||||
() => ({ scrollToBottom, getIsAtBottom, subscribe, detachFromBottom }),
|
||||
[scrollToBottom, getIsAtBottom, subscribe, detachFromBottom],
|
||||
);
|
||||
|
||||
return { ref, context };
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue