feat: add inline actions into workspaces table #17636

BrunoQuaresma · 2025-05-01T16:53:00Z

Related to #17311

This PR adds inline actions in the workspaces page. It is a bit different of the original design because I'm splitting the work into three phases that I will explain in more details in the demo.

Screen.Recording.2025-05-01.at.09.50.50.mp4

…e-actions

Copilot

Pull Request Overview

This PR introduces inline actions to the workspaces table while refactoring how workspace data is queried and updated. Key changes include:

Refactoring the useWorkspacesData hook to accept a WorkspacesRequest object and updating query keys.
Implementing a new WorkspaceActionsCell with inline action buttons and associated success/error callbacks.
Removing redundant isOwner props by computing ownership via the authenticated user, and updating related update/cancellation hooks.

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
site/src/pages/WorkspacesPage/data.ts	Refactors workspace data fetching to use WorkspacesRequest and updates the queryKey handling.
site/src/pages/WorkspacesPage/WorkspacesTable.tsx	Introduces the WorkspaceActionsCell component and updates table cell layout.
site/src/pages/WorkspacesPage/WorkspacesPageView.tsx	Passes inline action callbacks to the table component.
site/src/pages/WorkspacesPage/WorkspacesPage.tsx	Updates query parameter naming and adds action success/error callbacks.
site/src/pages/WorkspacePage/WorkspaceTopbar.tsx	Removes the isOwner prop; ownership is now derived via authentication.
site/src/pages/WorkspacePage/WorkspaceReadyPage.tsx	Switches to the new workspace update hook and renders WorkspaceUpdateDialogs.
site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx	Refactors cancel button logic by computing isOwner locally and replacing the showCancel variable.
site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.stories.tsx	Adds an auth provider for story consistency.
site/src/modules/workspaces/useWorkspaceUpdate.tsx	Implements a new hook for workspace update flows with confirmation/missing parameter dialogs.
site/src/modules/workspaces/actions.ts	Adjusts abilities logic to take a permissions object instead of a boolean for canDebug.
site/src/hooks/usePagination.ts	Replaces manual offset calculation with the calcOffset helper function.
site/src/components/Dialogs/Dialog.tsx	Adds stopPropagation logic to cancel button clicks to avoid undesired events.
site/src/api/queries/workspaces.ts	Updates queryFn to pass full request config to the API.

Comments suppressed due to low confidence (2)

site/src/pages/WorkspacePage/WorkspaceTopbar.tsx:55

[nitpick] Since the isOwner prop is removed and ownership is now derived from the authenticated user, ensure that all child components relying on 'isOwner' are updated accordingly to prevent any unexpected UI behavior.

 // Removed: isOwner prop

site/src/pages/WorkspacePage/WorkspaceActions/WorkspaceActions.tsx:174

[nitpick] Replacing the 'showCancel' condition with 'canCancel' changes the criteria for displaying the CancelButton; please verify that this aligns with the intended cancellation permissions and workspace state logic.

{canCancel && <CancelButton handleAction={handleCancel} />}

site/src/api/queries/workspaces.ts

…e-actions

Parkreiner

Looks good to me overall! Just had a few questions/comments on some small details

site/src/api/queries/workspaces.ts

Parkreiner · 2025-05-02T18:43:10Z

site/src/hooks/usePagination.ts

@@ -23,3 +23,7 @@ export const usePagination = ({
 		offset,
 	};
 };
