Skip to content

feat: add extra workspace actions in the workspaces table #17775

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
May 13, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Refactor duplicate and clean up code
  • Loading branch information
BrunoQuaresma committed May 12, 2025
commit f7e9ba9ac27d83340f09757d92983cc28a5470bb
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,18 @@ import { Link as RouterLink } from "react-router-dom";
import { ChangeWorkspaceVersionDialog } from "./ChangeWorkspaceVersionDialog";
import { WorkspaceDeleteDialog } from "./WorkspaceDeleteDialog";
import type { WorkspacePermissions } from "../permissions";
import { useWorkspaceDuplication } from "pages/CreateWorkspacePage/useWorkspaceDuplication";

type WorkspaceMoreActionsProps = {
workspace: Workspace;
isDuplicationReady: boolean;
permissions: WorkspacePermissions;
disabled?: boolean;
Copy link
Member

@Parkreiner Parkreiner May 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two points of feedback on this prop:

  1. I don't think a disabled prop like this should ever be optional. I think that whoever is using this component should be forced to think though when it's allowed to be used. Otherwise, there's a risk that someone forgets the prop, and that'd let the user do an operation on a workspace while it's going through a state transition (which I don't think is ever valid)
  2. But I think that we can actually do better, because even with the prop being required, there's still this race condition that I was able to reproduce reliably:
image The dropdown is able to be open while the button is disabled. Steps to reproduce:
  1. Find a workspace that's running
  2. Go into the workspace details page
  3. Stop the workspace
  4. Quickly go back to the Workspaces directory page, and click the More Actions button
  5. The UI will eventually update to prevent the dropdown from opening, but if it's already open, there's nothing in place to make sure it closes

You have more context than I do, but I was thinking:

  1. We remove the disabled prop entirely
  2. We make it so that when a workspace is having a transition, the MoreActions component unmounts entirely. That way, the user still can't click a button they shouldn't be able to, but that also causes any open dropdowns and messages to close (because they no longer exist)
  3. If the user had a dropdown open already, we display a toast message explaining why the dropdown suddenly closed without them doing anything

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I guess if we want to make sure the dots always appear for aesthetic consistency, no matter what transitions we have, another option is to make it so that the dropdowns have conditional rendering tied to the disabled prop

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be solved not in the component level, but in the data level. What I want to say is, when stopping the workspace, or triggering a build, we should revalidate the workspaces data to represent the correct state of each workspace. I'm going to take a look into that 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol, update the data is more complex than I thought because of pagination so I will just close the menu when the button becomes disabled.

onDuplicate: () => void;
};

export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
workspace,
disabled,
permissions,
isDuplicationReady,
onDuplicate,
}) => {
const queryClient = useQueryClient();

Expand All @@ -58,6 +55,10 @@ export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
deleteWorkspace(workspace, queryClient),
);

// Duplicate
const { duplicateWorkspace, isDuplicationReady } =
useWorkspaceDuplication(workspace);

