From 7c248f0a1d1d697d5bb9ee89250f0ce18f7af385 Mon Sep 17 00:00:00 2001 From: Camiel van Schoonhoven Date: Fri, 12 Dec 2025 10:22:42 -0800 Subject: [PATCH] Rework Pipeline Action Buttons into Action Framework --- .../Editor/Context/PipelineDetails.tsx | 82 ++++++++++--- .../Editor/Context/RenamePipeline.tsx | 64 ---------- .../ContextPanel/Blocks/ActionBlock.test.tsx | 28 ----- .../ContextPanel/Blocks/ActionBlock.tsx | 17 +-- .../shared/Dialogs/PipelineNameDialog.tsx | 109 +++++++++++------- 5 files changed, 138 insertions(+), 162 deletions(-) delete mode 100644 src/components/Editor/Context/RenamePipeline.tsx diff --git a/src/components/Editor/Context/PipelineDetails.tsx b/src/components/Editor/Context/PipelineDetails.tsx index 96deefde8..8519d58ba 100644 --- a/src/components/Editor/Context/PipelineDetails.tsx +++ b/src/components/Editor/Context/PipelineDetails.tsx @@ -1,24 +1,30 @@ +import { useLocation, useNavigate } from "@tanstack/react-router"; import { useEffect, useState } from "react"; import { useValidationIssueNavigation } from "@/components/Editor/hooks/useValidationIssueNavigation"; -import TooltipButton from "@/components/shared/Buttons/TooltipButton"; import { CodeViewer } from "@/components/shared/CodeViewer"; -import { ActionBlock } from "@/components/shared/ContextPanel/Blocks/ActionBlock"; +import { + type Action, + ActionBlock, +} from "@/components/shared/ContextPanel/Blocks/ActionBlock"; import { ContentBlock } from "@/components/shared/ContextPanel/Blocks/ContentBlock"; import { ListBlock } from "@/components/shared/ContextPanel/Blocks/ListBlock"; import { TextBlock } from "@/components/shared/ContextPanel/Blocks/TextBlock"; import { CopyText } from "@/components/shared/CopyText/CopyText"; -import { Icon } from "@/components/ui/icon"; +import { PipelineNameDialog } from "@/components/shared/Dialogs"; import { BlockStack } from "@/components/ui/layout"; import useToastNotification from "@/hooks/useToastNotification"; import { useComponentSpec } from "@/providers/ComponentSpecProvider"; -import { getComponentFileFromList } from "@/utils/componentStore"; +import { APP_ROUTES } from "@/routes/router"; +import { + getComponentFileFromList, + renameComponentFileInList, +} from "@/utils/componentStore"; import { USER_PIPELINES_LIST_NAME } from "@/utils/constants"; import { componentSpecToText } from "@/utils/yaml"; import PipelineIO from "../../shared/Execution/PipelineIO"; import { PipelineValidationList } from "./PipelineValidationList"; -import RenamePipeline from "./RenamePipeline"; const PipelineDetails = () => { const notify = useToastNotification(); @@ -27,12 +33,22 @@ const PipelineDetails = () => { digest, isComponentTreeValid, globalValidationIssues, + saveComponentSpec, } = useComponentSpec(); + const notify = useToastNotification(); + const navigate = useNavigate(); + + const location = useLocation(); + const pathname = location.pathname; + + const title = componentSpec?.name; + const { handleIssueClick, groupedIssues } = useValidationIssueNavigation( globalValidationIssues, ); + const [isRenameDialogOpen, setIsRenameDialogOpen] = useState(false); const [isYamlFullscreen, setIsYamlFullscreen] = useState(false); // State for file metadata @@ -42,6 +58,31 @@ const PipelineDetails = () => { createdBy?: string; }>({}); + const isSubmitDisabled = (name: string) => { + return name === title; + }; + + const handleTitleUpdate = async (name: string) => { + if (!componentSpec) { + notify("Update failed: ComponentSpec not found", "error"); + return; + } + + await renameComponentFileInList( + USER_PIPELINES_LIST_NAME, + title ?? "", + name, + pathname, + ); + + await saveComponentSpec(name); + + const urlName = encodeURIComponent(name); + const url = APP_ROUTES.PIPELINE_EDITOR.replace("$name", urlName); + + navigate({ to: url }); + }; + // Fetch file metadata on mount or when componentSpec.name changes useEffect(() => { const fetchMeta = async () => { @@ -86,16 +127,17 @@ const PipelineDetails = () => { componentSpec.metadata?.annotations || {}, ).map(([key, value]) => ({ label: key, value: String(value) })); - const actions = [ - , - setIsYamlFullscreen(true)} - key="view-yaml-action" - > - - , + const actions: Action[] = [ + { + label: "Rename Pipeline", + icon: "PencilLine", + onClick: () => setIsRenameDialogOpen(true), + }, + { + label: "View YAML", + icon: "FileCodeCorner", + onClick: () => setIsYamlFullscreen(true), + }, ]; return ( @@ -151,6 +193,16 @@ const PipelineDetails = () => { onClose={() => setIsYamlFullscreen(false)} /> )} + ); }; diff --git a/src/components/Editor/Context/RenamePipeline.tsx b/src/components/Editor/Context/RenamePipeline.tsx deleted file mode 100644 index 787342578..000000000 --- a/src/components/Editor/Context/RenamePipeline.tsx +++ /dev/null @@ -1,64 +0,0 @@ -import { useLocation, useNavigate } from "@tanstack/react-router"; -import { Edit3 } from "lucide-react"; - -import TooltipButton from "@/components/shared/Buttons/TooltipButton"; -import { PipelineNameDialog } from "@/components/shared/Dialogs"; -import useToastNotification from "@/hooks/useToastNotification"; -import { useComponentSpec } from "@/providers/ComponentSpecProvider"; -import { APP_ROUTES } from "@/routes/router"; -import { renameComponentFileInList } from "@/utils/componentStore"; -import { USER_PIPELINES_LIST_NAME } from "@/utils/constants"; - -const RenamePipeline = () => { - const { componentSpec, saveComponentSpec } = useComponentSpec(); - const notify = useToastNotification(); - const navigate = useNavigate(); - - const location = useLocation(); - const pathname = location.pathname; - - const title = componentSpec?.name; - - const isSubmitDisabled = (name: string) => { - return name === title; - }; - - const handleTitleUpdate = async (name: string) => { - if (!componentSpec) { - notify("Update failed: ComponentSpec not found", "error"); - return; - } - - await renameComponentFileInList( - USER_PIPELINES_LIST_NAME, - title ?? "", - name, - pathname, - ); - - await saveComponentSpec(name); - - const urlName = encodeURIComponent(name); - const url = APP_ROUTES.PIPELINE_EDITOR.replace("$name", urlName); - - navigate({ to: url }); - }; - - return ( - - - - } - title="Name Pipeline" - description="Unsaved pipeline changes will be lost." - initialName={title ?? ""} - onSubmit={handleTitleUpdate} - submitButtonText="Update Title" - isSubmitDisabled={isSubmitDisabled} - /> - ); -}; - -export default RenamePipeline; diff --git a/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx b/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx index 1f9c0ec40..13a17e47d 100644 --- a/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx +++ b/src/components/shared/ContextPanel/Blocks/ActionBlock.test.tsx @@ -99,34 +99,6 @@ describe("", () => { expect(screen.getByTestId("action-Action 2")).toBeInTheDocument(); expect(screen.getByTestId("action-Action 3")).toBeInTheDocument(); }); - - test("renders ReactNode as action (backward compatibility)", () => { - const actions = [ - { label: "Action 1", icon: "Copy" as const, onClick: vi.fn() }, - , - ]; - - render(); - - expect(screen.getByTestId("action-Action 1")).toBeInTheDocument(); - expect(screen.getByTestId("custom-button")).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(); - }); }); describe("action variants", () => { diff --git a/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx b/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx index 54cb6afe1..f36eda70b 100644 --- a/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx +++ b/src/components/shared/ContextPanel/Blocks/ActionBlock.tsx @@ -1,4 +1,4 @@ -import { isValidElement, type ReactNode, useState } from "react"; +import { type ReactNode, useState } from "react"; import { Icon, type IconName } from "@/components/ui/icon"; import { BlockStack, InlineStack } from "@/components/ui/layout"; @@ -20,12 +20,9 @@ export type Action = { | { content: ReactNode; icon?: never } ); -// Temporary: ReactNode included for backward compatibility with some existing buttons. In the long-term we should strive for only Action types. -export type ActionOrReactNode = Action | ReactNode; - interface ActionBlockProps { title?: string; - actions: ActionOrReactNode[]; + actions: Action[]; className?: string; } @@ -60,15 +57,7 @@ export const ActionBlock = ({ {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}; - } - + {actions.map((action) => { if (action.hidden) { return null; } diff --git a/src/components/shared/Dialogs/PipelineNameDialog.tsx b/src/components/shared/Dialogs/PipelineNameDialog.tsx index 33b3c7cab..cd710db4f 100644 --- a/src/components/shared/Dialogs/PipelineNameDialog.tsx +++ b/src/components/shared/Dialogs/PipelineNameDialog.tsx @@ -23,8 +23,7 @@ import { Input } from "@/components/ui/input"; import { BlockStack } from "@/components/ui/layout"; import useLoadUserPipelines from "@/hooks/useLoadUserPipelines"; -interface PipelineNameDialogProps { - trigger: ReactNode; +type PipelineNameDialogPropsBase = { title: string; description?: string; initialName: string; @@ -32,11 +31,25 @@ interface PipelineNameDialogProps { submitButtonIcon?: ReactNode; onSubmit: (name: string) => void; isSubmitDisabled?: (name: string, error: string | null) => boolean; - onOpenChange?: (open: boolean) => void; -} +}; + +type PipelineNameDialogProps = PipelineNameDialogPropsBase & + ( + | { + trigger: ReactNode; + open?: never; + onOpenChange?: never; + } + | { + trigger?: never; + open: boolean; + onOpenChange: (open: boolean) => void; + } + ); const PipelineNameDialog = ({ trigger, + open, title, description = "Please, name your pipeline.", initialName, @@ -78,14 +91,14 @@ const PipelineNameDialog = ({ ); const handleDialogOpenChange = useCallback( - (open: boolean) => { - if (!open) { + (newOpen: boolean) => { + if (!newOpen) { setError(null); } else { setName(initialName); refetchUserPipelines(); } - onOpenChange?.(open); + onOpenChange?.(newOpen); }, [initialName, onOpenChange, refetchUserPipelines], ); @@ -100,43 +113,57 @@ const PipelineNameDialog = ({ !name || !!isSubmitDisabled?.(name, error); + const dialogContent = ( + + + {title} + {description} + + + + + + + {error} + + + + + + + + + + + + + ); + + // Controlled mode + if (open !== undefined) { + return ( + + {dialogContent} + + ); + } + + // Uncontrolled mode return ( {trigger} - - - {title} - {description} - - - - - - - {error} - - - - - - - - - - - - + {dialogContent} ); };