fix v2 sidebar nav-away on workspace delete#3771
Conversation
The dashboard sidebar's navigate-away check used useParams({ strict:
false }) from a layout-level component, which didn't reliably surface
the leaf route's workspaceId — so deletion silently no-op'd the nav and
left the user on the doomed workspace's URL. Switch to useMatchRoute
(the same pattern the sidebar item uses for isActive).
|
Caution Review failedPull request was closed or merged during review 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 (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 fixes a silent no-op in the "navigate away from a deleted workspace" logic by replacing Confidence Score: 5/5Safe to merge — the fix is minimal, correct, and follows the established pattern in the codebase. The change replaces a fragile No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/DashboardSidebar/hooks/useNavigateAwayFromWorkspace/useNavigateAwayFromWorkspace.ts | Replaces unreliable useParams({ strict: false }) with useMatchRoute() for detecting the current workspace; aligns with the existing isActive pattern in the sidebar item actions hook. |
Sequence Diagram
sequenceDiagram
participant User
participant DestroyDialog as useDestroyDialogState
participant NavAway as useNavigateAwayFromWorkspace
participant Router as TanStack Router
User->>DestroyDialog: confirm delete
DestroyDialog->>NavAway: navigateAway(workspaceId)
NavAway->>Router: matchRoute({ to: "/v2-workspace/$workspaceId", params: { workspaceId }, fuzzy: true })
Router-->>NavAway: isViewingWorkspace (true/false)
alt isViewingWorkspace = true
NavAway->>Router: navigateToV2Workspace(next) or navigate("/")
Router-->>User: redirected to sibling or home
else isViewingWorkspace = false
NavAway-->>DestroyDialog: no-op (user is on a different workspace)
end
DestroyDialog->>DestroyDialog: onOpenChange(false), markDeleting, destroy()
Reviews (1): Last reviewed commit: "fix v2 sidebar nav-away on workspace del..." | Re-trigger Greptile
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
useNavigateAwayFromWorkspaceuseduseParams({ strict: false })from a layout-level component, which didn't reliably surface the leaf route'sworkspaceId. The early-returnif (params.workspaceId !== workspaceId) return;was firing even when the user was viewing the doomed workspace.useMatchRoute({ to: "/v2-workspace/\$workspaceId", params, fuzzy: true })— the same patternuseDashboardSidebarWorkspaceItemActionsalready uses forisActive.run()inuseDestroyDialogState(before the dialog closes /markDeleting/destroyfires), so the user moves off the doomed URL as soon as they confirm.Test plan
/.Summary by cubic
Fixes v2 sidebar navigation when deleting the current workspace. We now detect the active workspace reliably and navigate to the next workspace (or
/) immediately after confirm, so users aren’t left on a deleted URL.useParams({ strict: false })withuseMatchRouteagainst"/v2-workspace/$workspaceId"to correctly detect the active workspace.navigateToV2Workspace, or to/if none exist.Written for commit 253cccf. Summary will update on new commits.
Summary by CodeRabbit