feat(desktop): focus neighbor workspace after v2 delete#3401
Merged
Conversation
When the active v2 workspace is deleted, navigate to the previous workspace in visual order, falling back to the next, then to / if no neighbors exist. Mirrors v1 behavior (without the wrap-around). Also switches the delete flow to toast.promise for feedback.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughRemoved component-level delete pending state and moved deletion flow into the hook: delete now closes the dialog immediately, performs the TRPC delete with toast.promise, computes a focus target from flattened workspace IDs, and navigates to a neighboring workspace when the deleted workspace was active. Changes
Sequence DiagramsequenceDiagram
actor User
participant Dialog as Delete Dialog
participant Hook as Delete Hook
participant Collections as Collections/State
participant API as TRPC Delete
participant Nav as Navigation
User->>Dialog: confirm delete
Dialog->>Hook: handleDelete()
Hook->>Collections: getFlattenedV2WorkspaceIds()
Collections-->>Hook: flattened list
Hook->>Hook: getDeleteFocusTargetWorkspaceId(flattened, deletedId)
Hook->>Dialog: close dialog
Hook->>API: trigger delete mutation (promise)
Hook->>Collections: remove workspace from sidebar
API-->>Hook: delete result
alt deleted workspace was active
Hook->>Nav: navigateToV2Workspace(focusTargetId or "/")
else
Hook->>Nav: no navigation
end
Hook-->>User: toast.promise shows status
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR improves the v2 workspace delete UX in two ways: it navigates to the visually adjacent workspace (previous → next → Key implementation details:
Confidence Score: 5/5Safe to merge — the core neighbor-navigation logic is correct across all edge cases and the refactor to toast.promise is clean. The getFlattenedV2WorkspaceIds and getDeleteFocusTargetWorkspaceId utilities correctly handle all described scenarios (middle, first, last, only workspace, cross-project). The pre-deletion snapshot approach is intentional and sound. The only concern is a P2: if an in-app navigation unexpectedly rejects after a successful server delete, the toast message is misleading. This is an extremely unlikely scenario that does not affect the primary delete path. No files require special attention; the one P2 suggestion is in useDashboardSidebarWorkspaceItemActions.ts around the navigation error propagation. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as User
participant D as DeleteDialog
participant H as handleDelete
participant C as collections (snapshot)
participant T as toast.promise
participant API as apiTrpcClient
participant S as useDashboardSidebarState
participant R as TanStack Router
U->>D: Click "Confirm Delete"
D->>H: handleDelete()
H->>C: getFlattenedV2WorkspaceIds(collections)
C-->>H: [w1, w2, ..., wN] (pre-deletion snapshot)
H->>H: getDeleteFocusTargetWorkspaceId(ids, workspaceId) → focusTargetId
H->>D: setIsDeleteDialogOpen(false) — dialog closes immediately
H->>T: toast.promise(deletePromise, { loading, success, error })
T-->>U: "Deleting workspace..." toast
H->>API: v2Workspace.delete.mutate({ id })
API-->>H: success
H->>S: removeWorkspaceFromSidebar(workspaceId)
alt isActive && focusTargetId
H->>R: navigateToV2Workspace(focusTargetId)
else isActive && no neighbor
H->>R: navigate({ to: "/" })
else not active
Note over H: no navigation needed
end
H-->>T: promise resolves
T-->>U: "Workspace deleted" toast
Reviews (1): Last reviewed commit: "feat(desktop): focus neighbor workspace ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+82
to
+99
| const deletePromise = (async () => { | ||
| await apiTrpcClient.v2Workspace.delete.mutate({ id: workspaceId }); | ||
| removeWorkspaceFromSidebar(workspaceId); | ||
| setIsDeleteDialogOpen(false); | ||
| toast.success("Workspace deleted"); | ||
| if (isActive) { | ||
| navigate({ to: "/" }); | ||
| if (focusTargetId) { | ||
| await navigateToV2Workspace(focusTargetId, navigate); | ||
| } else { | ||
| await navigate({ to: "/" }); | ||
| } | ||
| } | ||
| } catch (error) { | ||
| toast.error( | ||
| })(); | ||
|
|
||
| toast.promise(deletePromise, { | ||
| loading: "Deleting workspace...", | ||
| success: "Workspace deleted", | ||
| error: (error) => | ||
| `Failed to delete: ${error instanceof Error ? error.message : "Unknown error"}`, | ||
| ); | ||
| } finally { | ||
| setIsDeleting(false); | ||
| } | ||
| }); |
There was a problem hiding this comment.
The deletePromise IIFE chains removeWorkspaceFromSidebar and the navigation call after the server mutate. If the mutate succeeds but navigateToV2Workspace (or navigate({ to: "/" })) throws, deletePromise rejects and toast.promise will display:
"Failed to delete: <navigation error>"
…even though the workspace was already deleted on the server and removed from the sidebar. In practice, in-app TanStack Router navigations almost never throw, so the risk is very low — but if it does happen the user could believe the workspace still exists and retry the delete unnecessarily.
One option is to isolate the navigation so its failure doesn't propagate to the promise used by toast.promise:
Suggested change
| const deletePromise = (async () => { | |
| await apiTrpcClient.v2Workspace.delete.mutate({ id: workspaceId }); | |
| removeWorkspaceFromSidebar(workspaceId); | |
| setIsDeleteDialogOpen(false); | |
| toast.success("Workspace deleted"); | |
| if (isActive) { | |
| navigate({ to: "/" }); | |
| if (focusTargetId) { | |
| await navigateToV2Workspace(focusTargetId, navigate); | |
| } else { | |
| await navigate({ to: "/" }); | |
| } | |
| } | |
| } catch (error) { | |
| toast.error( | |
| })(); | |
| toast.promise(deletePromise, { | |
| loading: "Deleting workspace...", | |
| success: "Workspace deleted", | |
| error: (error) => | |
| `Failed to delete: ${error instanceof Error ? error.message : "Unknown error"}`, | |
| ); | |
| } finally { | |
| setIsDeleting(false); | |
| } | |
| }); | |
| const deletePromise = (async () => { | |
| await apiTrpcClient.v2Workspace.delete.mutate({ id: workspaceId }); | |
| removeWorkspaceFromSidebar(workspaceId); | |
| })(); | |
| toast.promise(deletePromise, { | |
| loading: "Deleting workspace...", | |
| success: "Workspace deleted", | |
| error: (error) => | |
| `Failed to delete: ${error instanceof Error ? error.message : "Unknown error"}`, | |
| }); | |
| void deletePromise.then(() => { | |
| if (!isActive) return; | |
| if (focusTargetId) { | |
| void navigateToV2Workspace(focusTargetId, navigate); | |
| } else { | |
| void navigate({ to: "/" }); | |
| } | |
| }); |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
1 issue found across 6 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/utils/getFlattenedV2WorkspaceIds/getFlattenedV2WorkspaceIds.ts">
<violation number="1" location="apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/utils/getFlattenedV2WorkspaceIds/getFlattenedV2WorkspaceIds.ts:48">
P2: Sort top-level items with the same section-before-workspace tie-breaker as the sidebar, otherwise delete can focus the wrong neighbor when `tabOrder` values collide.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
- Isolate navigation from toast.promise so a router rejection after a successful delete doesn't surface a misleading "Failed to delete" toast (greptile). - Break sort ties in getFlattenedV2WorkspaceIds with section-before- workspace to match the sidebar's ordering when tabOrder collides (cubic).
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
MocA-Love
pushed a commit
to MocA-Love/superset
that referenced
this pull request
Apr 13, 2026
…3401) * feat(desktop): focus neighbor workspace after v2 delete When the active v2 workspace is deleted, navigate to the previous workspace in visual order, falling back to the next, then to / if no neighbors exist. Mirrors v1 behavior (without the wrap-around). Also switches the delete flow to toast.promise for feedback. * fix(desktop): address PR feedback on v2 delete focus - Isolate navigation from toast.promise so a router rejection after a successful delete doesn't surface a misleading "Failed to delete" toast (greptile). - Break sort ties in getFlattenedV2WorkspaceIds with section-before- workspace to match the sidebar's ordering when tabOrder collides (cubic).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/if no neighbors exist. Matches v1 behavior (minus the wrap-around).toast.promiseso the user gets a loading → success/error toast instead of a manual try/catch.isDeletingstate since the dialog is dismissed immediately and toast owns the feedback.Neighbor lookup reads directly from TanStack DB / ElectricSQL collection snapshots (`v2SidebarProjects`, `v2SidebarSections`, `v2WorkspaceLocalState`) at click time via the new `getFlattenedV2WorkspaceIds` util — no React context or prop drilling.
Test plan
Summary by cubic
Focuses the nearest neighbor workspace after deleting the active v2 workspace, falling back to
/if none. Also switches the delete flow totoast.promisefor clear loading/success/error feedback.New Features
/. No wrap-around.v2SidebarProjects,v2SidebarSections, andv2WorkspaceLocalStateviagetFlattenedV2WorkspaceIds/getDeleteFocusTargetWorkspaceId. Delete dialog closes immediately;isDeletingremoved. Deleting a non-active workspace doesn’t change focus.Bug Fixes
toast.promise) to avoid false failure toasts; tie-breaker sets section-before-workspace whentabOrdermatches to mirror sidebar order.Written for commit 089f2c1. Summary will update on new commits.
Summary by CodeRabbit
Improvements
New Features