Skip to content

Commit 92ca0d1

Browse files
ammar-agentammar-agent
committed
🤖 fix: disable project removal when workspaces exist
- Remove project button is disabled when project has any workspaces (active or archived) - Shows explanatory tooltip based on workspace type: - 'Delete workspace first' (1 active workspace) - 'Delete all N workspaces first' (multiple active) - 'Delete N archived workspaces first' (archived only) - 'Delete N active + M archived workspaces first' (both) - Added archivedCountByProject to WorkspaceContext for efficient archived count tracking - Button uses aria-disabled and cursor-not-allowed styling when disabled - Fixed pre-existing lint issues in ArchivedWorkspaces.tsx
1 parent c5778f8 commit 92ca0d1

File tree

4 files changed

+105
-68
lines changed

4 files changed

+105
-68
lines changed

‎src/browser/components/ArchivedWorkspaces.tsx‎

Lines changed: 17 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ interface BulkOperationState {
2424
}
2525

2626
/** Group workspaces by time period for timeline display */
27-
function groupByTimePeriod(workspaces: FrontendWorkspaceMetadata[]): Map<string, FrontendWorkspaceMetadata[]> {
27+
function groupByTimePeriod(
28+
workspaces: FrontendWorkspaceMetadata[]
29+
): Map<string, FrontendWorkspaceMetadata[]> {
2830
const groups = new Map<string, FrontendWorkspaceMetadata[]>();
2931
const now = new Date();
3032
const today = new Date(now.getFullYear(), now.getMonth(), now.getDate());
@@ -67,7 +69,9 @@ function groupByTimePeriod(workspaces: FrontendWorkspaceMetadata[]): Map<string,
6769
}
6870

6971
/** Flatten grouped workspaces back to ordered array for index-based selection */
70-
function flattenGrouped(grouped: Map<string, FrontendWorkspaceMetadata[]>): FrontendWorkspaceMetadata[] {
72+
function flattenGrouped(
73+
grouped: Map<string, FrontendWorkspaceMetadata[]>
74+
): FrontendWorkspaceMetadata[] {
7175
const result: FrontendWorkspaceMetadata[] = [];
7276
for (const workspaces of grouped.values()) {
7377
result.push(...workspaces);
@@ -257,17 +261,13 @@ export const ArchivedWorkspaces: React.FC<ArchivedWorkspacesProps> = ({
257261
for (let i = 0; i < idsToRestore.length; i++) {
258262
const id = idsToRestore[i];
259263
const ws = workspaces.find((w) => w.id === id);
260-
setBulkOperation((prev) =>
261-
prev ? { ...prev, current: ws?.title ?? ws?.name ?? id } : prev
262-
);
264+
setBulkOperation((prev) => (prev ? { ...prev, current: ws?.title ?? ws?.name ?? id } : prev));
263265

264266
try {
265267
await unarchiveWorkspace(id);
266-
} catch (err) {
268+
} catch {
267269
setBulkOperation((prev) =>
268-
prev
269-
? { ...prev, errors: [...prev.errors, `Failed to restore ${ws?.name ?? id}`] }
270-
: prev
270+
prev ? { ...prev, errors: [...prev.errors, `Failed to restore ${ws?.name ?? id}`] } : prev
271271
);
272272
}
273273

@@ -292,17 +292,13 @@ export const ArchivedWorkspaces: React.FC<ArchivedWorkspacesProps> = ({
292292
for (let i = 0; i < idsToDelete.length; i++) {
293293
const id = idsToDelete[i];
294294
const ws = workspaces.find((w) => w.id === id);
295-
setBulkOperation((prev) =>
296-
prev ? { ...prev, current: ws?.title ?? ws?.name ?? id } : prev
297-
);
295+
setBulkOperation((prev) => (prev ? { ...prev, current: ws?.title ?? ws?.name ?? id } : prev));
298296

299297
try {
300298
await removeWorkspace(id, { force: true });
301-
} catch (err) {
299+
} catch {
302300
setBulkOperation((prev) =>
303-
prev
304-
? { ...prev, errors: [...prev.errors, `Failed to delete ${ws?.name ?? id}`] }
305-
: prev
301+
prev ? { ...prev, errors: [...prev.errors, `Failed to delete ${ws?.name ?? id}`] } : prev
306302
);
307303
}
308304

@@ -422,13 +418,13 @@ export const ArchivedWorkspaces: React.FC<ArchivedWorkspacesProps> = ({
422418
/>
423419
{workspaces.length > 3 && (
424420
<div className="relative flex-1">
425-
<Search className="text-muted pointer-events-none absolute left-2 top-1/2 h-4 w-4 -translate-y-1/2" />
421+
<Search className="text-muted pointer-events-none absolute top-1/2 left-2 h-4 w-4 -translate-y-1/2" />
426422
<input
427423
type="text"
428424
placeholder="Search archived workspaces..."
429425
value={searchQuery}
430426
onChange={(e) => setSearchQuery(e.target.value)}
431-
className="bg-bg-dark placeholder:text-muted text-foreground w-full rounded border border-transparent py-1.5 pl-8 pr-3 text-sm focus:border-border-light focus:outline-none"
427+
className="bg-bg-dark text-foreground placeholder:text-muted focus:border-border-light w-full rounded border border-transparent py-1.5 pr-3 pl-8 text-sm focus:outline-none"
432428
/>
433429
</div>
434430
)}
@@ -439,7 +435,7 @@ export const ArchivedWorkspaces: React.FC<ArchivedWorkspacesProps> = ({
439435
<div>
440436
{filteredWorkspaces.length === 0 ? (
441437
<div className="text-muted px-4 py-6 text-center text-sm">
442-
No workspaces match "{searchQuery}"
438+
No workspaces match &quot;{searchQuery}&quot;
443439
</div>
444440
) : (
445441
Array.from(groupedWorkspaces.entries()).map(([period, periodWorkspaces]) => (
@@ -468,7 +464,8 @@ export const ArchivedWorkspaces: React.FC<ArchivedWorkspacesProps> = ({
468464
type="checkbox"
469465
checked={isSelected}
470466
onClick={(e) => handleCheckboxClick(workspace.id, e)}
471-
onChange={() => {}} // Controlled by onClick for shift-click support
467+
// onChange is controlled by onClick for shift-click support
468+
readOnly
472469
className="h-4 w-4 rounded border-gray-600 bg-transparent"
473470
aria-label={`Select ${displayTitle}`}
474471
/>

‎src/browser/components/ProjectSidebar.tsx‎

Lines changed: 56 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,7 @@ const ProjectSidebarInner: React.FC<ProjectSidebarProps> = ({
200200
archiveWorkspace: onArchiveWorkspace,
201201
renameWorkspace: onRenameWorkspace,
202202
beginWorkspaceCreation: onAddWorkspace,
203+
archivedCountByProject,
203204
} = useWorkspaceContext();
204205

205206
// Get project state and operations from context
@@ -518,34 +519,61 @@ const ProjectSidebarInner: React.FC<ProjectSidebarProps> = ({
518519
</TooltipTrigger>
519520
<TooltipContent align="end">Manage secrets</TooltipContent>
520521
</Tooltip>
521-
<Tooltip>
522-
<TooltipTrigger asChild>
523-
<button
524-
onClick={(event) => {
525-
event.stopPropagation();
526-
const buttonElement = event.currentTarget;
527-
void (async () => {
528-
const result = await onRemoveProject(projectPath);
529-
if (!result.success) {
530-
const error = result.error ?? "Failed to remove project";
531-
const rect = buttonElement.getBoundingClientRect();
532-
const anchor = {
533-
top: rect.top + window.scrollY,
534-
left: rect.right + 10,
535-
};
536-
projectRemoveError.showError(projectPath, error, anchor);
537-
}
538-
})();
539-
}}
540-
aria-label={`Remove project ${projectName}`}
541-
data-project-path={projectPath}
542-
className="text-muted-dark hover:text-danger-light hover:bg-danger-light/10 mr-1 flex h-5 w-5 shrink-0 cursor-pointer items-center justify-center rounded-[3px] border-none bg-transparent text-base opacity-0 transition-all duration-200"
543-
>
544-
×
545-
</button>
546-
</TooltipTrigger>
547-
<TooltipContent align="end">Remove project</TooltipContent>
548-
</Tooltip>
522+
{(() => {
523+
const activeCount =
524+
sortedWorkspacesByProject.get(projectPath)?.length ?? 0;
525+
const archivedCount = archivedCountByProject.get(projectPath) ?? 0;
526+
const canDelete = activeCount === 0 && archivedCount === 0;
527+
528+
// Build tooltip message based on what needs deletion
529+
const getTooltip = () => {
530+
if (canDelete) return "Remove project";
531+
if (activeCount > 0 && archivedCount === 0) {
532+
return `Delete ${activeCount === 1 ? "workspace" : `all ${activeCount} workspaces`} first`;
533+
}
534+
if (activeCount === 0 && archivedCount > 0) {
535+
return `Delete ${archivedCount} archived workspace${archivedCount === 1 ? "" : "s"} first`;
536+
}
537+
return `Delete ${activeCount} active + ${archivedCount} archived workspaces first`;
538+
};
539+
540+
return (
541+
<Tooltip>
542+
<TooltipTrigger asChild>
543+
<button
544+
onClick={(event) => {
545+
event.stopPropagation();
546+
if (!canDelete) return;
547+
const buttonElement = event.currentTarget;
548+
void (async () => {
549+
const result = await onRemoveProject(projectPath);
550+
if (!result.success) {
551+
const error = result.error ?? "Failed to remove project";
552+
const rect = buttonElement.getBoundingClientRect();
553+
const anchor = {
554+
top: rect.top + window.scrollY,
555+
left: rect.right + 10,
556+
};
557+
projectRemoveError.showError(projectPath, error, anchor);
558+
}
559+
})();
560+
}}
561+
aria-label={`Remove project ${projectName}`}
562+
aria-disabled={!canDelete}
563+
data-project-path={projectPath}
564+
className={`mr-1 flex h-5 w-5 shrink-0 items-center justify-center rounded-[3px] border-none bg-transparent text-base opacity-0 transition-all duration-200 ${
565+
canDelete
566+
? "text-muted-dark hover:text-danger-light hover:bg-danger-light/10 cursor-pointer"
567+
: "text-muted cursor-not-allowed"
568+
}`}
569+
>
570+
×
571+
</button>
572+
</TooltipTrigger>
573+
<TooltipContent align="end">{getTooltip()}</TooltipContent>
574+
</Tooltip>
575+
);
576+
})()}
549577
<button
550578
onClick={(event) => {
551579
event.stopPropagation();

‎src/browser/contexts/WorkspaceContext.tsx‎

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@ function ensureCreatedAt(metadata: FrontendWorkspaceMetadata): void {
8686
export interface WorkspaceContext {
8787
// Workspace data
8888
workspaceMetadata: Map<string, FrontendWorkspaceMetadata>;
89+
/** Count of archived workspaces per project path */
90+
archivedCountByProject: Map<string, number>;
8991
loading: boolean;
9092

9193
// Workspace operations
@@ -160,6 +162,9 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
160162
);
161163
const [loading, setLoading] = useState(true);
162164
const [pendingNewWorkspaceProject, setPendingNewWorkspaceProject] = useState<string | null>(null);
165+
const [archivedCountByProject, setArchivedCountByProject] = useState<Map<string, number>>(
166+
new Map()
167+
);
163168

164169
// Manage selected workspace internally with localStorage persistence
165170
const [selectedWorkspace, setSelectedWorkspace] = usePersistedState<WorkspaceSelection | null>(
@@ -178,7 +183,11 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
178183
if (!api) return false; // Return false to indicate metadata wasn't loaded
179184
try {
180185
const includePostCompaction = isExperimentEnabled(EXPERIMENT_IDS.POST_COMPACTION_CONTEXT);
181-
const metadataList = await api.workspace.list({ includePostCompaction });
186+
// Fetch active and archived workspaces in parallel
187+
const [metadataList, archivedList] = await Promise.all([
188+
api.workspace.list({ includePostCompaction }),
189+
api.workspace.list({ archived: true }),
190+
]);
182191
console.log(
183192
"[WorkspaceContext] Loaded metadata list:",
184193
metadataList.map((m) => ({ id: m.id, name: m.name, title: m.title }))
@@ -193,10 +202,20 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
193202
metadataMap.set(metadata.id, metadata);
194203
}
195204
setWorkspaceMetadata(metadataMap);
205+
206+
// Compute archived counts by project
207+
const archivedCounts = new Map<string, number>();
208+
for (const ws of archivedList) {
209+
const count = archivedCounts.get(ws.projectPath) ?? 0;
210+
archivedCounts.set(ws.projectPath, count + 1);
211+
}
212+
setArchivedCountByProject(archivedCounts);
213+
196214
return true; // Return true to indicate metadata was loaded
197215
} catch (error) {
198216
console.error("Failed to load workspace metadata:", error);
199217
setWorkspaceMetadata(new Map());
218+
setArchivedCountByProject(new Map());
200219
return true; // Still return true - we tried to load, just got empty result
201220
}
202221
}, [setWorkspaceMetadata, api]);
@@ -606,6 +625,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
606625
const value = useMemo<WorkspaceContext>(
607626
() => ({
608627
workspaceMetadata,
628+
archivedCountByProject,
609629
loading,
610630
createWorkspace,
611631
removeWorkspace,
@@ -623,6 +643,7 @@ export function WorkspaceProvider(props: WorkspaceProviderProps) {
623643
}),
624644
[
625645
workspaceMetadata,
646+
archivedCountByProject,
626647
loading,
627648
createWorkspace,
628649
removeWorkspace,

‎src/browser/stories/App.errors.stories.tsx‎

Lines changed: 10 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -248,12 +248,12 @@ export const LargeDiff: AppStory = {
248248
};
249249

250250
/**
251-
* Project removal error popover.
251+
* Project removal disabled when workspaces exist.
252252
*
253-
* Shows the error popup when attempting to remove a project that has active workspaces.
254-
* The play function hovers the project and clicks the remove button to trigger the error.
253+
* The remove button is disabled and shows an explanatory tooltip when a project
254+
* has active or archived workspaces. The play function hovers to reveal the disabled state.
255255
*/
256-
export const ProjectRemovalError: AppStory = {
256+
export const ProjectRemovalDisabled: AppStory = {
257257
render: () => (
258258
<AppWithMocks
259259
setup={() => {
@@ -268,11 +268,6 @@ export const ProjectRemovalError: AppStory = {
268268
return createMockORPCClient({
269269
projects: groupWorkspacesByProject(workspaces),
270270
workspaces,
271-
onProjectRemove: () => ({
272-
success: false,
273-
error:
274-
"Cannot remove project with active workspaces. Please remove all 2 workspace(s) first.",
275-
}),
276271
});
277272
}}
278273
/>
@@ -297,16 +292,12 @@ export const ProjectRemovalError: AppStory = {
297292
// Small delay for hover state to apply
298293
await new Promise((r) => setTimeout(r, 100));
299294

300-
// Click the remove button
301-
await userEvent.click(removeButton);
295+
// Verify the button is disabled
296+
if (removeButton.getAttribute("aria-disabled") !== "true") {
297+
throw new Error("Remove button should be disabled when project has multiple workspaces");
298+
}
302299

303-
// Wait for the error popover to appear
304-
await waitFor(
305-
() => {
306-
const errorPopover = document.querySelector('[role="alert"]');
307-
if (!errorPopover) throw new Error("Error popover not found");
308-
},
309-
{ timeout: 2000 }
310-
);
300+
// Hover the disabled button to trigger tooltip
301+
await userEvent.hover(removeButton);
311302
},
312303
};

0 commit comments

Comments
 (0)