revert: v2 workspace loading screen animation (#3788)#3820
Conversation
)" This reverts commit 95cff6b.
📝 WalkthroughWalkthroughThe pull request removes the keypad-based and step-progress loading UI components, deletes the v2-workspace-loading route, and replaces it with a custom progress UI in the pending workspace page that directly maps host-service progress steps. Navigation logic is simplified to return Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 SummaryThis PR reverts the v2 workspace keypad loading screen (#3788) to fix crashes in 1.7.0 caused by a ping-pong navigation loop. The core fix is in Confidence Score: 5/5Safe to merge — cleanly reverts the crash-inducing route and restores the null-return pattern while adding a functional inline progress UI. All changes are P2 or lower. The root-cause fix (removing the ping-pong useEffect in layout.tsx) is minimal and correct. The deleted route and components are fully de-referenced. The replacement inline UI in pending/page.tsx is straightforward and preserves all prior UX states (creating, succeeded, sync-timed-out, failed). No files require special attention.
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx | Core fix: removes the useEffect that navigated to /v2-workspace-loading/ when !isReady, eliminating the ping-pong redirect loop; now returns null while collections hydrate. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsx | Replaces the keypad-based loading UI with inline step progress using raw host-service step data; adds HiCheck import, removes V2WorkspaceLoadingView and mapHostProgressToInitStep; Dismiss button retained during creating status. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/page.tsx | Entire route deleted; was the source of the isReady mismatch (queried only v2Workspaces, not v2Hosts) that triggered navigation loops. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/V2WorkspaceLoadingView.tsx | Deleted: the main keypad loading view wrapper component that composed KeypadLoader and StepProgress. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/KeypadLoader.tsx | Deleted: the animated 3D keypad loader component with per-step key press animations. |
| apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/StepProgress.tsx | Deleted: the animated step-scroll progress indicator component. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User navigates to /v2-workspace/:workspaceId] --> B{isReady?}
B -- "Before PR" --> C["useEffect → navigate to /v2-workspace-loading/:workspaceId"]
C --> D[Loading page reads only v2Workspaces]
D --> E{isReady in loading page?}
E -- "v2Workspaces ready, v2Hosts not" --> F["navigate back to /v2-workspace/:workspaceId"]
F --> B
B -- "v2Hosts not ready" --> C
C -.->|"Ping-pong loop!"| F
B -- "After PR" --> G["return null (wait silently)"]
G --> H[Both collections hydrate]
H --> I[isReady = true → render workspace]
Reviews (1): Last reviewed commit: "Revert "Migrate v1 keypad loading screen..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/`$pendingId/page.tsx:
- Around line 281-282: The progress gate currently sets intentHasProgress based
only on pending?.intent === "fork" || pending?.intent === "checkout", which
excludes the "pr-checkout" flow; update the intentHasProgress check (the
variable declared as intentHasProgress) to also include pending?.intent ===
"pr-checkout" so that flows handled by checkoutWorkspace() in useFireIntent
display the host-service progress steps instead of the generic spinner.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0dc42d0-79e7-4fb6-8e38-55012fa6336c
⛔ Files ignored due to path filters (2)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/assets/key-single.pngis excluded by!**/*.pngapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/assets/keypad-base.pngis excluded by!**/*.png
📒 Files selected for processing (11)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/$pendingId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/KeypadLoader.cssapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/KeypadLoader.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/StepProgress.cssapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/StepProgress.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/V2WorkspaceLoadingView.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/index.tsapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/page.tsxapps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace/layout.tsx
💤 Files with no reviewable changes (9)
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/KeypadLoader.css
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/StepProgress.css
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/index.ts
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/page.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/StepProgress/StepProgress.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/V2WorkspaceLoadingView.tsx
- apps/desktop/src/renderer/routes/_authenticated/_dashboard/v2-workspace-loading/$workspaceId/components/V2WorkspaceLoadingView/KeypadLoader/KeypadLoader.tsx
| const intentHasProgress = | ||
| pending?.intent === "fork" || pending?.intent === "checkout"; |
There was a problem hiding this comment.
Include pr-checkout in the progress gate.
pr-checkout goes through checkoutWorkspace() in useFireIntent, but this condition excludes it, so that flow falls back to the generic spinner instead of showing host-service steps like normal checkout.
Suggested fix
- const intentHasProgress =
- pending?.intent === "fork" || pending?.intent === "checkout";
+ const intentHasProgress =
+ pending?.intent === "fork" ||
+ pending?.intent === "checkout" ||
+ pending?.intent === "pr-checkout";📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const intentHasProgress = | |
| pending?.intent === "fork" || pending?.intent === "checkout"; | |
| const intentHasProgress = | |
| pending?.intent === "fork" || | |
| pending?.intent === "checkout" || | |
| pending?.intent === "pr-checkout"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/pending/`$pendingId/page.tsx
around lines 281 - 282, The progress gate currently sets intentHasProgress based
only on pending?.intent === "fork" || pending?.intent === "checkout", which
excludes the "pr-checkout" flow; update the intentHasProgress check (the
variable declared as intentHasProgress) to also include pending?.intent ===
"pr-checkout" so that flows handled by checkoutWorkspace() in useFireIntent
display the host-service progress steps instead of the generic spinner.
| <button | ||
| type="button" | ||
| className="rounded-md border px-3 py-1.5 text-sm hover:bg-accent" | ||
| onClick={doNavigate} | ||
| > | ||
| Open anyway |
There was a problem hiding this comment.
Open anyway still routes into the unsynced not-found path.
This button is only rendered when !workspaceSynced, but doNavigate() pushes to /v2-workspace/$workspaceId, and that route still resolves the workspace from collections.v2Workspaces before it can render. In the timeout case, this recovery action commonly lands on WorkspaceNotFoundState instead of opening the workspace.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
2 participants
Summary
Reverts #3788 — the v2 workspace keypad loading screen.
Suspected as the source of crashes folks are seeing in 1.7.0. The new
/v2-workspace-loading/$workspaceIdroute and the workspace layout query different collections forisReady(layout joins v2Workspaces+v2Hosts; loading page only reads v2Workspaces), so when v2Hosts hydrates after v2Workspaces the two effects can ping-pong navigations on cold start.Reverts back to the prior behavior where the layout returns
nullwhile not ready.Test plan
Summary by cubic
Reverts the v2 workspace loading screen and route to stop cold-start crashes and redirect loops in 1.7.0. The layout now waits inline until data is ready, and the pending page shows a simple progress list.
Bug Fixes
/v2-workspace-loading/$workspaceIdto eliminate ping-pong navigation whenv2Hostsandv2Workspaceshydrate out of order.v2-workspacelayout no longer redirects while not ready; it returnsnullto avoid startup freeze/flicker.Refactors
Written for commit c9eabba. Summary will update on new commits. Review in cubic
Summary by CodeRabbit
New Features
Bug Fixes
Style