+
+export const calcOffset = (page: number, limit: number) => {
+	return page <= 0 ? 0 : (page - 1) * limit;


This can be turned into Math.max(0, limit * (page - 1))

Also, do we want to put this function in this file? It's being imported by a lot of files that don't care about the hook

Parkreiner · 2025-05-02T18:45:07Z

site/src/modules/workspaces/actions.ts

@@ -58,7 +66,7 @@ export const abilitiesByWorkspaceStatus = (
 		case "starting": {
 			return {
 				actions: ["starting"],
-				canCancel: true,
+				canCancel: true && hasPermissionToCancel,


The true literals can be removed here and for stopping. If you AND a boolean with true, you always get back the original value

Parkreiner · 2025-05-02T18:47:52Z

site/src/modules/workspaces/useWorkspaceUpdate.tsx

+	missingBuildParameters: MissingBuildParametersDialogProps;
+};
+
+export const WorkspaceUpdateDialogs: FC<WorkspaceUpdateDialogsProps> = ({


Because this file is exporting components that aren't simple pass-through components like context providers, I feel like it's probably better to rename the file to put more emphasis on the components over the hook (could probably name it WorkspaceUpdateDialogs?). The hook just feels like an implementation detail

Especially since this isn't the first useWorkspaceUpdate in the codebase

Parkreiner · 2025-05-02T19:03:56Z

site/src/modules/workspaces/useWorkspaceUpdate.tsx

+	};
+
+	return {
+		update,


I'm not opposed to having a general function that handles both the confirmed case and the non-confirmed case, but I'd prefer for that to be kept as an implementation detail. I feel like there's a risk of accidentally calling update the wrong way if we return it out directly

I think it'd be better if we did something like this:

update: () => update()

This also makes sure that the types can't lie to us

Parkreiner · 2025-05-02T19:40:25Z

site/src/modules/workspaces/useWorkspaceUpdate.tsx

+	return (
+		<>
+			<UpdateConfirmationDialog {...updateConfirmation} />
+			<MissingBuildParametersDialog {...missingBuildParameters} />
+		</>


I'm also wondering: do we want to make sure that both dialogs are mounted together? I feel like passing two separate, custom objects as props is a bit much, but I don't know if we get away with splitting them up as separate exports

Parkreiner · 2025-05-02T19:51:39Z

site/src/pages/WorkspacesPage/WorkspacesPage.test.tsx

@@ -264,7 +264,7 @@ describe("WorkspacesPage", () => {
 		await user.click(getWorkspaceCheckbox(workspaces[0]));
 		await user.click(getWorkspaceCheckbox(workspaces[1]));
 		await user.click(screen.getByRole("button", { name: /actions/i }));
-		const stopButton = await screen.findByText(/stop/i);
+		const stopButton = await screen.findByRole("menuitem", { name: /stop/i });


Appreciate the more accessible selectors!

Parkreiner · 2025-05-02T20:09:51Z

site/src/pages/WorkspacesPage/WorkspacesTable.tsx

+	return (
+		<TableCell>
+			<div className="flex gap-1 justify-end">
+				{abilities.actions.includes("start") && (


Wondering if it might be worthwhile to throw all of these into a .map callback. We're iterating over abilities.actions three times, which shouldn't be a huge deal, but a .map could let us iterate over everything exactly once

Parkreiner · 2025-05-02T20:17:59Z

site/src/modules/workspaces/useWorkspaceUpdate.tsx

+	};
+};
+
+export const useWorkspaceUpdate = ({


Actually, now that I think about it, do we need to export the custom hook? We could keep it as an implementation detail, but couldn't we update WorkspaceUpdateDialogs to accept the hook inputs directly, and then have it hook everything up?

BrunoQuaresma added 6 commits May 1, 2025 12:47

feat: add inline actions into workspaces table

0ba8372

Fix is retrying

25d986e

Refactor actions

50d9d74

Simplify permissions check

a20a188

Handle action errors

6f054b9

Refactor workspace update action

fcc5a27

github-actions bot assigned BrunoQuaresma May 1, 2025

Merge branch 'main' of https://github.com/coder/coder into bq/add-bas…

10dcd68

…e-actions

BrunoQuaresma requested a review from Copilot May 1, 2025 16:56

Copilot AI reviewed May 1, 2025

View reviewed changes

site/src/api/queries/workspaces.ts Show resolved Hide resolved

BrunoQuaresma and others added 11 commits May 1, 2025 17:00

Run fmt

766463d

Merge branch 'main' of https://github.com/coder/coder into bq/add-bas…

c14442c

…e-actions

Fix WorkspacesPage tests

281cf1a

Run FMT

71c6369

Fix WorkspacePage tests

3502a22

Fix storybook failed tests

7d2b8b3

Fix one more storybook test

61dc162

Fixes

ddcb1e9

Adjust skeleton

53c4332

Align inline actions to the right

86337e2

Merge branch 'main' into bq/add-base-actions

821f9d0

BrunoQuaresma marked this pull request as ready for review May 2, 2025 17:48

BrunoQuaresma requested review from a team and Parkreiner and removed request for a team May 2, 2025 17:48

Parkreiner approved these changes May 2, 2025

View reviewed changes

Parkreiner reviewed May 2, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add inline actions into workspaces table #17636

feat: add inline actions into workspaces table #17636

BrunoQuaresma commented May 1, 2025 •

edited

Loading

Copilot AI left a comment

Parkreiner left a comment

Parkreiner May 2, 2025

Parkreiner May 2, 2025 •

edited

Loading

Parkreiner May 2, 2025

Parkreiner May 2, 2025

Parkreiner May 2, 2025

Parkreiner May 2, 2025

Parkreiner May 2, 2025

Parkreiner May 2, 2025

Parkreiner May 2, 2025

Parkreiner May 2, 2025

feat: add inline actions into workspaces table #17636

Are you sure you want to change the base?

feat: add inline actions into workspaces table #17636

Conversation

BrunoQuaresma commented May 1, 2025 • edited Loading

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Parkreiner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Parkreiner May 2, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BrunoQuaresma commented May 1, 2025 •

edited

Loading

Parkreiner May 2, 2025 •

edited

Loading