Fix expandable messages (#5650)

Co-authored-by: amanape <[email protected]>
All-Hands-AI · Dec 17, 2024 · 8bd2205 · 8bd2205
1 parent 6ae84bf
commit 8bd2205
Show file tree

Hide file tree

Showing 2 changed files with 87 additions and 41 deletions.
diff --git a/frontend/__tests__/components/chat/expandable-message.test.tsx b/frontend/__tests__/components/chat/expandable-message.test.tsx
@@ -2,34 +2,51 @@ import { describe, expect, it } from "vitest";
 import { screen } from "@testing-library/react";
 import { renderWithProviders } from "test-utils";
 import { ExpandableMessage } from "#/components/features/chat/expandable-message";
+import { vi } from 'vitest';
+
+vi.mock('react-i18next', async () => {
+  const actual = await vi.importActual('react-i18next');
+  return {
+    ...actual,
+    useTranslation: () => ({
+      t: (key:string) => key,
+      i18n: {
+        changeLanguage: () => new Promise(() => {}),
+        language: 'en',
+        exists: () => true,
+      },
+    }),
+  }
+});
 
 describe("ExpandableMessage", () => {
   it("should render with neutral border for non-action messages", () => {
     renderWithProviders(<ExpandableMessage message="Hello" type="thought" />);
     const element = screen.getByText("Hello");
-    const container = element.closest("div.flex.gap-2.items-center.justify-between");
+    const container = element.closest("div.flex.gap-2.items-center.justify-start");
     expect(container).toHaveClass("border-neutral-300");
     expect(screen.queryByTestId("status-icon")).not.toBeInTheDocument();
   });
 
   it("should render with neutral border for error messages", () => {
     renderWithProviders(<ExpandableMessage message="Error occurred" type="error" />);
     const element = screen.getByText("Error occurred");
-    const container = element.closest("div.flex.gap-2.items-center.justify-between");
-    expect(container).toHaveClass("border-neutral-300");
+    const container = element.closest("div.flex.gap-2.items-center.justify-start");
+    expect(container).toHaveClass("border-danger");
     expect(screen.queryByTestId("status-icon")).not.toBeInTheDocument();
   });
 
   it("should render with success icon for successful action messages", () => {
     renderWithProviders(
       <ExpandableMessage
+        id="OBSERVATION_MESSAGE$RUN"
         message="Command executed successfully"
         type="action"
         success={true}
       />
     );
-    const element = screen.getByText("Command executed successfully");
-    const container = element.closest("div.flex.gap-2.items-center.justify-between");
+    const element = screen.getByText("OBSERVATION_MESSAGE$RUN");
+    const container = element.closest("div.flex.gap-2.items-center.justify-start");
     expect(container).toHaveClass("border-neutral-300");
     const icon = screen.getByTestId("status-icon");
     expect(icon).toHaveClass("fill-success");
@@ -38,22 +55,29 @@ describe("ExpandableMessage", () => {
   it("should render with error icon for failed action messages", () => {
     renderWithProviders(
       <ExpandableMessage
+        id="OBSERVATION_MESSAGE$RUN"
         message="Command failed"
         type="action"
         success={false}
       />
     );
-    const element = screen.getByText("Command failed");
-    const container = element.closest("div.flex.gap-2.items-center.justify-between");
+    const element = screen.getByText("OBSERVATION_MESSAGE$RUN");
+    const container = element.closest("div.flex.gap-2.items-center.justify-start");
     expect(container).toHaveClass("border-neutral-300");
     const icon = screen.getByTestId("status-icon");
     expect(icon).toHaveClass("fill-danger");
   });
 
   it("should render with neutral border and no icon for action messages without success prop", () => {
-    renderWithProviders(<ExpandableMessage message="Running command" type="action" />);
-    const element = screen.getByText("Running command");
-    const container = element.closest("div.flex.gap-2.items-center.justify-between");
+    renderWithProviders(
+      <ExpandableMessage
+        id="OBSERVATION_MESSAGE$RUN"
+        message="Running command"
+        type="action"
+      />
+    );
+    const element = screen.getByText("OBSERVATION_MESSAGE$RUN");
+    const container = element.closest("div.flex.gap-2.items-center.justify-start");
     expect(container).toHaveClass("border-neutral-300");
     expect(screen.queryByTestId("status-icon")).not.toBeInTheDocument();
   });

diff --git a/frontend/src/components/features/chat/expandable-message.tsx b/frontend/src/components/features/chat/expandable-message.tsx
@@ -8,6 +8,7 @@ import ArrowUp from "#/icons/angle-up-solid.svg?react";
 import ArrowDown from "#/icons/angle-down-solid.svg?react";
 import CheckCircle from "#/icons/check-circle-solid.svg?react";
 import XCircle from "#/icons/x-circle-solid.svg?react";
+import { cn } from "#/utils/utils";
 
 interface ExpandableMessageProps {
   id?: string;
@@ -35,27 +36,63 @@ export function ExpandableMessage({
     }
   }, [id, message, i18n.language]);
 
-  const arrowClasses = "h-4 w-4 ml-2 inline fill-neutral-300";
   const statusIconClasses = "h-4 w-4 ml-2 inline";
 
   return (
-    <div className="flex gap-2 items-center justify-between border-l-2 border-neutral-300 pl-2 my-2 py-2">
-      <div className="text-sm leading-4 flex flex-col gap-2 max-w-full">
+    <div
+      className={cn(
+        "flex gap-2 items-center justify-start border-l-2 pl-2 my-2 py-2",
+        type === "error" ? "border-danger" : "border-neutral-300",
+      )}
+    >
+      <div className="text-sm w-full">
         {headline && (
-          <p className="text-neutral-300 font-bold">
-            {headline}
-            <button
-              type="button"
-              onClick={() => setShowDetails(!showDetails)}
-              className="cursor-pointer text-left"
-            >
-              {showDetails ? (
-                <ArrowUp className={arrowClasses} />
-              ) : (
-                <ArrowDown className={arrowClasses} />
+          <div className="flex flex-row justify-between items-center w-full">
+            <span
+              className={cn(
+                "font-bold",
+                type === "error" ? "text-danger" : "text-neutral-300",
               )}
-            </button>
-          </p>
+            >
+              {headline}
+              <button
+                type="button"
+                onClick={() => setShowDetails(!showDetails)}
+                className="cursor-pointer text-left"
+              >
+                {showDetails ? (
+                  <ArrowUp
+                    className={cn(
+                      "h-4 w-4 ml-2 inline",
+                      type === "error" ? "fill-danger" : "fill-neutral-300",
+                    )}
+                  />
+                ) : (
+                  <ArrowDown
+                    className={cn(
+                      "h-4 w-4 ml-2 inline",
+                      type === "error" ? "fill-danger" : "fill-neutral-300",
+                    )}
+                  />
+                )}
+              </button>
+            </span>
+            {type === "action" && success !== undefined && (
+              <span className="flex-shrink-0">
+                {success ? (
+                  <CheckCircle
+                    data-testid="status-icon"
+                    className={cn(statusIconClasses, "fill-success")}
+                  />
+                ) : (
+                  <XCircle
+                    data-testid="status-icon"
+                    className={cn(statusIconClasses, "fill-danger")}
+                  />
+                )}
+              </span>
+            )}
+          </div>
         )}
         {showDetails && (
           <Markdown
@@ -71,21 +108,6 @@ export function ExpandableMessage({
           </Markdown>
         )}
       </div>
-      {type === "action" && success !== undefined && (
-        <div className="flex-shrink-0">
-          {success ? (
-            <CheckCircle
-              data-testid="status-icon"
-              className={`${statusIconClasses} fill-success`}
-            />
-          ) : (
-            <XCircle
-              data-testid="status-icon"
-              className={`${statusIconClasses} fill-danger`}
-            />
-          )}
-        </div>
-      )}
     </div>
   );
 }