Skip to content

Commit 6b84d09

Browse files
authored
chore: Cleanup Task Action Buttons (#1633)
## Description <!-- Please provide a brief description of the changes made in this pull request. Include any relevant context or reasoning for the changes. --> Extract Task Actions into individual Button Components and then render them conditionally within `TaskActions`. Currently the definition of actions on a task is scattered across multiple different files in an ever-growing `actions` array. This change simplifies things greatly: actions are defined as their own button and then rendered via `TaskActions`. This change separates concerns for all the different task actions and substantially reduces the amount of code lingering about in `TaskNodeCard`, `TaskOverview`, `TaskActions` and `ComponentDetailsDialog`. Note: this change renders the general Action Framework created in #1470 somewhat obsolete. However, since the framework was initially created to support ReactNodes for backward-compatibility not change is actually needed to the framework. For now it can be left in place and in the future we can choose to use it or remove it. Next Steps: Pipeline & Run Actions ## Related Issue and Pull requests <!-- Link to any related issues using the format #<issue-number> --> Supersedes #1534 Related to Shopify/oasis-frontend#401 ## Type of Change <!-- Please delete options that are not relevant --> - [x] Cleanup/Refactor ## Checklist <!-- Please ensure the following are completed before submitting the PR --> - [ ] I have tested this does not break current pipelines / runs functionality - [ ] I have tested the changes on staging ## Screenshots (if applicable) <!-- Include any screenshots that might help explain the changes or provide visual context --> No change to UI. ## Test Instructions <!-- Detail steps and prerequisites for testing the changes in this PR --> Check that every button in the task details panel and component details dialog works as expected. ## Additional Comments <!-- Add any additional context or information that reviewers might need to know regarding this PR -->
1 parent 80ffdbe commit 6b84d09

File tree

18 files changed

+452
-385
lines changed

18 files changed

+452
-385
lines changed
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { type ReactNode } from "react";
2+
3+
import { Icon, type IconName } from "@/components/ui/icon";
4+
5+
import TooltipButton from "./TooltipButton";
6+
7+
type ActionButtonProps = {
8+
label: string;
9+
destructive?: boolean;
10+
disabled?: boolean;
11+
onClick: () => void;
12+
className?: string;
13+
} & (
14+
| { icon: IconName; children?: never }
15+
| { children: ReactNode; icon?: never }
16+
);
17+
18+
export const ActionButton = ({
19+
label,
20+
destructive,
21+
disabled,
22+
onClick,
23+
className,
24+
icon,
25+
children,
26+
}: ActionButtonProps) => {
27+
return (
28+
<TooltipButton
29+
data-testid={`action-${label}`}
30+
variant={destructive ? "destructive" : "outline"}
31+
tooltip={label}
32+
onClick={onClick}
33+
disabled={disabled}
34+
className={className}
35+
>
36+
{children === undefined && icon ? <Icon name={icon} /> : children}
37+
</TooltipButton>
38+
);
39+
};

src/components/shared/Buttons/TooltipButton.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
TooltipTrigger,
99
} from "@/components/ui/tooltip";
1010

