feat(desktop): show pick-workspace prompt on cross-version routes#4211
feat(desktop): show pick-workspace prompt on cross-version routes#4211
Conversation
When the active version (v1 vs v2) doesn't match the current workspace route, swap the content Outlet for a "Pick a workspace" placeholder so users land on the version-correct sidebar instead of an empty/broken page.
|
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 (3)
📝 WalkthroughWalkthroughThe PR introduces cross-version mismatch detection in the dashboard. A new component displays a workspace selection prompt when the active workspace route version (V1 or V2) conflicts with the ChangesCross-Version Mismatch State
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 SummaryIntroduces a
Confidence Score: 3/5The content-area guard works correctly, but the CLOSE_WORKSPACE hotkey and workspace tRPC query remain active during the mismatch state, which can surface the delete dialog for a workspace the user has no visible path to. The new component and the mismatch detection logic are sound. The gap is that versionMismatch only gates Outlet — it does not disable the CLOSE_WORKSPACE hotkey, which is enabled whenever currentWorkspaceId is non-null. In the v2-mode + v1-route scenario, currentWorkspaceId comes from the v1 route match, so the hotkey is active and can open DeleteWorkspaceDialog for a workspace the user should not be operating on from this state. apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx — the hotkey enabled guard and the workspaces.get query enabled condition both need && !versionMismatch.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx | Adds v2 route matching and versionMismatch logic to conditionally render CrossVersionMismatchState; CLOSE_WORKSPACE hotkey remains enabled during mismatch, and the workspace query fires unnecessarily when mismatched on a v1 route. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/CrossVersionMismatchState/CrossVersionMismatchState.tsx | New empty-state component rendering 'Pick a workspace' placeholder with an icon; straightforward and correct. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/CrossVersionMismatchState/index.ts | Barrel re-export for the new component; no issues. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[DashboardLayout renders] --> B{isV2CloudEnabled?}
B -- Yes --> C{onV1WorkspaceRoute?\n/workspace/:id matched}
B -- No --> D{onV2WorkspaceRoute?\n/v2-workspace/:id matched}
C -- Yes --> E[versionMismatch = true]
C -- No --> F[versionMismatch = false]
D -- Yes --> E
D -- No --> F
E --> G[Render CrossVersionMismatchState\n'Pick a workspace']
F --> H[Render Outlet\nnormal workspace content]
G --> I[Sidebar still shows\nversion-correct workspace list]
H --> I
Comments Outside Diff (2)
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx, line 91-103 (link)CLOSE_WORKSPACE hotkey fires through the mismatch state
When the user is in v2 mode and lands on a
/workspace/$idroute,currentWorkspaceIdis still non-null (derived fromcurrentWorkspaceMatch), so this hotkey is enabled. If the user triggers it and theworkspaces.getquery has resolved,deleteTargetgets set andDeleteWorkspaceDialogopens for a workspace they can't navigate to from the current sidebar. The mismatch guard only gates the content<Outlet />, not this side-effect. Adding&& !versionMismatchto theenabledoption would prevent the hotkey from being active while the placeholder is shown.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx Line: 91-103 Comment: **CLOSE_WORKSPACE hotkey fires through the mismatch state** When the user is in v2 mode and lands on a `/workspace/$id` route, `currentWorkspaceId` is still non-null (derived from `currentWorkspaceMatch`), so this hotkey is enabled. If the user triggers it and the `workspaces.get` query has resolved, `deleteTarget` gets set and `DeleteWorkspaceDialog` opens for a workspace they can't navigate to from the current sidebar. The mismatch guard only gates the content `<Outlet />`, not this side-effect. Adding `&& !versionMismatch` to the `enabled` option would prevent the hotkey from being active while the placeholder is shown. How can I resolve this? If you propose a fix, please make it concise.
-
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx, line 55-58 (link)When the version mismatch is active on a v1 route (v2 mode),
currentWorkspaceIdis non-null so this query fires and fetches workspace data that is never shown. Gating with!versionMismatchavoids the unnecessary network call.Prompt To Fix With AI
This is a comment left during a code review. Path: apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx Line: 55-58 Comment: When the version mismatch is active on a v1 route (v2 mode), `currentWorkspaceId` is non-null so this query fires and fetches workspace data that is never shown. Gating with `!versionMismatch` avoids the unnecessary network call. How can I resolve this? If you propose a fix, please make it concise.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx:91-103
**CLOSE_WORKSPACE hotkey fires through the mismatch state**
When the user is in v2 mode and lands on a `/workspace/$id` route, `currentWorkspaceId` is still non-null (derived from `currentWorkspaceMatch`), so this hotkey is enabled. If the user triggers it and the `workspaces.get` query has resolved, `deleteTarget` gets set and `DeleteWorkspaceDialog` opens for a workspace they can't navigate to from the current sidebar. The mismatch guard only gates the content `<Outlet />`, not this side-effect. Adding `&& !versionMismatch` to the `enabled` option would prevent the hotkey from being active while the placeholder is shown.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/_dashboard/layout.tsx:55-58
When the version mismatch is active on a v1 route (v2 mode), `currentWorkspaceId` is non-null so this query fires and fetches workspace data that is never shown. Gating with `!versionMismatch` avoids the unnecessary network call.
```suggestion
const { data: currentWorkspace } = electronTrpc.workspaces.get.useQuery(
{ id: currentWorkspaceId ?? "" },
{ enabled: !!currentWorkspaceId && !versionMismatch },
);
```
Reviews (1): Last reviewed commit: "feat(desktop): show pick-workspace promp..." | Re-trigger Greptile
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
<Outlet />for a "Pick a workspace" placeholder so users see the version-correct sidebar list instead of a v1 page rendering inside a v2 shell (or vice versa).Why / Context
The dashboard sidebar shows v1 or v2 entries based on
useIsV2CloudEnabled, but the router still resolves the previously-visited/workspace/$idor/v2-workspace/$idroute after a version flip. The user lands on a workspace they can't navigate to from the current sidebar, with no obvious next step.How It Works
DashboardLayoutalready callsuseMatchRoutefor/workspace/$workspaceId. Added a parallel match for/v2-workspace/$workspaceIdand aversionMismatchboolean — true when the active version disagrees with the matched route.CrossVersionMismatchState(a small empty-state with the existingselect-text cursor-texttreatment) instead of<Outlet />. The sidebar is untouched, so picking any workspace from it routes the user back to a matching shell.Manual QA Checklist
v2 active, v1 workspace route
/workspace/$idURL (e.g. via back button after toggling)v1 active, v2 workspace route
/v2-workspace/$idURLNo regression on matching routes
/v2-workspace/$id→ workspace renders as before/workspace/$id→ workspace renders as beforeTesting
bun run lint:fix(clean)Summary by cubic
Show a "Pick a workspace" placeholder when the active version (v1/v2) doesn't match the current workspace route. In this case,
DashboardLayoutrendersCrossVersionMismatchStateinstead of the<Outlet />, keeping the sidebar intact so users can pick a workspace from the correct version.Written for commit a73988f. Summary will update on new commits.
Summary by CodeRabbit