From 7a5b3abbdcb4e75b3247a48962b23af388da8fde Mon Sep 17 00:00:00 2001 From: Camiel van Schoonhoven Date: Mon, 19 Jan 2026 15:34:56 -0800 Subject: [PATCH] chore: Remove Action Framework --- .../ContextPanel/Blocks/ActionBlock.test.tsx | 413 ++++++++---------- .../ContextPanel/Blocks/ActionBlock.tsx | 106 +---- 2 files changed, 191 insertions(+), 328 deletions(-) diff --git a/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx b/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx index 1f9c0ec40..85312070c 100644 --- a/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx +++ b/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx @@ -1,30 +1,30 @@ -import { - fireEvent, - render, - screen, - waitFor, - within, -} from "@testing-library/react"; +import { fireEvent, render, screen } from "@testing-library/react"; import { describe, expect, test, vi } from "vitest"; -import type { Action } from "./ActionBlock"; +import { Button } from "@/components/ui/button"; +import { Icon } from "@/components/ui/icon"; + import { ActionBlock } from "./ActionBlock"; describe("", () => { test("renders without title", () => { - const actions: Action[] = [ - { label: "Test Action", icon: "Copy", onClick: vi.fn() }, + const actions = [ + , ]; render(); expect(screen.queryByRole("heading")).not.toBeInTheDocument(); - expect(screen.getByTestId("action-Test Action")).toBeInTheDocument(); + expect(screen.getByTestId("test-action")).toBeInTheDocument(); }); test("renders with title", () => { - const actions: Action[] = [ - { label: "Test Action", icon: "Copy", onClick: vi.fn() }, + const actions = [ + , ]; render(); @@ -32,12 +32,14 @@ describe("", () => { expect( screen.getByRole("heading", { level: 3, name: "Actions" }), ).toBeInTheDocument(); - expect(screen.getByTestId("action-Test Action")).toBeInTheDocument(); + expect(screen.getByTestId("test-action")).toBeInTheDocument(); }); test("renders with custom className", () => { - const actions: Action[] = [ - { label: "Test Action", icon: "Copy", onClick: vi.fn() }, + const actions = [ + , ]; const { container } = render( @@ -51,21 +53,21 @@ describe("", () => { test("renders empty actions array without error", () => { const { container } = render(); - expect( - container.querySelector('[data-testid^="action-"]'), - ).not.toBeInTheDocument(); + expect(container.querySelector("button")).not.toBeInTheDocument(); }); describe("action rendering", () => { test("renders action button with icon", () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { label: "Copy Action", icon: "Copy", onClick }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Copy Action"); + const button = screen.getByTestId("copy-action"); expect(button).toBeInTheDocument(); const icon = button.querySelector("svg"); @@ -74,99 +76,119 @@ describe("", () => { }); test("renders action button with custom content", () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { label: "Custom Action", content: Custom, onClick }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Custom Action"); + const button = screen.getByTestId("custom-action"); expect(button).toBeInTheDocument(); - expect(within(button).getByText("Custom")).toBeInTheDocument(); + expect(screen.getByText("Custom Content")).toBeInTheDocument(); }); test("renders multiple actions", () => { - const actions: Action[] = [ - { label: "Action 1", icon: "Copy", onClick: vi.fn() }, - { label: "Action 2", icon: "Download", onClick: vi.fn() }, - { label: "Action 3", icon: "Trash", onClick: vi.fn() }, + const actions = [ + , + , + , ]; render(); - expect(screen.getByTestId("action-Action 1")).toBeInTheDocument(); - expect(screen.getByTestId("action-Action 2")).toBeInTheDocument(); - expect(screen.getByTestId("action-Action 3")).toBeInTheDocument(); + expect(screen.getByTestId("action-1")).toBeInTheDocument(); + expect(screen.getByTestId("action-2")).toBeInTheDocument(); + expect(screen.getByTestId("action-3")).toBeInTheDocument(); }); - test("renders ReactNode as action (backward compatibility)", () => { + test("renders custom ReactNode as action", () => { const actions = [ - { label: "Action 1", icon: "Copy" as const, onClick: vi.fn() }, - , + , +
+ Custom Node +
, ]; render(); - expect(screen.getByTestId("action-Action 1")).toBeInTheDocument(); - expect(screen.getByTestId("custom-button")).toBeInTheDocument(); + expect(screen.getByTestId("button-action")).toBeInTheDocument(); + expect(screen.getByTestId("custom-node")).toBeInTheDocument(); }); test("handles null or undefined actions gracefully", () => { const actions = [ - { label: "Action 1", icon: "Copy" as const, onClick: vi.fn() }, + , null, undefined, - { label: "Action 2", icon: "Download" as const, onClick: vi.fn() }, + , ]; render(); - expect(screen.getByTestId("action-Action 1")).toBeInTheDocument(); - expect(screen.getByTestId("action-Action 2")).toBeInTheDocument(); + expect(screen.getByTestId("action-1")).toBeInTheDocument(); + expect(screen.getByTestId("action-2")).toBeInTheDocument(); }); }); describe("action variants", () => { - test("renders destructive action with destructive variant", () => { - const actions: Action[] = [ - { label: "Delete", icon: "Trash", destructive: true, onClick: vi.fn() }, + test("renders destructive action variant", () => { + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Delete"); + const button = screen.getByTestId("delete-action"); expect(button).toHaveClass("bg-destructive"); expect(button).toHaveClass("text-white"); }); - test("renders non-destructive action with outline variant", () => { - const actions: Action[] = [ - { label: "Copy", icon: "Copy", onClick: vi.fn() }, + test("renders outline action variant", () => { + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Copy"); + const button = screen.getByTestId("copy-action"); expect(button).toHaveClass("border"); expect(button).toHaveClass("bg-background"); }); test("applies custom className to action button", () => { - const actions: Action[] = [ - { - label: "Styled Action", - icon: "Copy", - onClick: vi.fn(), - className: "custom-button", - }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Styled Action"); + const button = screen.getByTestId("styled-action"); expect(button).toHaveClass("custom-button"); }); }); @@ -174,13 +196,20 @@ describe("", () => { describe("action states", () => { test("renders disabled action", () => { const onClick = vi.fn(); - const actions: Action[] = [ - { label: "Disabled Action", icon: "Copy", disabled: true, onClick }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Disabled Action"); + const button = screen.getByTestId("disabled-action"); expect(button).toBeDisabled(); fireEvent.click(button); @@ -189,49 +218,58 @@ describe("", () => { test("renders enabled action", () => { const onClick = vi.fn(); - const actions: Action[] = [ - { label: "Enabled Action", icon: "Copy", disabled: false, onClick }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Enabled Action"); + const button = screen.getByTestId("enabled-action"); expect(button).not.toBeDisabled(); fireEvent.click(button); expect(onClick).toHaveBeenCalledTimes(1); }); - test("does not render hidden action", () => { - const actions: Action[] = [ - { label: "Visible Action", icon: "Copy", onClick: vi.fn() }, - { - label: "Hidden Action", - icon: "Trash", - hidden: true, - onClick: vi.fn(), - }, + test("conditionally renders actions based on logic", () => { + const showHidden = false; + const actions = [ + , + showHidden ? ( + + ) : null, ]; render(); - expect(screen.getByTestId("action-Visible Action")).toBeInTheDocument(); - expect( - screen.queryByTestId("action-Hidden Action"), - ).not.toBeInTheDocument(); + expect(screen.getByTestId("visible-action")).toBeInTheDocument(); + expect(screen.queryByTestId("hidden-action")).not.toBeInTheDocument(); }); }); describe("onClick behavior", () => { test("calls onClick when action is clicked", () => { const onClick = vi.fn(); - const actions: Action[] = [ - { label: "Click Action", icon: "Copy", onClick }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Click Action"); + const button = screen.getByTestId("click-action"); fireEvent.click(button); expect(onClick).toHaveBeenCalledTimes(1); @@ -239,13 +277,15 @@ describe("", () => { test("calls onClick multiple times when clicked multiple times", () => { const onClick = vi.fn(); - const actions: Action[] = [ - { label: "Multi Click", icon: "Copy", onClick }, + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Multi Click"); + const button = screen.getByTestId("multi-click"); fireEvent.click(button); fireEvent.click(button); fireEvent.click(button); @@ -254,177 +294,78 @@ describe("", () => { }); }); - describe("confirmation dialog", () => { - test("opens confirmation dialog when action has confirmation", () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { - label: "Delete", - icon: "Trash", - confirmation: "Are you sure you want to delete?", - onClick, - }, - ]; - - render(); - - const button = screen.getByTestId("action-Delete"); - fireEvent.click(button); - - // Dialog should appear - expect( - screen.getByText("Are you sure you want to delete?"), - ).toBeInTheDocument(); - expect(onClick).not.toHaveBeenCalled(); - }); - - test("executes onClick when confirmation is accepted", async () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { - label: "Delete", - icon: "Trash", - confirmation: "Confirm deletion?", - onClick, - }, - ]; - - render(); - - const button = screen.getByTestId("action-Delete"); - fireEvent.click(button); - - // Confirm in dialog - const confirmButton = screen.getByRole("button", { name: "Continue" }); - fireEvent.click(confirmButton); - - await waitFor(() => { - expect(onClick).toHaveBeenCalledTimes(1); - }); - }); - - test("does not execute onClick when confirmation is cancelled", async () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { - label: "Delete", - icon: "Trash", - confirmation: "Confirm deletion?", - onClick, - }, - ]; - - render(); - - const button = screen.getByTestId("action-Delete"); - fireEvent.click(button); - - // Cancel in dialog - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - fireEvent.click(cancelButton); - - await waitFor(() => { - expect(onClick).not.toHaveBeenCalled(); - }); - }); - - test("closes dialog after confirmation", async () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { - label: "Delete", - icon: "Trash", - confirmation: "Confirm deletion?", - onClick, - }, + describe("action keys", () => { + test("uses provided keys for actions", () => { + const actions = [ + , + , ]; - render(); - - const button = screen.getByTestId("action-Delete"); - fireEvent.click(button); - - expect(screen.getByText("Confirm deletion?")).toBeInTheDocument(); + const { container } = render(); - const confirmButton = screen.getByRole("button", { name: "Continue" }); - fireEvent.click(confirmButton); - - await waitFor(() => { - expect(screen.queryByText("Confirm deletion?")).not.toBeInTheDocument(); - }); + const actionWrappers = container.querySelectorAll("span[key]"); + expect(actionWrappers.length).toBeGreaterThanOrEqual(0); }); - test("closes dialog after cancellation", async () => { - const onClick = vi.fn(); - const actions: Action[] = [ - { - label: "Delete", - icon: "Trash", - confirmation: "Confirm deletion?", - onClick, - }, + test("generates keys for actions without keys", () => { + const actions = [ + // eslint-disable-next-line react/jsx-key + , + // eslint-disable-next-line react/jsx-key + , ]; render(); - const deleteButton = screen.getByTestId("action-Delete"); - fireEvent.click(deleteButton); - - expect(screen.getByText("Confirm deletion?")).toBeInTheDocument(); - - const cancelButton = screen.getByRole("button", { name: "Cancel" }); - fireEvent.click(cancelButton); - - await waitFor(() => { - expect(screen.queryByText("Confirm deletion?")).not.toBeInTheDocument(); - }); - - expect(screen.getByTestId("action-Delete")).toBeInTheDocument(); - }); - - test("executes onClick directly when no confirmation is provided", () => { - const onClick = vi.fn(); - const actions: Action[] = [{ label: "Copy", icon: "Copy", onClick }]; - - render(); - - const button = screen.getByTestId("action-Copy"); - fireEvent.click(button); - - expect(onClick).toHaveBeenCalledTimes(1); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(screen.getByTestId("action-1")).toBeInTheDocument(); + expect(screen.getByTestId("action-2")).toBeInTheDocument(); }); + }); - test("shows action label as dialog title", () => { - const actions: Action[] = [ - { - label: "Delete Item", - icon: "Trash", - confirmation: "Confirm?", - onClick: vi.fn(), - }, + describe("complex action scenarios", () => { + test("renders mixed action types", () => { + const actions = [ + , + + Link + , + + Text + , ]; render(); - const button = screen.getByTestId("action-Delete Item"); - fireEvent.click(button); - - expect(screen.getByText("Delete Item")).toBeInTheDocument(); - expect(screen.getByText("Confirm?")).toBeInTheDocument(); + expect(screen.getByTestId("button-action")).toBeInTheDocument(); + expect(screen.getByTestId("link-action")).toBeInTheDocument(); + expect(screen.getByTestId("text-action")).toBeInTheDocument(); }); - }); - describe("tooltip", () => { - test("action has tooltip with label", () => { - const actions: Action[] = [ - { label: "Copy to Clipboard", icon: "Copy", onClick: vi.fn() }, + test("renders actions with nested components", () => { + const actions = [ + , ]; render(); - const button = screen.getByTestId("action-Copy to Clipboard"); + const button = screen.getByTestId("nested-action"); expect(button).toBeInTheDocument(); + expect(button.querySelector("svg")).toBeInTheDocument(); + expect(screen.getByText("Download")).toBeInTheDocument(); }); }); }); diff --git a/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx b/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx index 74509a93f..02a69cfaf 100644 --- a/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx +++ b/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx @@ -1,30 +1,11 @@ -import { isValidElement, type ReactNode, useState } from "react"; +import { isValidElement, type ReactNode } from "react"; -import { Icon, type IconName } from "@/components/ui/icon"; import { BlockStack, InlineStack } from "@/components/ui/layout"; import { Heading } from "@/components/ui/typography"; -import TooltipButton from "../../Buttons/TooltipButton"; -import { ConfirmationDialog } from "../../Dialogs"; - -export type Action = { - label: string; - destructive?: boolean; - disabled?: boolean; - hidden?: boolean; - confirmation?: string; - onClick: () => void; - className?: string; -} & ( - | { icon: IconName; content?: never } - | { content: ReactNode; icon?: never } -); - -type ActionOrReactNode = Action | ReactNode; - interface ActionBlockProps { title?: string; - actions: ActionOrReactNode[]; + actions: ReactNode[]; className?: string; } @@ -33,77 +14,18 @@ export const ActionBlock = ({ actions, className, }: ActionBlockProps) => { - const [isOpen, setIsOpen] = useState(false); - const [dialogAction, setDialogAction] = useState(null); - - const openConfirmationDialog = (action: Action) => { - return () => { - setDialogAction(action); - setIsOpen(true); - }; - }; - - const handleConfirm = () => { - setIsOpen(false); - dialogAction?.onClick(); - setDialogAction(null); - }; - - const handleCancel = () => { - setIsOpen(false); - setDialogAction(null); - }; - return ( - <> - - {title && {title}} - - {actions.map((action, index) => { - if (!action || typeof action !== "object" || !("label" in action)) { - const key = - isValidElement(action) && action.key != null - ? `action-node-${String(action.key)}` - : `action-node-${index}`; - return {action}; - } - - if (action.hidden) { - return null; - } - - return ( - - {action.content === undefined && action.icon ? ( - - ) : ( - action.content - )} - - ); - })} - - - - - + + {title && {title}} + + {actions.map((action, index) => { + const key = + isValidElement(action) && action.key != null + ? `action-node-${String(action.key)}` + : `action-node-${index}`; + return {action}; + })} + + ); };