return (
<>
<DropdownMenu>
Expand Down Expand Up @@ -94,7 +95,7 @@ export const WorkspaceMoreActions: FC<WorkspaceMoreActionsProps> = ({
)}

<DropdownMenuItem
onClick={onDuplicate}
onClick={duplicateWorkspace}
disabled={!isDuplicationReady}
>
<CopyIcon />
Expand Down
59 changes: 26 additions & 33 deletions site/src/pages/WorkspacePage/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,43 +28,33 @@ import type { WorkspacePermissions } from "../../modules/workspaces/permissions"
import { resourceOptionValue, useResourcesNav } from "./useResourcesNav";

export interface WorkspaceProps {
handleStart: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
handleStop: () => void;
handleRestart: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
handleUpdate: () => void;
handleCancel: () => void;
handleSettings: () => void;
handleDormantActivate: () => void;
handleToggleFavorite: () => void;
workspace: TypesGen.Workspace;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate you reorganizing these!

template: TypesGen.Template;
permissions: WorkspacePermissions;
isUpdating: boolean;
isRestarting: boolean;
workspace: TypesGen.Workspace;
hideSSHButton?: boolean;
hideVSCodeDesktopButton?: boolean;
buildInfo?: TypesGen.BuildInfoResponse;
sshPrefix?: string;
template: TypesGen.Template;
canDebugMode: boolean;
handleRetry: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
handleDebug: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
buildLogs?: TypesGen.ProvisionerJobLog[];
latestVersion?: TypesGen.TemplateVersion;
permissions: WorkspacePermissions;
timings?: TypesGen.WorkspaceBuildTimings;
handleStart: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
handleStop: () => void;
handleRestart: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
handleUpdate: () => void;
handleCancel: () => void;
handleDormantActivate: () => void;
handleToggleFavorite: () => void;
handleRetry: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
handleDebug: (buildParameters?: TypesGen.WorkspaceBuildParameter[]) => void;
}

/**
* Workspace is the top-level component for viewing an individual workspace
*/
export const Workspace: FC<WorkspaceProps> = ({
handleStart,
handleStop,
handleRestart,
handleUpdate,
handleCancel,
handleSettings,
handleDormantActivate,
handleToggleFavorite,
workspace,
isUpdating,
isRestarting,
Expand All @@ -73,13 +63,19 @@ export const Workspace: FC<WorkspaceProps> = ({
buildInfo,
sshPrefix,
template,
canDebugMode,
handleRetry,
handleDebug,
buildLogs,
latestVersion,
permissions,
timings,
handleStart,
handleStop,
handleRestart,
handleUpdate,
handleCancel,
handleDormantActivate,
handleToggleFavorite,
handleRetry,
handleDebug,
}) => {
const navigate = useNavigate();
const theme = useTheme();
Expand Down Expand Up @@ -136,23 +132,20 @@ export const Workspace: FC<WorkspaceProps> = ({
>
<WorkspaceTopbar
workspace={workspace}
template={template}
permissions={permissions}
latestVersion={latestVersion}
isUpdating={isUpdating}
isRestarting={isRestarting}
handleStart={handleStart}
handleStop={handleStop}
handleRestart={handleRestart}
handleUpdate={handleUpdate}
handleCancel={handleCancel}
handleSettings={handleSettings}
handleRetry={handleRetry}
handleDebug={handleDebug}
handleDormantActivate={handleDormantActivate}
handleToggleFavorite={handleToggleFavorite}
canDebugMode={canDebugMode}
isUpdating={isUpdating}
isRestarting={isRestarting}
canUpdateWorkspace={permissions.updateWorkspace}
template={template}
permissions={permissions}
latestVersion={latestVersion}
/>

<div
Expand Down
85 changes: 24 additions & 61 deletions site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx
Original file line number Diff line number Diff line change
@@ -1,23 +1,10 @@
import DownloadOutlined from "@mui/icons-material/DownloadOutlined";
import DuplicateIcon from "@mui/icons-material/FileCopyOutlined";
import SettingsIcon from "@mui/icons-material/SettingsOutlined";
import type { Workspace, WorkspaceBuildParameter } from "api/typesGenerated";
import { Button } from "components/Button/Button";
import {
DropdownMenu,
DropdownMenuContent,
DropdownMenuItem,
DropdownMenuSeparator,
DropdownMenuTrigger,
} from "components/DropdownMenu/DropdownMenu";
import { useAuthenticated } from "hooks/useAuthenticated";
import { EllipsisVertical } from "lucide-react";
import {
type ActionType,
abilitiesByWorkspaceStatus,
} from "modules/workspaces/actions";
import { useWorkspaceDuplication } from "pages/CreateWorkspacePage/useWorkspaceDuplication";
import { type FC, Fragment, type ReactNode, useState } from "react";
import { type FC, Fragment, type ReactNode } from "react";
import { mustUpdateWorkspace } from "utils/workspace";
import {
ActivateButton,
Expand All @@ -33,56 +20,57 @@ import {
} from "./Buttons";
import { DebugButton } from "./DebugButton";
import { RetryButton } from "./RetryButton";
import { WorkspaceMoreActions } from "modules/workspaces/WorkspaceMoreActions/WorkspaceMoreActions";
import type { WorkspacePermissions } from "modules/workspaces/permissions";

export interface WorkspaceActionsProps {
workspace: Workspace;
isUpdating: boolean;
isRestarting: boolean;
permissions: WorkspacePermissions;
handleToggleFavorite: () => void;
handleStart: (buildParameters?: WorkspaceBuildParameter[]) => void;
handleStop: () => void;
handleRestart: (buildParameters?: WorkspaceBuildParameter[]) => void;
handleUpdate: () => void;
handleCancel: () => void;
handleSettings: () => void;
handleRetry: (buildParameters?: WorkspaceBuildParameter[]) => void;
handleDebug: (buildParameters?: WorkspaceBuildParameter[]) => void;
handleDormantActivate: () => void;
isUpdating: boolean;
isRestarting: boolean;
children?: ReactNode;
canChangeVersions: boolean;
canDebug: boolean;
}

export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
workspace,
isUpdating,
isRestarting,
permissions,
handleToggleFavorite,
handleStart,
handleStop,
handleRestart,
handleUpdate,
handleCancel,
handleSettings,
handleRetry,
handleDebug,
handleDormantActivate,
isUpdating,
isRestarting,
canChangeVersions,
canDebug,
}) => {
const { duplicateWorkspace, isDuplicationReady } =
useWorkspaceDuplication(workspace);

const { user } = useAuthenticated();
const isOwner =
user.roles.find((role) => role.name === "owner") !== undefined;
const { actions, canCancel, canAcceptJobs } = abilitiesByWorkspaceStatus(
workspace,
{ canDebug, isOwner },
{ canDebug: permissions.deploymentConfig, isOwner },
);

const mustUpdate = mustUpdateWorkspace(workspace, canChangeVersions);
const tooltipText = getTooltipText(workspace, mustUpdate, canChangeVersions);
const mustUpdate = mustUpdateWorkspace(
workspace,
permissions.updateWorkspaceVersion,
);
const tooltipText = getTooltipText(
workspace,
mustUpdate,
permissions.updateWorkspaceVersion,
);

// A mapping of button type to the corresponding React component
const buttonMapping: Record<ActionType, ReactNode> = {
Expand Down Expand Up @@ -170,36 +158,11 @@ export const WorkspaceActions: FC<WorkspaceActionsProps> = ({
onToggle={handleToggleFavorite}
/>

<DropdownMenu>
<DropdownMenuTrigger asChild>
<Button
size="icon-lg"
variant="subtle"
aria-label="Workspace actions"
data-testid="workspace-options-button"
aria-controls="workspace-options"
disabled={!canAcceptJobs}
>
<EllipsisVertical aria-hidden="true" />
<span className="sr-only">Workspace actions</span>
</Button>
</DropdownMenuTrigger>

<DropdownMenuContent id="workspace-options" align="end">
<DropdownMenuItem onClick={handleSettings}>
<SettingsIcon />
Settings
</DropdownMenuItem>

<DropdownMenuItem
onClick={duplicateWorkspace}
disabled={!isDuplicationReady}
>
<DuplicateIcon />
Duplicate&hellip;
</DropdownMenuItem>
</DropdownMenuContent>
</DropdownMenu>
<WorkspaceMoreActions
workspace={workspace}
permissions={permissions}
disabled={!canAcceptJobs}
/>
</div>
);
};
Expand Down
Loading