fix(desktop): keep onboarding skip visible during loading#4175
Conversation
Every setup step short-circuited to a full-screen spinner while its initial query was pending, hiding the skip CTA. Users who hit a slow backing query (most often v2_projects on the project step) had no escape. Each loading state now renders inside the StepShell with the skip link still surfaced. Also swaps the project step from a v2_projects live query to an imperative v2Project.list trpc call so it doesn't gate on Electric shape warm-up across all 25 org collections.
📝 WalkthroughWalkthroughThis PR standardizes loading UI across five onboarding setup pages by replacing full-screen spinners with contextual StepShell/StepHeader layouts and skip buttons. Additionally, the project page migrates from local database queries to React Query TRPC API calls keyed by organization ID for improved data consistency and remote fetching. ChangesSetup Flow Loading UI Standardization
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 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 ensures "Skip for now" is always visible during onboarding by replacing full-screen spinners with
Confidence Score: 4/5Safe to merge; the UX improvement is straightforward and the two minor issues won't break any flows. The adopt-worktrees width mismatch produces a visible layout shift but no functional breakage. The disabled-query edge case in the project step could momentarily show the empty repo selection view instead of a spinner, but the authenticated route guard makes it nearly impossible to hit in practice. All skip/navigate logic is correctly wired up and consistent across steps. apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx (isLoading vs disabled-query semantics) and apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx (missing maxWidth on loading shell)
|
| Filename | Overview |
|---|---|
| apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx | Switched project listing from useLiveQuery (Electric sync) to useQuery (direct tRPC call); loading state now shows inside StepShell with skip CTA. Edge case: when activeOrganizationId is null the query is disabled and isLoading is false, briefly showing the empty "Select a repository" view before the session resolves. |
| apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx | Loading state now wrapped in StepShell with skip button; loading shell is missing maxWidth="lg" that the resolved AdoptWorktreesContent uses, causing a layout width shift on query resolution. |
| apps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsx | Loading state moved inside StepShell with skip button; maxWidth="lg" is consistent with the resolved state. |
| apps/desktop/src/renderer/routes/_authenticated/setup/gh-cli/page.tsx | Loading state moved inside StepShell with skip button; backTo and maxWidth are consistent with the resolved states. |
| apps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsx | Loading state moved inside StepShell with skip button; backTo and maxWidth="lg" are consistent with the resolved state. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User lands on setup step] --> B{Query isPending?}
B -- Yes --> C[StepShell loading state\nSpinner + Skip for now CTA]
B -- No --> D{Has data?}
C -- User clicks Skip --> E[markSkipped + navigate next]
C -- Query resolves --> D
D -- project step only\nactiveOrganizationId null --> F[Show empty Select a repo\nno loading state shown]
D -- Data ready --> G[Full step UI]
G --> H[Continue / Skip / Back]
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/setup/project/page.tsx:33-40
**Disabled query skips loading state when `activeOrganizationId` is null**
With `enabled: !!activeOrganizationId`, TanStack Query sets `isLoading: false` (not fetching) while the query is disabled. If `session?.session?.activeOrganizationId` resolves as `null`/`undefined` — even momentarily — the component falls straight through to the "Select a repository" view before transitioning to the loading state once the value becomes truthy. Consider using `isPending` instead of `isLoading`, or gating on `!session` as well, so the loading shell is shown for the full window between mount and the first successful fetch.
### Issue 2 of 2
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx:74-91
The loading shell omits `maxWidth="lg"` while `AdoptWorktreesContent` passes it. When the query resolves, the container width jumps from the default to `lg`, producing a visible layout shift. Add the prop to keep the width consistent during the transition.
```suggestion
if (isPending) {
return (
<StepShell backTo={STEP_ROUTES.project} maxWidth="lg">
<StepHeader
title="Looking for existing worktrees…"
subtitle="Scanning your recent projects."
/>
<div className="flex justify-center py-2">
<Spinner className="size-6 text-[#a8a5a3]" />
</div>
<div className="flex w-[273px] flex-col gap-2 self-center">
<SetupButton variant="link" onClick={skipFlow}>
Skip for now
</SetupButton>
</div>
</StepShell>
);
}
```
Reviews (1): Last reviewed commit: "fix(desktop): keep onboarding "Skip for ..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const { data: projects = [], isLoading } = useQuery({ | ||
| queryKey: ["onboarding", "v2Projects", activeOrganizationId], | ||
| enabled: !!activeOrganizationId, | ||
| queryFn: () => | ||
| apiTrpcClient.v2Project.list.query({ | ||
| organizationId: activeOrganizationId as string, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
activeOrganizationId is null
With enabled: !!activeOrganizationId, TanStack Query sets isLoading: false (not fetching) while the query is disabled. If session?.session?.activeOrganizationId resolves as null/undefined — even momentarily — the component falls straight through to the "Select a repository" view before transitioning to the loading state once the value becomes truthy. Consider using isPending instead of isLoading, or gating on !session as well, so the loading shell is shown for the full window between mount and the first successful fetch.
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx
Line: 33-40
Comment:
**Disabled query skips loading state when `activeOrganizationId` is null**
With `enabled: !!activeOrganizationId`, TanStack Query sets `isLoading: false` (not fetching) while the query is disabled. If `session?.session?.activeOrganizationId` resolves as `null`/`undefined` — even momentarily — the component falls straight through to the "Select a repository" view before transitioning to the loading state once the value becomes truthy. Consider using `isPending` instead of `isLoading`, or gating on `!session` as well, so the loading shell is shown for the full window between mount and the first successful fetch.
How can I resolve this? If you propose a fix, please make it concise.| if (isPending) { | ||
| return ( | ||
| <div className="flex h-full w-full items-center justify-center bg-[#151110]"> | ||
| <Spinner className="size-6 text-[#a8a5a3]" /> | ||
| </div> | ||
| <StepShell backTo={STEP_ROUTES.project}> | ||
| <StepHeader | ||
| title="Looking for existing worktrees…" | ||
| subtitle="Scanning your recent projects." | ||
| /> | ||
| <div className="flex justify-center py-2"> | ||
| <Spinner className="size-6 text-[#a8a5a3]" /> | ||
| </div> | ||
| <div className="flex w-[273px] flex-col gap-2 self-center"> | ||
| <SetupButton variant="link" onClick={skipFlow}> | ||
| Skip for now | ||
| </SetupButton> | ||
| </div> | ||
| </StepShell> | ||
| ); | ||
| } |
There was a problem hiding this comment.
maxWidth="lg" while AdoptWorktreesContent passes it. When the query resolves, the container width jumps from the default to lg, producing a visible layout shift. Add the prop to keep the width consistent during the transition.
| if (isPending) { | |
| return ( | |
| <div className="flex h-full w-full items-center justify-center bg-[#151110]"> | |
| <Spinner className="size-6 text-[#a8a5a3]" /> | |
| </div> | |
| <StepShell backTo={STEP_ROUTES.project}> | |
| <StepHeader | |
| title="Looking for existing worktrees…" | |
| subtitle="Scanning your recent projects." | |
| /> | |
| <div className="flex justify-center py-2"> | |
| <Spinner className="size-6 text-[#a8a5a3]" /> | |
| </div> | |
| <div className="flex w-[273px] flex-col gap-2 self-center"> | |
| <SetupButton variant="link" onClick={skipFlow}> | |
| Skip for now | |
| </SetupButton> | |
| </div> | |
| </StepShell> | |
| ); | |
| } | |
| if (isPending) { | |
| return ( | |
| <StepShell backTo={STEP_ROUTES.project} maxWidth="lg"> | |
| <StepHeader | |
| title="Looking for existing worktrees…" | |
| subtitle="Scanning your recent projects." | |
| /> | |
| <div className="flex justify-center py-2"> | |
| <Spinner className="size-6 text-[#a8a5a3]" /> | |
| </div> | |
| <div className="flex w-[273px] flex-col gap-2 self-center"> | |
| <SetupButton variant="link" onClick={skipFlow}> | |
| Skip for now | |
| </SetupButton> | |
| </div> | |
| </StepShell> | |
| ); | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsx
Line: 74-91
Comment:
The loading shell omits `maxWidth="lg"` while `AdoptWorktreesContent` passes it. When the query resolves, the container width jumps from the default to `lg`, producing a visible layout shift. Add the prop to keep the width consistent during the transition.
```suggestion
if (isPending) {
return (
<StepShell backTo={STEP_ROUTES.project} maxWidth="lg">
<StepHeader
title="Looking for existing worktrees…"
subtitle="Scanning your recent projects."
/>
<div className="flex justify-center py-2">
<Spinner className="size-6 text-[#a8a5a3]" />
</div>
<div className="flex w-[273px] flex-col gap-2 self-center">
<SetupButton variant="link" onClick={skipFlow}>
Skip for now
</SetupButton>
</div>
</StepShell>
);
}
```
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx`:
- Around line 33-40: The projects query currently only destructures data and
isLoading from useQuery, which hides failures; update the useQuery call that
calls apiTrpcClient.v2Project.list.query to also return isError, error, and
refetch (e.g., const { data: projects = [], isLoading, isError, error, refetch }
= useQuery(...)), then add an error state in the component that renders when
isError is true showing the error.message (or a fallback) and a retry control
that calls refetch; ensure the error text element includes the CSS classes
"select-text cursor-text" so the message is selectable.
🪄 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: 0e5f8c73-5000-430b-9742-d8885eb4f2de
📒 Files selected for processing (5)
apps/desktop/src/renderer/routes/_authenticated/setup/adopt-worktrees/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/gh-cli/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/permissions/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxapps/desktop/src/renderer/routes/_authenticated/setup/providers/page.tsx
| const { data: projects = [], isLoading } = useQuery({ | ||
| queryKey: ["onboarding", "v2Projects", activeOrganizationId], | ||
| enabled: !!activeOrganizationId, | ||
| queryFn: () => | ||
| apiTrpcClient.v2Project.list.query({ | ||
| organizationId: activeOrganizationId as string, | ||
| }), | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C3 'data:\s*projects\s*=\s*\[\]|isError|error|refetch' apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxRepository: superset-sh/superset
Length of output: 546
🏁 Script executed:
cat -n apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx | head -120Repository: superset-sh/superset
Length of output: 5035
🏁 Script executed:
wc -l apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxRepository: superset-sh/superset
Length of output: 139
🏁 Script executed:
cat -n apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx | tail -n +100Repository: superset-sh/superset
Length of output: 2702
🏁 Script executed:
# Verify if useQuery has any default error handling or retry config visible
rg -n 'retry|onError|enabled' apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxRepository: superset-sh/superset
Length of output: 151
🏁 Script executed:
# Check if there's any global error handling or default config for useQuery in this file
rg -n 'useQuery|QueryClient|error|catch' apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsxRepository: superset-sh/superset
Length of output: 223
Add error handling for the projects query to prevent silent failures.
Line 33 only destructures data and isLoading from useQuery, with no error handling. When the v2Project.list query fails, projects silently defaults to [], causing the UI to render "Select a repository" instead of showing an error. This masks query failures and can misroute users during onboarding.
Extract isError, error, and refetch from the query result and add an error state UI. As per coding guidelines, error text must be selectable with select-text cursor-text classes.
💡 Suggested fix
- const { data: projects = [], isLoading } = useQuery({
+ const {
+ data: projects,
+ isLoading,
+ isError,
+ error,
+ refetch,
+ } = useQuery({
queryKey: ["onboarding", "v2Projects", activeOrganizationId],
enabled: !!activeOrganizationId,
queryFn: () =>
apiTrpcClient.v2Project.list.query({
organizationId: activeOrganizationId as string,
}),
});
+ const projectList = projects ?? [];
...
- const projectCount = projects.length;
+ const projectCount = projectList.length;
...
+ if (isError) {
+ return (
+ <StepShell backTo={STEP_ROUTES.permissions}>
+ <StepHeader
+ icon={supersetIcon}
+ title="Couldn't load projects"
+ subtitle="We hit an error while checking your organization projects."
+ />
+ <p className="select-text cursor-text text-[12px] text-red-400">
+ {error instanceof Error ? error.message : "Unknown error"}
+ </p>
+ <div className="flex w-[273px] flex-col gap-2 self-center">
+ <SetupButton variant="secondary" onClick={() => refetch()}>
+ Retry
+ </SetupButton>
+ <SetupButton variant="link" onClick={handleSkipStep}>
+ Skip for now
+ </SetupButton>
+ </div>
+ </StepShell>
+ );
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/routes/_authenticated/setup/project/page.tsx`
around lines 33 - 40, The projects query currently only destructures data and
isLoading from useQuery, which hides failures; update the useQuery call that
calls apiTrpcClient.v2Project.list.query to also return isError, error, and
refetch (e.g., const { data: projects = [], isLoading, isError, error, refetch }
= useQuery(...)), then add an error state in the component that renders when
isError is true showing the error.message (or a fallback) and a retry control
that calls refetch; ensure the error text element includes the CSS classes
"select-text cursor-text" so the message is selectable.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
1 participant
Summary
providers,gh-cli,permissions,project,adopt-worktrees) short-circuited to a full-screen spinner while its initial query was pending, hiding the "Skip for now" CTA. The project step was hit hardest because its underlying Electricv2_projectsshape has to wait behind ~25 org collections warming up at signin — users with no escape from the spinner reported feeling stuck.StepShellwith the skip link still surfaced, so users always have a way out.useLiveQueryovercollections.v2Projectsto an imperativeapiTrpcClient.v2Project.listquery — the page only needs "do you have any projects?" and shouldn't pay the Electric warm-up cost for an answer used once.Test plan
/setup/adopt-worktrees.Summary by cubic
Keep “Skip for now” visible on all onboarding steps during loading so users aren’t stuck behind full-screen spinners. Also reduces load time on the project step by avoiding Electric shape warm‑up.
Bug Fixes
StepShellwith the skip CTA for providers, gh-cli, permissions, project, and adopt-worktrees.Refactors
useLiveQueryovercollections.v2ProjectswithapiTrpcClient.v2Project.listvia@tanstack/react-query, fetching only the needed project presence to avoid ~25 org collections warm-up.Written for commit 471c44f. Summary will update on new commits.
Summary by CodeRabbit