11-
export interface TooltipButtonProps extends ButtonProps {
11+
interface TooltipButtonProps extends ButtonProps {
1212
tooltip: React.ReactNode;
1313
tooltipSide?: "top" | "right" | "bottom" | "left";
1414
tooltipAlign?: "start" | "center" | "end";

src/components/shared/Buttons/ViewYamlButton.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,13 @@
11
import { useState } from "react";
22

3-
import { Icon } from "@/components/ui/icon";
43
import type {
54
ComponentSpec,
65
HydratedComponentReference,
76
} from "@/utils/componentSpec";
87
import { getComponentName } from "@/utils/getComponentName";
98

109
import TaskImplementation from "../TaskDetails/Implementation";
11-
import TooltipButton from "./TooltipButton";
10+
import { ActionButton } from "./ActionButton";
1211

1312
type ViewYamlButtonProps =
1413
| { componentRef: HydratedComponentReference; componentSpec?: never }
@@ -34,13 +33,11 @@ export const ViewYamlButton = ({
3433

3534
return (
3635
<>
37-
<TooltipButton
38-
variant="outline"
39-
tooltip="View YAML"
36+
<ActionButton
37+
label="View YAML"
38+
icon="FileCodeCorner"
4039
onClick={handleClick}
41-
>
42-
<Icon name="FileCodeCorner" />
43-
</TooltipButton>
40+
/>
4441

4542
{showCodeViewer && (
4643
<TaskImplementation

src/components/shared/ContextPanel/Blocks/ActionBlock.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ export type Action = {
2020
| { content: ReactNode; icon?: never }
2121
);
2222

23-
// Temporary: ReactNode included for backward compatibility with some existing buttons. In the long-term we should strive for only Action types.
24-
export type ActionOrReactNode = Action | ReactNode;
23+
type ActionOrReactNode = Action | ReactNode;
2524

2625
interface ActionBlockProps {
2726
title?: string;

src/components/shared/Dialogs/ComponentDetailsDialog.tsx

Lines changed: 30 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@ import { useHydrateComponentReference } from "@/hooks/useHydrateComponentReferen
1717
import type { ComponentReference } from "@/utils/componentSpec";
1818

1919
import InfoIconButton from "../Buttons/InfoIconButton";
20-
import TooltipButton from "../Buttons/TooltipButton";
21-
import { ComponentEditorDialog } from "../ComponentEditor/ComponentEditorDialog";
2220
import { ComponentFavoriteToggle } from "../FavoriteComponentToggle";
2321
import { InfoBox } from "../InfoBox";
2422
import { PublishComponent } from "../ManageComponent/PublishComponent";
@@ -32,9 +30,7 @@ interface ComponentDetailsProps {
3230
component: ComponentReference;
3331
displayName: string;
3432
trigger?: ReactNode;
35-
actions?: ReactNode[];
3633
onClose?: () => void;
37-
onDelete?: () => void;
3834
}
3935

4036
const ComponentDetailsDialogContentSkeleton = () => {
@@ -64,12 +60,7 @@ const ComponentDetailsDialogContentSkeleton = () => {
6460
};
6561

6662
const ComponentDetailsDialogContent = withSuspenseWrapper(
67-
({
68-
component,
69-
displayName,
70-
actions = [],
71-
onDelete,
72-
}: ComponentDetailsProps) => {
63+
({ component, displayName }: ComponentDetailsProps) => {
7364
const remoteComponentLibrarySearchEnabled = useBetaFlagValue(
7465
"remote-component-library-search",
7566
);
@@ -84,7 +75,7 @@ const ComponentDetailsDialogContent = withSuspenseWrapper(
8475
);
8576
}
8677

87-
const { url, spec: componentSpec, digest: componentDigest } = componentRef;
78+
const componentSpec = componentRef.spec;
8879

8980
const hasPublishSection =
9081
remoteComponentLibrarySearchEnabled && component.owned;
@@ -133,14 +124,7 @@ const ComponentDetailsDialogContent = withSuspenseWrapper(
133124
<PublishedComponentDetails component={componentRef} />
134125
) : null}
135126

136-
<TaskDetails
137-
displayName={displayName}
138-
componentRef={componentRef}
139-
componentDigest={componentDigest}
140-
url={url}
141-
actions={actions}
142-
onDelete={onDelete}
143-
/>
127+
<TaskDetails componentRef={componentRef} />
144128
</TabsContent>
145129

146130
<TabsContent value="io" className="h-full">
@@ -175,18 +159,11 @@ const ComponentDetails = ({
175159
component,
176160
displayName,
177161
trigger,
178-
actions = [],
179162
onClose,
180-
onDelete,
181163
}: ComponentDetailsProps) => {
182-
const hasEnabledInAppEditor = useBetaFlagValue("in-app-component-editor");
183-
184-
const [isEditDialogOpen, setIsEditDialogOpen] = useState(false);
185164
const [open, setOpen] = useState(false);
186165
const dialogTriggerButton = trigger || <InfoIconButton />;
187166

188-
const componentText = component.text;
189-
190167
const dialogContextValue = useMemo(
191168
() => ({
192169
name: "ComponentDetails",
@@ -197,79 +174,42 @@ const ComponentDetails = ({
197174
[],
198175
);
199176

200-
const handleCloseEditDialog = useCallback(() => {
201-
setIsEditDialogOpen(false);
202-
}, []);
203-
204177
const onOpenChange = useCallback((open: boolean) => {
205178
setOpen(open);
206179
if (!open) {
207180
onClose?.();
208181
}
209182
}, []);
210183

211-
const handleEditComponent = useCallback(() => {
212-
setIsEditDialogOpen(true);
213-
}, []);
214-
215-
const actionsWithEdit = useMemo(() => {
216-
if (!hasEnabledInAppEditor) return actions;
217-
218-
const EditButton = (
219-
<TooltipButton
220-
variant="secondary"
221-
onClick={handleEditComponent}
222-
tooltip="Edit Component Definition"
223-
key={`${displayName}-edit-button`}
224-
>
225-
<Icon name="FilePenLine" />
226-
</TooltipButton>
227-
);
228-
229-
return [...actions, EditButton];
230-
}, [actions, hasEnabledInAppEditor, handleEditComponent]);
231-
232184
return (
233-
<>
234-
<Dialog modal open={open} onOpenChange={onOpenChange}>
235-
<DialogTrigger asChild>{dialogTriggerButton}</DialogTrigger>
236-
237-
<DialogDescription
238-
className="hidden"
239-
aria-label={`${displayName} component details`}
240-
>
241-
{`${displayName} component details`}
242-
</DialogDescription>
243-
<DialogContent
244-
className="max-w-2xl min-w-2xl overflow-hidden"
245-
aria-label={`${displayName} component details`}
246-
>
247-
<DialogHeader>
248-
<DialogTitle className="flex items-center gap-2 mr-5">
249-
<span>{displayName}</span>
250-
<ComponentFavoriteToggle component={component} />
251-
</DialogTitle>
252-
</DialogHeader>
185+
<Dialog modal open={open} onOpenChange={onOpenChange}>
186+
<DialogTrigger asChild>{dialogTriggerButton}</DialogTrigger>
253187

254-
<DialogContext.Provider value={dialogContextValue}>
255-
<ComponentDetailsDialogContent
256-
component={component}
257-
displayName={displayName}
258-
trigger={dialogTriggerButton}
259-
actions={actionsWithEdit}
260-
onClose={onClose}
261-
onDelete={onDelete}
262-
/>
263-
</DialogContext.Provider>
264-
</DialogContent>
265-
</Dialog>
266-
{isEditDialogOpen && (
267-
<ComponentEditorDialog
268-
text={componentText}
269-
onClose={handleCloseEditDialog}
270-
/>
271-
)}
272-
</>
188+
<DialogDescription
189+
className="hidden"
190+
aria-label={`${displayName} component details`}
191+
>
192+
{`${displayName} component details`}
193+
</DialogDescription>
194+
<DialogContent
195+
className="max-w-2xl min-w-2xl overflow-hidden"
196+
aria-label={`${displayName} component details`}
197+
>
198+
<DialogHeader>
199+
<DialogTitle className="flex items-center gap-2 mr-5">
200+
<span>{displayName}</span>
201+
<ComponentFavoriteToggle component={component} />
202+
</DialogTitle>
203+
</DialogHeader>
204+
205+
<DialogContext.Provider value={dialogContextValue}>
206+
<ComponentDetailsDialogContent
207+
component={component}
208+
displayName={displayName}
209+
/>
210+
</DialogContext.Provider>
211+
</DialogContent>
212+
</Dialog>
273213
);
274214
};
275215

0 commit comments

Comments
 (0)