fix(desktop): restore new workspace modal persistence#2225
Merged
Kitenite merged 1 commit intoMar 8, 2026
Conversation
Contributor
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughCentralizes NewWorkspaceModal state into a new draft context/provider, replaces CommandDialog with Dialog-based UI, moves per-tab and selection state into the draft, rewires child group components to use the draft and runAsyncAction/closeAndResetDraft flows, and introduces NewWorkspaceModalContent. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as Dialog Modal
participant D as DraftProvider
participant C as NewWorkspaceModalContent
participant G as Group (Prompt / List)
participant API as Workspace API
participant T as toast.promise
U->>M: open modal / interact (select project, type, switch tab)
M->>D: read/updateDraft (selectedProjectId, activeTab, queries)
C->>D: read draft snapshot (to render UI)
U->>C: trigger create/import action
G->>D: gather draft payload
G->>API: call mutateAsync (create workspace) -- returns Promise
G->>T: runAsyncAction / toast.promise(promise)
API-->>T: success / failure
T-->>G: show result
G->>D: closeAndResetDraft or resetDraftIfVersion on success
D->>M: modal closed/state reset
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
2 issues 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/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx:161">
P3: Empty `.catch(() => undefined)` silently swallows the rejection. Although `toast.promise` shows the error to the user, this hides it from console/logs. Replace with a minimal log so failures remain observable during debugging.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) [FEEDBACK_USED]</violation>
</file>
<file name="apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx">
<violation number="1" location="apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx:113">
P1: `DialogHeader` with `DialogTitle` and `DialogDescription` must be inside `DialogContent`, not a sibling. Because `DialogContent` renders through a Radix portal, these elements end up in a different DOM subtree, breaking the `aria-labelledby`/`aria-describedby` associations and triggering Radix console warnings.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| .then(() => { | ||
| clearInputsIfDraftVersion(submitDraftVersion); | ||
| }) | ||
| .catch(() => undefined); |
Contributor
There was a problem hiding this comment.
P3: Empty .catch(() => undefined) silently swallows the rejection. Although toast.promise shows the error to the user, this hides it from console/logs. Replace with a minimal log so failures remain observable during debugging.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx, line 161:
<comment>Empty `.catch(() => undefined)` silently swallows the rejection. Although `toast.promise` shows the error to the user, this hides it from console/logs. Replace with a minimal log so failures remain observable during debugging.
(Based on your team's feedback about avoiding empty catch blocks that hide failures.) </comment>
<file context>
@@ -125,26 +134,31 @@ export function IssuesGroup({ projectId, onClose }: IssuesGroupProps) {
+ .then(() => {
+ clearInputsIfDraftVersion(submitDraftVersion);
+ })
+ .catch(() => undefined);
}}
className="group h-12"
</file context>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/stores/new-workspace-modal.ts (1)
80-81: Consider whether tab changes should incrementdraftVersion.Currently,
setActiveTabincrementsdraftVersion, which means switching tabs after initiating workspace creation (but before the promise resolves) will prevent inputs from being cleared. This may be intentional (preserving draft if user continues interacting), but it could also surprise users if they switch tabs while waiting and then return to find stale inputs.If the intent is to only track content edits, consider not incrementing
draftVersionon tab changes:♻️ Optional: Don't increment draftVersion on tab change
- setActiveTab: (activeTab) => - set((state) => ({ activeTab, draftVersion: state.draftVersion + 1 })), + setActiveTab: (activeTab) => set({ activeTab }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/stores/new-workspace-modal.ts` around lines 80 - 81, The setActiveTab updater currently increments draftVersion which causes tab switches to count as content edits; update the setActiveTab implementation (the setActiveTab function that calls set(...) and touches draftVersion) to stop modifying draftVersion when changing tabs so that only real content edits bump draftVersion; if you intend to preserve the current behavior, instead add a clear comment in setActiveTab noting why draftVersion increments on tab change and consider adding an explicit method (e.g., bumpDraftVersion) for content edits to avoid surprising side-effects.
🤖 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/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 58-66: The useEffect in NewWorkspaceModal runs
setSelectedProjectId unconditionally when no matching project is found, causing
an infinite loop because setSelectedProjectId always increments draftVersion;
update the effect to only call setSelectedProjectId when the new id differs from
the current selectedProjectId and is non-null (e.g. compute const newId =
recentProjects[0]?.id ?? null and call setSelectedProjectId(newId) only if newId
!== selectedProjectId), or skip calling it when recentProjects is empty;
reference the useEffect block and the selectedProjectId / setSelectedProjectId
variables to locate the change.
---
Nitpick comments:
In `@apps/desktop/src/renderer/stores/new-workspace-modal.ts`:
- Around line 80-81: The setActiveTab updater currently increments draftVersion
which causes tab switches to count as content edits; update the setActiveTab
implementation (the setActiveTab function that calls set(...) and touches
draftVersion) to stop modifying draftVersion when changing tabs so that only
real content edits bump draftVersion; if you intend to preserve the current
behavior, instead add a clear comment in setActiveTab noting why draftVersion
increments on tab change and consider adding an explicit method (e.g.,
bumpDraftVersion) for content edits to avoid surprising side-effects.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 97cc95b5-5fa5-486a-8c6d-140071217c22
📒 Files selected for processing (6)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxapps/desktop/src/renderer/stores/new-workspace-modal.ts
Kitenite
force-pushed
the
kitenite/did-we-remove-persistence-on-the-new-workspace-mod
branch
from
05380ec to
3d770ee
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
103-126:⚠️ Potential issue | 🟠 MajorInfinite loop when
recentProjectsis empty andselectedProjectIdisnull.When
recentProjectsis empty,hasSelectedProjectisfalse, triggeringupdateDraft({ selectedProjectId: null }). SinceupdateDraftalways incrementsdraftVersion, this causes a re-render and the effect runs again indefinitely.🐛 Proposed fix: Guard against setting the same value
useEffect(() => { if (!isOpen) return; if ( preSelectedProjectId && preSelectedProjectId !== draft.selectedProjectId ) { updateDraft({ selectedProjectId: preSelectedProjectId }); return; } const hasSelectedProject = recentProjects.some( (project) => project.id === draft.selectedProjectId, ); if (!hasSelectedProject) { - updateDraft({ selectedProjectId: recentProjects[0]?.id ?? null }); + const newProjectId = recentProjects[0]?.id ?? null; + if (newProjectId !== draft.selectedProjectId) { + updateDraft({ selectedProjectId: newProjectId }); + } } }, [ draft.selectedProjectId, isOpen, preSelectedProjectId, recentProjects, updateDraft, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx` around lines 103 - 126, The effect in NewWorkspaceModal's useEffect is causing an infinite loop because when recentProjects is empty it calls updateDraft({ selectedProjectId: null }) even if draft.selectedProjectId is already null; change the logic in the effect to only call updateDraft when the new selectedProjectId differs from the current draft.selectedProjectId (i.e., compute const newId = recentProjects[0]?.id ?? null and only call updateDraft({ selectedProjectId: newId }) if newId !== draft.selectedProjectId), leaving the existing preSelectedProjectId branch intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 103-126: The effect in NewWorkspaceModal's useEffect is causing an
infinite loop because when recentProjects is empty it calls updateDraft({
selectedProjectId: null }) even if draft.selectedProjectId is already null;
change the logic in the effect to only call updateDraft when the new
selectedProjectId differs from the current draft.selectedProjectId (i.e.,
compute const newId = recentProjects[0]?.id ?? null and only call updateDraft({
selectedProjectId: newId }) if newId !== draft.selectedProjectId), leaving the
existing preSelectedProjectId branch intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19dea91e-d775-45a0-aa75-92593853691b
📒 Files selected for processing (7)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxapps/desktop/src/renderer/stores/new-workspace-modal.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/stores/new-workspace-modal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
Kitenite
force-pushed
the
kitenite/did-we-remove-persistence-on-the-new-workspace-mod
branch
from
3d770ee to
68457a5
Compare
Kitenite
force-pushed
the
kitenite/did-we-remove-persistence-on-the-new-workspace-mod
branch
from
68457a5 to
0fb2d5b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
100-116:⚠️ Potential issue | 🟠 MajorDon't overwrite the caller's preselected project in the fallback effect.
After the first sync, Lines 111-116 still run. If
openModal(projectId)supplied a valid project that is not ingetRecents(), this effect immediately replaces it withrecentProjects[0]. WhenrecentProjectsis empty, it also keeps writingnull, which can spin the modal becauseupdateDraft()bumpsdraftVersionon every call.Proposed fix
useEffect(() => { if (!isOpen) return; if ( preSelectedProjectId && preSelectedProjectId !== draft.selectedProjectId ) { updateDraft({ selectedProjectId: preSelectedProjectId }); return; } + if (preSelectedProjectId) { + return; + } + + const fallbackProjectId = recentProjects[0]?.id ?? null; const hasSelectedProject = recentProjects.some( (project) => project.id === draft.selectedProjectId, ); - if (!hasSelectedProject) { - updateDraft({ selectedProjectId: recentProjects[0]?.id ?? null }); + if (!hasSelectedProject && fallbackProjectId !== draft.selectedProjectId) { + updateDraft({ selectedProjectId: fallbackProjectId }); } }, [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx` around lines 100 - 116, The effect in NewWorkspaceModal.tsx currently overwrites a caller-supplied preSelectedProjectId via the fallback that sets draft.selectedProjectId from recentProjects; change the logic in the useEffect so the fallback block (the hasSelectedProject check and call to updateDraft) only runs when preSelectedProjectId is falsy, and avoid writing null into updateDraft when recentProjects is empty (keep the existing draft.selectedProjectId instead). In practice: after the existing preSelectedProjectId check/return, add a guard that if preSelectedProjectId is truthy skip the fallback, and when picking a fallback id use recentProjects[0]?.id ?? draft.selectedProjectId (or simply skip updateDraft if no recentProjects[0]) to prevent spinning updates from writing null.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx (1)
90-219: ExtractNewWorkspaceModalContentinto its own file.This file now contains two components, which makes the modal shell harder to scan and falls outside the repo's component layout rules.
As per coding guidelines,
**/{components,pages}/**/*.{tsx,ts}: No multi-component files - maintain one component per file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx` around lines 90 - 219, Extract the NewWorkspaceModalContent component into its own file: create a new NewWorkspaceModalContent.tsx that exports the NewWorkspaceModalContent function (and its props type NewWorkspaceModalContentProps if declared inline) and move all logic including useNewWorkspaceModalDraft, useEffect, listQuery/handleListQueryChange, and JSX rendering (Tabs, ProjectSelector, Command/CommandInput/CommandList, PullRequestsGroup, BranchesGroup, IssuesGroup, PromptGroup) into it; then replace the in-file definition with an import of NewWorkspaceModalContent in the original file and ensure any referenced symbols (useNewWorkspaceModalDraft, ProjectSelector, PullRequestsGroup, BranchesGroup, IssuesGroup, PromptGroup, COMMAND_CLASS_NAME) are imported in the new file and exported types/props are re-exported if the parent expects them.
🤖 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/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx`:
- Line 36: The code currently calls closeAndResetDraft() from
useNewWorkspaceModalDraft when navigating users to Integrations, which clears
the draft and loses restored modal state; change those calls to closeModal()
instead so the modal is closed but the draft is preserved. Update the two places
that call closeAndResetDraft() (the one at PullRequestsGroup.tsx where
closeAndResetDraft is destructured and the call sites around the Integrations
navigation — also the similar calls at the other mentioned location around lines
107-109) to call closeModal() while leaving closeAndResetDraft() reserved for
successful create/open flows.
---
Duplicate comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 100-116: The effect in NewWorkspaceModal.tsx currently overwrites
a caller-supplied preSelectedProjectId via the fallback that sets
draft.selectedProjectId from recentProjects; change the logic in the useEffect
so the fallback block (the hasSelectedProject check and call to updateDraft)
only runs when preSelectedProjectId is falsy, and avoid writing null into
updateDraft when recentProjects is empty (keep the existing
draft.selectedProjectId instead). In practice: after the existing
preSelectedProjectId check/return, add a guard that if preSelectedProjectId is
truthy skip the fallback, and when picking a fallback id use
recentProjects[0]?.id ?? draft.selectedProjectId (or simply skip updateDraft if
no recentProjects[0]) to prevent spinning updates from writing null.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx`:
- Around line 90-219: Extract the NewWorkspaceModalContent component into its
own file: create a new NewWorkspaceModalContent.tsx that exports the
NewWorkspaceModalContent function (and its props type
NewWorkspaceModalContentProps if declared inline) and move all logic including
useNewWorkspaceModalDraft, useEffect, listQuery/handleListQueryChange, and JSX
rendering (Tabs, ProjectSelector, Command/CommandInput/CommandList,
PullRequestsGroup, BranchesGroup, IssuesGroup, PromptGroup) into it; then
replace the in-file definition with an import of NewWorkspaceModalContent in the
original file and ensure any referenced symbols (useNewWorkspaceModalDraft,
ProjectSelector, PullRequestsGroup, BranchesGroup, IssuesGroup, PromptGroup,
COMMAND_CLASS_NAME) are imported in the new file and exported types/props are
re-exported if the parent expects them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a351f52d-c0f7-45ab-90b4-3699f0c88e80
📒 Files selected for processing (7)
apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsxapps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxapps/desktop/src/renderer/stores/new-workspace-modal.ts
💤 Files with no reviewable changes (1)
- apps/desktop/src/renderer/stores/new-workspace-modal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx
| const navigate = useNavigate(); | ||
| const { gateFeature } = usePaywall(); | ||
| const createFromPr = useCreateFromPr(); | ||
| const { closeAndResetDraft, runAsyncAction } = useNewWorkspaceModalDraft(); |
Contributor
There was a problem hiding this comment.
Keep the draft when sending users to Integrations.
This path is not a successful create/open action, so clearing here throws away the restored modal state the PR is trying to preserve. Use closeModal() instead and reserve closeAndResetDraft() for successful workspace open/create flows.
Proposed fix
- const { closeAndResetDraft, runAsyncAction } = useNewWorkspaceModalDraft();
+ const { closeModal, closeAndResetDraft, runAsyncAction } =
+ useNewWorkspaceModalDraft();
…
gateFeature(GATED_FEATURES.INTEGRATIONS, () => {
- closeAndResetDraft();
+ closeModal();
navigate({ to: "/settings/integrations" });
});Also applies to: 107-109
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsx`
at line 36, The code currently calls closeAndResetDraft() from
useNewWorkspaceModalDraft when navigating users to Integrations, which clears
the draft and loses restored modal state; change those calls to closeModal()
instead so the modal is closed but the draft is preserved. Update the two places
that call closeAndResetDraft() (the one at PullRequestsGroup.tsx where
closeAndResetDraft is destructured and the call sites around the Integrations
navigation — also the similar calls at the other mentioned location around lines
107-109) to call closeModal() while leaving closeAndResetDraft() reserved for
successful create/open flows.
Contributor
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
saddlepaddle
added a commit
that referenced
this pull request
Mar 8, 2026
The switch from CommandDialog to Dialog/DialogContent in #2225 lost the bg-popover/text-popover-foreground colors that Command previously provided, causing the modal background to use bg-background instead.
2 tasks
saddlepaddle
added a commit
that referenced
this pull request
Mar 8, 2026
The switch from CommandDialog to Dialog/DialogContent in #2225 lost the bg-popover/text-popover-foreground colors that Command previously provided, causing the modal background to use bg-background instead.
9 tasks
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
cmdkcommand list again so the textarea keeps native multiline and arrow-key behavioruseStatecontext with shared close/reset and async-success helpers to cut down repeated action wiringTesting
bunx biome check apps/desktop/src/renderer/stores/new-workspace-modal.ts apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModal.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/NewWorkspaceModalDraftContext.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/PromptGroup/PromptGroup.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/IssuesGroup/IssuesGroup.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/BranchesGroup/BranchesGroup.tsx apps/desktop/src/renderer/components/NewWorkspaceModal/components/PullRequestsGroup/PullRequestsGroup.tsxbunx tsc -p apps/desktop/tsconfig.json --noEmitSummary by CodeRabbit
New Features
Bug Fixes
Style