From 1907ebeaa89a2950e81894721c19fa5eb0bde8fe Mon Sep 17 00:00:00 2001 From: Ryanakml <117444976+Ryanakml@users.noreply.github.com> Date: Wed, 7 Jan 2026 03:29:08 +0700 Subject: [PATCH] feat: add chat message skeletons and improve routing stability (#12223) Co-authored-by: amanape <83104063+amanape@users.noreply.github.com> --- .../components/chat/chat-interface.test.tsx | 81 +++++++++++++++---- .../conversation-websocket-handler.test.tsx | 24 ++++-- .../features/chat/chat-interface.tsx | 29 ++++--- .../features/chat/chat-messages-skeleton.tsx | 37 +++++++++ frontend/src/hooks/use-scroll-to-bottom.ts | 12 +-- frontend/test-utils.tsx | 6 +- 6 files changed, 142 insertions(+), 47 deletions(-) create mode 100644 frontend/src/components/features/chat/chat-messages-skeleton.tsx diff --git a/frontend/__tests__/components/chat/chat-interface.test.tsx b/frontend/__tests__/components/chat/chat-interface.test.tsx index 7e8f0b8d2b..fe0cf5f91b 100644 --- a/frontend/__tests__/components/chat/chat-interface.test.tsx +++ b/frontend/__tests__/components/chat/chat-interface.test.tsx @@ -10,13 +10,14 @@ import { } from "vitest"; import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { MemoryRouter } from "react-router"; +import { MemoryRouter, Route, Routes } from "react-router"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -import { renderWithProviders } from "test-utils"; +import { renderWithProviders, useParamsMock } from "test-utils"; import type { Message } from "#/message"; import { SUGGESTIONS } from "#/utils/suggestions"; import { ChatInterface } from "#/components/features/chat/chat-interface"; import { useWsClient } from "#/context/ws-client-provider"; +import { useConversationId } from "#/hooks/use-conversation-id"; import { useErrorMessageStore } from "#/stores/error-message-store"; import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store"; import { useConfig } from "#/hooks/query/use-config"; @@ -31,19 +32,8 @@ vi.mock("#/context/ws-client-provider"); vi.mock("#/hooks/query/use-config"); vi.mock("#/hooks/mutation/use-get-trajectory"); vi.mock("#/hooks/mutation/use-unified-upload-files"); +vi.mock("#/hooks/use-conversation-id"); -// Mock React Router hooks at the top level -vi.mock("react-router", async () => { - const actual = await vi.importActual("react-router"); - return { - ...actual, - useNavigate: () => vi.fn(), - useParams: () => ({ conversationId: "test-conversation-id" }), - useRouteLoaderData: vi.fn(() => ({})), - }; -}); - -// Mock other hooks that might be used by the component vi.mock("#/hooks/use-user-providers", () => ({ useUserProviders: () => ({ providers: [], @@ -87,13 +77,26 @@ const renderChatInterface = (messages: Message[]) => const renderWithQueryClient = ( ui: React.ReactElement, queryClient: QueryClient, + route = "/test-conversation-id", ) => render( - {ui} + + + + + + , ); +beforeEach(() => { + useParamsMock.mockReturnValue({ conversationId: "test-conversation-id" }); + vi.mocked(useConversationId).mockReturnValue({ + conversationId: "test-conversation-id", + }); +}); + describe("ChatInterface - Chat Suggestions", () => { // Create a new QueryClient for each test let queryClient: QueryClient; @@ -129,7 +132,9 @@ describe("ChatInterface - Chat Suggestions", () => { mutateAsync: vi.fn(), isLoading: false, }); - (useUnifiedUploadFiles as unknown as ReturnType).mockReturnValue({ + ( + useUnifiedUploadFiles as unknown as ReturnType + ).mockReturnValue({ mutateAsync: vi .fn() .mockResolvedValue({ skipped_files: [], uploaded_files: [] }), @@ -260,7 +265,9 @@ describe("ChatInterface - Empty state", () => { mutateAsync: vi.fn(), isLoading: false, }); - (useUnifiedUploadFiles as unknown as ReturnType).mockReturnValue({ + ( + useUnifiedUploadFiles as unknown as ReturnType + ).mockReturnValue({ mutateAsync: vi .fn() .mockResolvedValue({ skipped_files: [], uploaded_files: [] }), @@ -635,3 +642,43 @@ describe.skip("ChatInterface - General functionality", () => { expect(screen.getByTestId("feedback-actions")).toBeInTheDocument(); }); }); + +describe("ChatInterface – skeleton loading state", () => { + test("renders chat message skeleton when loading existing conversation", () => { + (useWsClient as unknown as ReturnType).mockReturnValue({ + send: vi.fn(), + isLoadingMessages: true, + parsedEvents: [], + }); + + renderWithQueryClient(, new QueryClient()); + + expect(screen.getByTestId("chat-messages-skeleton")).toBeInTheDocument(); + + expect(screen.queryByTestId("loading-spinner")).not.toBeInTheDocument(); + + expect(screen.queryByTestId("chat-suggestions")).not.toBeInTheDocument(); + }); +}); + +test("does not render skeleton for new conversation (shows spinner instead)", () => { + useParamsMock.mockReturnValue({ conversationId: undefined } as unknown as { + conversationId: string; + }); + (useConversationId as unknown as ReturnType).mockReturnValue({ + conversationId: "", + }); + (useWsClient as unknown as ReturnType).mockReturnValue({ + send: vi.fn(), + isLoadingMessages: true, + parsedEvents: [], + }); + + renderWithQueryClient(, new QueryClient(), "/"); + + expect(screen.getAllByTestId("loading-spinner").length).toBeGreaterThan(0); + + expect( + screen.queryByTestId("chat-messages-skeleton"), + ).not.toBeInTheDocument(); +}); diff --git a/frontend/__tests__/conversation-websocket-handler.test.tsx b/frontend/__tests__/conversation-websocket-handler.test.tsx index ab6243e100..6eb1186268 100644 --- a/frontend/__tests__/conversation-websocket-handler.test.tsx +++ b/frontend/__tests__/conversation-websocket-handler.test.tsx @@ -11,6 +11,7 @@ import { import { screen, waitFor, render, cleanup } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { http, HttpResponse } from "msw"; +import { MemoryRouter, Route, Routes } from "react-router"; import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store"; import { useBrowserStore } from "#/stores/browser-store"; import { useCommandStore } from "#/stores/command-store"; @@ -78,13 +79,22 @@ function renderWithWebSocketContext( return render( - - {children} - + + + + {children} + + } + /> + + , ); } diff --git a/frontend/src/components/features/chat/chat-interface.tsx b/frontend/src/components/features/chat/chat-interface.tsx index 1a3b169198..d57b5acc3e 100644 --- a/frontend/src/components/features/chat/chat-interface.tsx +++ b/frontend/src/components/features/chat/chat-interface.tsx @@ -21,6 +21,7 @@ import { useAgentState } from "#/hooks/use-agent-state"; import { ScrollToBottomButton } from "#/components/shared/buttons/scroll-to-bottom-button"; import { LoadingSpinner } from "#/components/shared/loading-spinner"; +import { ChatMessagesSkeleton } from "./chat-messages-skeleton"; import { displayErrorToast } from "#/utils/custom-toast-handlers"; import { useErrorMessageStore } from "#/stores/error-message-store"; import { useOptimisticUserMessageStore } from "#/stores/optimistic-user-message-store"; @@ -124,6 +125,13 @@ export function ChatInterface() { prevV1LoadingRef.current = isLoading; }, [conversationWebSocket?.isLoadingHistory]); + const isReturningToConversation = !!params.conversationId; + const isHistoryLoading = + (isLoadingMessages && !isV1Conversation) || + (isV1Conversation && + (conversationWebSocket?.isLoadingHistory || !showV1Messages)); + const isChatLoading = isHistoryLoading && !isTask; + // Filter V0 events const v0Events = storeEvents .filter(isV0Event) @@ -267,7 +275,8 @@ export function ChatInterface() {
{!hasSubstantiveAgentActions && !optimisticUserMessage && - !userEventsExist && ( + !userEventsExist && + !isChatLoading && ( setMessageToSend(message)} /> @@ -277,22 +286,18 @@ export function ChatInterface() {
onChatBodyScroll(e.currentTarget)} - className="custom-scrollbar-always flex flex-col grow overflow-y-auto overflow-x-hidden px-4 pt-4 gap-2 fast-smooth-scroll" + className="custom-scrollbar-always flex flex-col grow overflow-y-auto overflow-x-hidden px-4 pt-4 gap-2" > - {isLoadingMessages && !isV1Conversation && !isTask && ( -
+ {isChatLoading && isReturningToConversation && ( + + )} + + {isChatLoading && !isReturningToConversation && ( +
)} - {(conversationWebSocket?.isLoadingHistory || !showV1Messages) && - isV1Conversation && - !isTask && ( -
- -
- )} - {!isLoadingMessages && v0UserEventsExist && ( + ); +} + +export function ChatMessagesSkeleton() { + return ( +
+ {SKELETON_PATTERN.map((item, i) => ( +
+ +
+ ))} +
+ ); +} diff --git a/frontend/src/hooks/use-scroll-to-bottom.ts b/frontend/src/hooks/use-scroll-to-bottom.ts index 18516785fa..0f59ce184a 100644 --- a/frontend/src/hooks/use-scroll-to-bottom.ts +++ b/frontend/src/hooks/use-scroll-to-bottom.ts @@ -61,11 +61,7 @@ export function useScrollToBottom(scrollRef: RefObject) { setAutoscroll(true); setHitBottom(true); - // Use smooth scrolling but with a fast duration - dom.scrollTo({ - top: dom.scrollHeight, - behavior: "smooth", - }); + dom.scrollTop = dom.scrollHeight; }); } }, [scrollRef]); @@ -77,11 +73,7 @@ export function useScrollToBottom(scrollRef: RefObject) { if (autoscroll) { const dom = scrollRef.current; if (dom) { - // Scroll to bottom - this will trigger on any DOM change - dom.scrollTo({ - top: dom.scrollHeight, - behavior: "smooth", - }); + dom.scrollTop = dom.scrollHeight; } } }); // No dependency array - runs after every render to follow new content diff --git a/frontend/test-utils.tsx b/frontend/test-utils.tsx index a55cb7d132..0d2a55f51d 100644 --- a/frontend/test-utils.tsx +++ b/frontend/test-utils.tsx @@ -8,13 +8,17 @@ import i18n from "i18next"; import { vi } from "vitest"; import { AxiosError } from "axios"; +export const useParamsMock = vi.fn(() => ({ + conversationId: "test-conversation-id", +})); + // Mock useParams before importing components vi.mock("react-router", async () => { const actual = await vi.importActual("react-router"); return { ...actual, - useParams: () => ({ conversationId: "test-conversation-id" }), + useParams: useParamsMock, }; });