mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix(site/src/pages/AgentsPage): restore sticky user message pinning after react-infinite-scroll-component refactor (#24937)
Restores the sticky user message pinning behavior in the Agents chat that regressed after #24687 swapped the chat scroll container for `react-infinite-scroll-component`. ## Root cause `react-infinite-scroll-component` renders two wrapper divs between the `.overflow-y-auto` scroller and the rendered messages, and its inner wrapper hard-codes `overflow: auto` in its inline style. With the new layout, `position: sticky` on a user message resolved against that inner wrapper rather than the real scroller, so the message scrolled out with its sentinel and the existing fade/clip overlay never engaged. ## Fix Force both InfiniteScroll wrappers to `display: contents` so they no longer participate in layout. The user message's nearest scrolling ancestor is once again the `.overflow-y-auto` element, and `position: sticky` anchors to the scroll container as it did before #24687. The outer wrapper is reached via the Tailwind arbitrary selector `[&>[class$=outerdiv]]:contents` because the library only exposes `style` for the inner wrapper. The inverse infinite-scroll behavior is preserved: the scroller itself stays `flex-col-reverse`, so it remains bottom-anchored and the library's load-more sentinel still lands at the visual top of the content stack. Also drops the dead `overflow-y-auto` class on the floating scroll-to-bottom button wrapper noted in the bug report. ## Test coverage Adds `StickyUserMessagePinsOnScroll` to `AgentChatPageView.stories.tsx`. With a 40-message conversation it walks the user-message sentinels in reverse DOM order to find the one currently pinned (the latest sentinel above the scroller's top edge) and asserts the matching sticky container is anchored within a few pixels of that edge. Without the fix the container ends up hundreds of pixels above the scroller because `position: sticky` silently no-ops. The existing structural `StickyUserMessageStructure` story in `ConversationTimeline.stories.tsx` continues to pass unchanged. <details> <summary>Verification</summary> ```sh pnpm exec tsc -p . # 0 errors pnpm run lint:check # passes pnpm exec vitest run --project=unit # 2303 passed pnpm exec vitest run --project=storybook \ src/pages/AgentsPage/AgentChatPage.stories.tsx \ src/pages/AgentsPage/AgentChatPageView.stories.tsx \ src/pages/AgentsPage/components/ChatConversation/ConversationTimeline.stories.tsx # 90 passed ``` Confirmed the new story fails on `main` (sticky container at the sentinel's position instead of the scroller top) and passes with the fix applied. </details> --- Generated by Coder Agents.
This commit is contained in:
@@ -954,6 +954,87 @@ export const MessageOrderIsStillCorrect: Story = {
|
||||
},
|
||||
};
|
||||
|
||||
const stickyPinningStore = buildStoreWithMessages(buildLongConversation(40));
|
||||
|
||||
/**
|
||||
* Regression guard for the StickyUserMessage push-up logic.
|
||||
*
|
||||
* `react-infinite-scroll-component` renders two wrapper divs between the
|
||||
* scroll container and the message tree. The library applies `overflow:
|
||||
* auto` to its inner wrapper, which used to make `position: sticky` on a
|
||||
* user message resolve against that wrapper instead of the actual scroller.
|
||||
* The fix forces both wrappers to `display: contents` so the sticky
|
||||
* container's nearest scrolling ancestor is once again the
|
||||
* `.overflow-y-auto` element.
|
||||
*
|
||||
* This story scrolls past the most recent user message and asserts the
|
||||
* message is pinned within a few pixels of the scroll container's top.
|
||||
*/
|
||||
export const StickyUserMessagePinsOnScroll: Story = {
|
||||
parameters: { chromatic: { disableSnapshot: true } },
|
||||
decorators: scrollStoryDecorators,
|
||||
render: () => <StoryAgentChatPageView store={stickyPinningStore} />,
|
||||
play: async ({ canvasElement }) => {
|
||||
resetScrollStoryStore(stickyPinningStore, 40);
|
||||
const canvas = within(canvasElement);
|
||||
const scrollContainer = canvas.getByTestId("scroll-container");
|
||||
|
||||
await waitForScrollOverflow(scrollContainer);
|
||||
|
||||
// Each sticky user message is the element immediately following its
|
||||
// `data-user-sentinel` marker. The push-up logic depends on the
|
||||
// sticky container resolving against the real scroll container,
|
||||
// which is the regression this story guards against.
|
||||
const sentinels = scrollContainer.querySelectorAll("[data-user-sentinel]");
|
||||
expect(sentinels.length).toBeGreaterThan(0);
|
||||
for (const sentinel of sentinels) {
|
||||
expect(sentinel.closest("[data-testid='scroll-container']")).toBe(
|
||||
scrollContainer,
|
||||
);
|
||||
const container = sentinel.nextElementSibling;
|
||||
expect(container).not.toBeNull();
|
||||
expect(window.getComputedStyle(container as Element).position).toBe(
|
||||
"sticky",
|
||||
);
|
||||
}
|
||||
|
||||
// At the default `scrollTop = 0`, the inverse layout shows the
|
||||
// newest messages at the bottom of the viewport. Older user
|
||||
// messages whose sentinels have already scrolled above the
|
||||
// scroller's top edge should be pinned by `position: sticky`. Pick
|
||||
// a sentinel that is comfortably above the top edge so a tiny
|
||||
// scroll offset cannot flip it on or off the boundary.
|
||||
const scrollerRect = scrollContainer.getBoundingClientRect();
|
||||
// Walk the sentinels in reverse DOM order so we land on the
|
||||
// most recent user message whose sentinel has scrolled above
|
||||
// the scroll container's top edge. That is the message the
|
||||
// push-up logic actively pins at the top; earlier pinned
|
||||
// messages will have been pushed out of view by it.
|
||||
const pinnedSentinel = Array.from(sentinels)
|
||||
.reverse()
|
||||
.find(
|
||||
(sentinel) =>
|
||||
sentinel.getBoundingClientRect().top < scrollerRect.top - 4,
|
||||
) as HTMLElement | undefined;
|
||||
expect(pinnedSentinel).toBeDefined();
|
||||
if (!pinnedSentinel) {
|
||||
return;
|
||||
}
|
||||
const pinnedContainer = pinnedSentinel.nextElementSibling as HTMLElement;
|
||||
|
||||
// `position: sticky` should pin the user message container near
|
||||
// the scroll container's top edge while the assistant response
|
||||
// below it is on screen. Before the fix, the sticky container
|
||||
// resolved against the InfiniteScroll wrapper rather than the
|
||||
// real scroll container, so it scrolled out with its sentinel
|
||||
// and ended up far above the viewport.
|
||||
const pinnedRect = pinnedContainer.getBoundingClientRect();
|
||||
expect(window.getComputedStyle(pinnedContainer).position).toBe("sticky");
|
||||
expect(pinnedRect.top - scrollerRect.top).toBeGreaterThanOrEqual(-1);
|
||||
expect(pinnedRect.top - scrollerRect.top).toBeLessThan(40);
|
||||
},
|
||||
};
|
||||
|
||||
/**
|
||||
* Selecting the Terminal tab in the sidebar must move keyboard focus into
|
||||
* the terminal so typing goes there, not the chat input.
|
||||
|
||||
@@ -74,7 +74,9 @@ const ScrollToBottomButton: FC<{
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="pointer-events-none absolute inset-x-0 bottom-2 z-10 flex justify-center overflow-y-auto py-2 [scrollbar-gutter:stable] [scrollbar-width:thin]">
|
||||
// Floating overlay above the scroll container. The button has its own
|
||||
// fixed-size box so the wrapper does not need overflow handling.
|
||||
<div className="pointer-events-none absolute inset-x-0 bottom-2 z-10 flex justify-center py-2">
|
||||
<Button
|
||||
variant="outline"
|
||||
size="icon"
|
||||
@@ -139,7 +141,15 @@ const ChatScrollContainer: FC<{
|
||||
ref={setScrollContainer}
|
||||
data-testid="scroll-container"
|
||||
aria-busy={isFetchingMoreMessages || undefined}
|
||||
className="flex min-h-0 flex-1 flex-col-reverse overflow-y-auto [scrollbar-gutter:stable] [scrollbar-width:thin] [scrollbar-color:hsl(var(--surface-quaternary))_transparent]"
|
||||
// `react-infinite-scroll-component` renders two wrapper divs
|
||||
// between this scroller and the rendered messages. Force both
|
||||
// out of the layout tree with `display: contents` so that
|
||||
// (a) `position: sticky` on a user message resolves against
|
||||
// this scroller rather than the inner wrapper (which has
|
||||
// `overflow: auto` baked in by the library), and (b) the
|
||||
// column-reverse inverse layout places the library's
|
||||
// load-more sentinel at the visual top of the content stack.
|
||||
className="flex min-h-0 flex-1 flex-col-reverse overflow-y-auto [&>[class$=outerdiv]]:contents [scrollbar-gutter:stable] [scrollbar-width:thin] [scrollbar-color:hsl(var(--surface-quaternary))_transparent]"
|
||||
>
|
||||
<div aria-hidden className="flex-1 basis-0" />
|
||||
<InfiniteScroll
|
||||
@@ -152,7 +162,12 @@ const ChatScrollContainer: FC<{
|
||||
hasChildren={messageCount > 0}
|
||||
loader={isFetchingMoreMessages ? <div aria-hidden /> : null}
|
||||
endMessage={null}
|
||||
style={{ display: "flex", flexDirection: "column-reverse" }}
|
||||
// `display: contents` removes this wrapper's box from the
|
||||
// layout tree. Combined with the `outerdiv:contents`
|
||||
// selector on the scroller above, the children render as
|
||||
// direct flex items of the scroller so sticky messages
|
||||
// can pin to its top edge.
|
||||
style={{ display: "contents" }}
|
||||
>
|
||||
{children}
|
||||
</InfiniteScroll>
|
||||
|
||||
Reference in New Issue
Block a user