mirror of
https://github.com/All-Hands-AI/OpenHands.git
synced 2026-01-09 14:57:59 -05:00
feat: add chat message skeletons and improve routing stability (#12223)
Co-authored-by: amanape <83104063+amanape@users.noreply.github.com>
This commit is contained in:
@@ -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(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<MemoryRouter>{ui}</MemoryRouter>
|
||||
<MemoryRouter initialEntries={[route]}>
|
||||
<Routes>
|
||||
<Route path="/:conversationId" element={ui} />
|
||||
<Route path="/" element={ui} />
|
||||
</Routes>
|
||||
</MemoryRouter>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
|
||||
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<typeof vi.fn>).mockReturnValue({
|
||||
(
|
||||
useUnifiedUploadFiles as unknown as ReturnType<typeof vi.fn>
|
||||
).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<typeof vi.fn>).mockReturnValue({
|
||||
(
|
||||
useUnifiedUploadFiles as unknown as ReturnType<typeof vi.fn>
|
||||
).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<typeof vi.fn>).mockReturnValue({
|
||||
send: vi.fn(),
|
||||
isLoadingMessages: true,
|
||||
parsedEvents: [],
|
||||
});
|
||||
|
||||
renderWithQueryClient(<ChatInterface />, 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<typeof vi.fn>).mockReturnValue({
|
||||
conversationId: "",
|
||||
});
|
||||
(useWsClient as unknown as ReturnType<typeof vi.fn>).mockReturnValue({
|
||||
send: vi.fn(),
|
||||
isLoadingMessages: true,
|
||||
parsedEvents: [],
|
||||
});
|
||||
|
||||
renderWithQueryClient(<ChatInterface />, new QueryClient(), "/");
|
||||
|
||||
expect(screen.getAllByTestId("loading-spinner").length).toBeGreaterThan(0);
|
||||
|
||||
expect(
|
||||
screen.queryByTestId("chat-messages-skeleton"),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
@@ -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(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<ConversationWebSocketProvider
|
||||
conversationId={conversationId}
|
||||
conversationUrl={conversationUrl}
|
||||
sessionApiKey={sessionApiKey}
|
||||
>
|
||||
{children}
|
||||
</ConversationWebSocketProvider>
|
||||
<MemoryRouter initialEntries={["/test-conversation-default"]}>
|
||||
<Routes>
|
||||
<Route
|
||||
path="/:conversationId"
|
||||
element={
|
||||
<ConversationWebSocketProvider
|
||||
conversationId={conversationId}
|
||||
conversationUrl={conversationUrl}
|
||||
sessionApiKey={sessionApiKey}
|
||||
>
|
||||
{children}
|
||||
</ConversationWebSocketProvider>
|
||||
}
|
||||
/>
|
||||
</Routes>
|
||||
</MemoryRouter>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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() {
|
||||
<div className="h-full flex flex-col justify-between pr-0 md:pr-4 relative">
|
||||
{!hasSubstantiveAgentActions &&
|
||||
!optimisticUserMessage &&
|
||||
!userEventsExist && (
|
||||
!userEventsExist &&
|
||||
!isChatLoading && (
|
||||
<ChatSuggestions
|
||||
onSuggestionsClick={(message) => setMessageToSend(message)}
|
||||
/>
|
||||
@@ -277,22 +286,18 @@ export function ChatInterface() {
|
||||
<div
|
||||
ref={scrollRef}
|
||||
onScroll={(e) => 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 && (
|
||||
<div className="flex justify-center">
|
||||
{isChatLoading && isReturningToConversation && (
|
||||
<ChatMessagesSkeleton />
|
||||
)}
|
||||
|
||||
{isChatLoading && !isReturningToConversation && (
|
||||
<div className="flex justify-center" data-testid="loading-spinner">
|
||||
<LoadingSpinner size="small" />
|
||||
</div>
|
||||
)}
|
||||
|
||||
{(conversationWebSocket?.isLoadingHistory || !showV1Messages) &&
|
||||
isV1Conversation &&
|
||||
!isTask && (
|
||||
<div className="flex justify-center">
|
||||
<LoadingSpinner size="small" />
|
||||
</div>
|
||||
)}
|
||||
|
||||
{!isLoadingMessages && v0UserEventsExist && (
|
||||
<V0Messages
|
||||
messages={v0Events}
|
||||
|
||||
@@ -0,0 +1,37 @@
|
||||
import React from "react";
|
||||
|
||||
const SKELETON_PATTERN = [
|
||||
{ width: "w-[25%]", height: "h-4", align: "justify-end" },
|
||||
{ width: "w-[60%]", height: "h-4", align: "justify-start" },
|
||||
{ width: "w-[45%]", height: "h-4", align: "justify-start" },
|
||||
{ width: "w-[65%]", height: "h-20", align: "justify-start" },
|
||||
{ width: "w-[35%]", height: "h-4", align: "justify-end" },
|
||||
{ width: "w-[50%]", height: "h-4", align: "justify-start" },
|
||||
{ width: "w-[30%]", height: "h-4", align: "justify-end" },
|
||||
{ width: "w-[75%]", height: "h-4", align: "justify-start" },
|
||||
{ width: "w-[55%]", height: "h-4", align: "justify-start" },
|
||||
];
|
||||
|
||||
function SkeletonBlock({ width, height }: { width: string; height: string }) {
|
||||
return (
|
||||
<div
|
||||
className={`rounded-md bg-foreground/5 animate-pulse ${width} ${height}`}
|
||||
/>
|
||||
);
|
||||
}
|
||||
|
||||
export function ChatMessagesSkeleton() {
|
||||
return (
|
||||
<div
|
||||
className="flex flex-col gap-6 p-4 w-full h-full overflow-hidden"
|
||||
data-testid="chat-messages-skeleton"
|
||||
aria-label="Loading conversation"
|
||||
>
|
||||
{SKELETON_PATTERN.map((item, i) => (
|
||||
<div key={i} className={`flex w-full ${item.align}`}>
|
||||
<SkeletonBlock width={item.width} height={item.height} />
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
);
|
||||
}
|
||||
@@ -61,11 +61,7 @@ export function useScrollToBottom(scrollRef: RefObject<HTMLDivElement | null>) {
|
||||
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<HTMLDivElement | null>) {
|
||||
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
|
||||
|
||||
@@ -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<typeof import("react-router")>("react-router");
|
||||
return {
|
||||
...actual,
|
||||
useParams: () => ({ conversationId: "test-conversation-id" }),
|
||||
useParams: useParamsMock,
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user