refactor(frontend): remove HeroUI BaseModal and migrate MetricsModal (#12174)

Co-authored-by: openhands <openhands@all-hands.dev>
Co-authored-by: amanape <83104063+amanape@users.noreply.github.com>

frontend/__tests__/components/modals/base-modal/base-modal.test.tsx

@@ -1,151 +0,0 @@
-import { render, screen, act } from "@testing-library/react";
-import userEvent from "@testing-library/user-event";
-import { describe, it, vi, expect } from "vitest";
-import { BaseModal } from "#/components/shared/modals/base-modal/base-modal";
-
-describe("BaseModal", () => {
-  const onOpenChangeMock = vi.fn();
-
-  it("should render if the modal is open", () => {
-    const { rerender } = render(
-      <BaseModal
-        isOpen={false}
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-      />,
-    );
-    expect(screen.queryByText("Settings")).not.toBeInTheDocument();
-
-    rerender(
-      <BaseModal title="Settings" onOpenChange={onOpenChangeMock} isOpen />,
-    );
-    expect(screen.getByText("Settings")).toBeInTheDocument();
-  });
-
-  it("should render an optional subtitle", () => {
-    render(
-      <BaseModal
-        isOpen
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-        subtitle="Subtitle"
-      />,
-    );
-    expect(screen.getByText("Subtitle")).toBeInTheDocument();
-  });
-
-  it("should render actions", async () => {
-    const onPrimaryClickMock = vi.fn();
-    const onSecondaryClickMock = vi.fn();
-
-    const primaryAction = {
-      action: onPrimaryClickMock,
-      label: "Save",
-    };
-
-    const secondaryAction = {
-      action: onSecondaryClickMock,
-      label: "Cancel",
-    };
-
-    render(
-      <BaseModal
-        isOpen
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-        actions={[primaryAction, secondaryAction]}
-      />,
-    );
-
-    expect(screen.getByText("Save")).toBeInTheDocument();
-    expect(screen.getByText("Cancel")).toBeInTheDocument();
-
-    await userEvent.click(screen.getByText("Save"));
-    expect(onPrimaryClickMock).toHaveBeenCalledTimes(1);
-
-    await userEvent.click(screen.getByText("Cancel"));
-    expect(onSecondaryClickMock).toHaveBeenCalledTimes(1);
-  });
-
-  it("should close the modal after an action is performed", async () => {
-    render(
-      <BaseModal
-        isOpen
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-        actions={[
-          {
-            label: "Save",
-            action: () => {},
-            closeAfterAction: true,
-          },
-        ]}
-      />,
-    );
-
-    await userEvent.click(screen.getByText("Save"));
-    expect(onOpenChangeMock).toHaveBeenCalledTimes(1);
-  });
-
-  it("should render children", () => {
-    render(
-      <BaseModal isOpen onOpenChange={onOpenChangeMock} title="Settings">
-        <div>Children</div>
-      </BaseModal>,
-    );
-    expect(screen.getByText("Children")).toBeInTheDocument();
-  });
-
-  it("should disable the action given the condition", () => {
-    const { rerender } = render(
-      <BaseModal
-        isOpen
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-        actions={[
-          {
-            label: "Save",
-            action: () => {},
-            isDisabled: true,
-          },
-        ]}
-      />,
-    );
-
-    expect(screen.getByText("Save")).toBeDisabled();
-
-    rerender(
-      <BaseModal
-        isOpen
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-        actions={[
-          {
-            label: "Save",
-            action: () => {},
-            isDisabled: false,
-          },
-        ]}
-      />,
-    );
-
-    expect(screen.getByText("Save")).not.toBeDisabled();
-  });
-
-  it.skip("should not close if the backdrop or escape key is pressed", () => {
-    render(
-      <BaseModal
-        isOpen
-        onOpenChange={onOpenChangeMock}
-        title="Settings"
-        isDismissable={false}
-      />,
-    );
-
-    act(() => {
-      userEvent.keyboard("{esc}");
-    });
-    // fails because the nextui component wraps the modal content in an aria-hidden div
-    expect(screen.getByRole("dialog")).toBeVisible();
-  });
-});

frontend/src/components/features/conversation/metrics-modal/metrics-modal.tsx

@@ -1,5 +1,7 @@
 import { useTranslation } from "react-i18next";
-import { BaseModal } from "../../../shared/modals/base-modal/base-modal";
+import { BaseModalTitle } from "#/components/shared/modals/confirmation-modals/base-modal";
+import { ModalBackdrop } from "#/components/shared/modals/modal-backdrop";
+import { ModalBody } from "#/components/shared/modals/modal-body";
 import { I18nKey } from "#/i18n/declaration";
 import { CostSection } from "./cost-section";
 import { UsageSection } from "./usage-section";
@@ -15,38 +17,37 @@ interface MetricsModalProps {
 export function MetricsModal({ isOpen, onOpenChange }: MetricsModalProps) {
   const { t } = useTranslation();
   const metrics = useMetricsStore();
+  if (!isOpen) return null;
 
   return (
-    <BaseModal
-      isOpen={isOpen}
-      onOpenChange={onOpenChange}
-      title={t(I18nKey.CONVERSATION$METRICS_INFO)}
-      testID="metrics-modal"
-    >
-      <div className="space-y-4">
-        {(metrics?.cost !== null || metrics?.usage !== null) && (
-          <div className="rounded-md p-3">
-            <div className="grid gap-3">
-              <CostSection
-                cost={metrics?.cost ?? null}
-                maxBudgetPerTask={metrics?.max_budget_per_task ?? null}
-              />
+    <ModalBackdrop onClose={() => onOpenChange(false)}>
+      <ModalBody className="items-center border border-tertiary">
+        <BaseModalTitle title={t(I18nKey.CONVERSATION$METRICS_INFO)} />
+        <div className="space-y-4 w-full">
+          {(metrics?.cost !== null || metrics?.usage !== null) && (
+            <div className="rounded-md p-3">
+              <div className="grid gap-3">
+                <CostSection
+                  cost={metrics?.cost ?? null}
+                  maxBudgetPerTask={metrics?.max_budget_per_task ?? null}
+                />
 
-              {metrics?.usage !== null && (
-                <>
-                  <UsageSection usage={metrics.usage} />
-                  <ContextWindowSection
-                    perTurnToken={metrics.usage.per_turn_token}
-                    contextWindow={metrics.usage.context_window}
-                  />
-                </>
-              )}
+                {metrics?.usage !== null && (
+                  <>
+                    <UsageSection usage={metrics.usage} />
+                    <ContextWindowSection
+                      perTurnToken={metrics.usage.per_turn_token}
+                      contextWindow={metrics.usage.context_window}
+                    />
+                  </>
+                )}
+              </div>
             </div>
-          </div>
-        )}
+          )}
 
-        {!metrics?.cost && !metrics?.usage && <EmptyState />}
-      </div>
-    </BaseModal>
+          {!metrics?.cost && !metrics?.usage && <EmptyState />}
+        </div>
+      </ModalBody>
+    </ModalBackdrop>
   );
 }

frontend/src/components/shared/modals/base-modal/base-modal.tsx

@@ -1,69 +0,0 @@
-import {
-  Modal,
-  ModalBody,
-  ModalContent,
-  ModalFooter,
-  ModalHeader,
-} from "@heroui/react";
-import React from "react";
-import { Action, FooterContent } from "./footer-content";
-import { HeaderContent } from "./header-content";
-
-interface BaseModalProps {
-  isOpen: boolean;
-  onOpenChange: (isOpen: boolean) => void;
-  title: string;
-  contentClassName?: string;
-  bodyClassName?: string;
-  isDismissable?: boolean;
-  subtitle?: string;
-  actions?: Action[];
-  children?: React.ReactNode;
-  testID?: string;
-}
-
-export function BaseModal({
-  isOpen,
-  onOpenChange,
-  title,
-  contentClassName = "max-w-[30rem] p-[40px]",
-  bodyClassName = "px-0 py-[20px]",
-  isDismissable = true,
-  subtitle = undefined,
-  actions = [],
-  children = null,
-  testID,
-}: BaseModalProps) {
-  return (
-    <Modal
-      data-testid={testID}
-      isOpen={isOpen}
-      onOpenChange={onOpenChange}
-      isDismissable={isDismissable}
-      backdrop="blur"
-      hideCloseButton
-      size="sm"
-      className="bg-base-secondary rounded-lg"
-    >
-      <ModalContent className={contentClassName}>
-        {(closeModal) => (
-          <>
-            {title && (
-              <ModalHeader className="flex flex-col p-0">
-                <HeaderContent maintitle={title} subtitle={subtitle} />
-              </ModalHeader>
-            )}
-
-            <ModalBody className={bodyClassName}>{children}</ModalBody>
-
-            {actions && actions.length > 0 && (
-              <ModalFooter className="flex-row flex justify-start p-0">
-                <FooterContent actions={actions} closeModal={closeModal} />
-              </ModalFooter>
-            )}
-          </>
-        )}
-      </ModalContent>
-    </Modal>
-  );
-}

frontend/src/components/shared/modals/base-modal/footer-content.tsx

@@ -1,38 +0,0 @@
-import { Button } from "@heroui/react";
-import React from "react";
-
-export interface Action {
-  action: () => void;
-  isDisabled?: boolean;
-  label: string;
-  className?: string;
-  closeAfterAction?: boolean;
-}
-
-interface FooterContentProps {
-  actions: Action[];
-  closeModal: () => void;
-}
-
-export function FooterContent({ actions, closeModal }: FooterContentProps) {
-  return (
-    <>
-      {actions.map(
-        ({ action, isDisabled, label, className, closeAfterAction }) => (
-          <Button
-            key={label}
-            type="button"
-            isDisabled={isDisabled}
-            onPress={() => {
-              action();
-              if (closeAfterAction) closeModal();
-            }}
-            className={className}
-          >
-            {label}
-          </Button>
-        ),
-      )}
-    </>
-  );
-}

frontend/src/components/shared/modals/base-modal/header-content.tsx

@@ -1,20 +0,0 @@
-import React from "react";
-
-interface HeaderContentProps {
-  maintitle: string;
-  subtitle?: string;
-}
-
-export function HeaderContent({
-  maintitle,
-  subtitle = undefined,
-}: HeaderContentProps) {
-  return (
-    <>
-      <h3>{maintitle}</h3>
-      {subtitle && (
-        <span className="text-neutral-400 text-sm font-light">{subtitle}</span>
-      )}
-    </>
-  );
-}