fix: persist workspace panel visibility across restarts#69
Merged
Conversation
Move sidebar and file explorer visibility into global app settings so panel state survives restarts and project switches. Serialize immediate writes, add rollback on failure, and wait on close so the latest toggle is not lost. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
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 (10)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds persistence-backed panel visibility for sidebar and file explorer, a concurrency-managed update API (useUpdatePanelVisibility), integrates persistence into TitleBar and WorkspaceLayout (including keyboard shortcut sidebarToggle), migrates file explorer visibility out of editor persistence, and updates close-flow to await pending app-settings writes. Changes
Sequence Diagram(s)sequenceDiagram
participant Component as rgba(100,149,237,0.5) Component (TitleBar/WorkspaceLayout)
participant Hook as rgba(60,179,113,0.5) useUpdatePanelVisibility
participant Store as rgba(255,165,0,0.5) UI Store (Sidebar/FileExplorer)
participant Persist as rgba(199,21,133,0.5) Persistence API
participant Toast as rgba(128,128,128,0.5) Toast
Component->>Hook: updatePanelVisibility(panel, visible)
Hook->>Store: Optimistically set visibility
Hook->>Persist: enqueue/write new visibility
alt persist success
Persist-->>Hook: write OK
Hook-->>Component: resolve
else persist failure
Persist-->>Hook: error
Hook->>Store: rollback visibility
Hook->>Toast: show error message
Hook-->>Component: reject
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
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 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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/layouts/WorkspaceLayout.tsx (1)
243-253:⚠️ Potential issue | 🔴 CriticalReturn the
Promise<boolean>thatonCloseRequested()expects.Line 243 currently passes a
voidcallback intowindowApi.onCloseRequested(), but the real API awaits aPromise<boolean>. That is off-contract fortypecheck, and the test mock inWorkspaceLayout.test.tsxalso using() => voidmeans it will not catch the regression. ReturnPromise.resolve(false)after kicking off the dialog/respondToCloseflow so the close is explicitly prevented while the async persistence wait finishes.Suggested fix
useEffect(() => { return windowApi.onCloseRequested(() => { const dirtyCount = useEditorStore.getState().getDirtyFileCount() if (dirtyCount > 0) { setAppCloseDirtyCount(dirtyCount) setIsAppCloseDialogOpen(true) } else { void waitForPendingAppSettingsPersistence().then(() => { windowApi.respondToClose('close') }) } + return Promise.resolve(false) }) }, [])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/layouts/WorkspaceLayout.tsx` around lines 243 - 253, The onCloseRequested callback currently returns void but must return a Promise<boolean>; update the callback passed to windowApi.onCloseRequested so it returns Promise.resolve(false) after initiating the close logic (i.e., after calling useEditorStore.getState().getDirtyFileCount(), setAppCloseDirtyCount, setIsAppCloseDialogOpen or kicking off waitForPendingAppSettingsPersistence().then(() => windowApi.respondToClose('close'))). Ensure the callback signature remains async-compatible and always returns a Promise<boolean> (false to prevent immediate close while async persistence runs).
🧹 Nitpick comments (1)
src/renderer/lib/tauri-keyboard-api.ts (1)
53-72: UseKeyboardShortcutCallbackfrom shared types to eliminate duplication.The shortcut union is repeated in both the callback parameter (line 53) and the type cast (lines 66-71). Since
KeyboardShortcutCallbackis already defined insrc/shared/types/ipc.types.tsand used in theKeyboardApiinterface, import and use it here instead of repeating the literal union.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/renderer/lib/tauri-keyboard-api.ts` around lines 53 - 72, Replace the duplicated literal union with the shared type: import the KeyboardShortcutCallback type and use it as the callback parameter type in onShortcut and for the payload cast inside the listen handler; update the signature onShortcut(callback: KeyboardShortcutCallback): () => void and cast payload as KeyboardShortcutCallback (while keeping the existing listen/UnlistenFn logic and IPC_EVENTS.SHORTCUT reference) so the union is not duplicated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/renderer/hooks/use-app-settings.ts`:
- Around line 11-16: The queued panel-write logic (panelWriteChain,
panelWriteRequestIds, pendingPanelWriteCount) currently snapshots nothing and
reads live state when the async task runs, causing stale writes and incorrect
rollbacks; change the queue to capture each request’s specific panel key and
intended visible value in the queued payload (persist the panel/visible pair
with the task) and add a global write revision/check (e.g., increment a
writeRevision or include a requestId in the payload) that rollback logic must
compare against so only the most recent successful write can undo UI state;
update the corresponding enqueue/dequeue code paths (the functions that push
tasks onto panelWriteChain and the rollback code around the final write
success/failure) to use the captured payload and the revision guard—apply the
same pattern to the other similar blocks referenced (around the other panel
write variables at the other occurrences).
- Around line 19-24: The lint rule lint/suspicious/useIterableCallbackReturn is
triggered by the implicit-return arrow in notifyPanelWriteSettled; change the
callback passed to pendingPanelWriteWaiters.forEach/waiters.forEach to use a
block body so it has an explicit statement (e.g., replace (resolve) => resolve()
with (resolve) => { resolve(); }) or use an equivalent explicit loop, ensuring
you still clear pendingPanelWriteWaiters and call each resolve.
---
Outside diff comments:
In `@src/renderer/layouts/WorkspaceLayout.tsx`:
- Around line 243-253: The onCloseRequested callback currently returns void but
must return a Promise<boolean>; update the callback passed to
windowApi.onCloseRequested so it returns Promise.resolve(false) after initiating
the close logic (i.e., after calling
useEditorStore.getState().getDirtyFileCount(), setAppCloseDirtyCount,
setIsAppCloseDialogOpen or kicking off
waitForPendingAppSettingsPersistence().then(() =>
windowApi.respondToClose('close'))). Ensure the callback signature remains
async-compatible and always returns a Promise<boolean> (false to prevent
immediate close while async persistence runs).
---
Nitpick comments:
In `@src/renderer/lib/tauri-keyboard-api.ts`:
- Around line 53-72: Replace the duplicated literal union with the shared type:
import the KeyboardShortcutCallback type and use it as the callback parameter
type in onShortcut and for the payload cast inside the listen handler; update
the signature onShortcut(callback: KeyboardShortcutCallback): () => void and
cast payload as KeyboardShortcutCallback (while keeping the existing
listen/UnlistenFn logic and IPC_EVENTS.SHORTCUT reference) so the union is not
duplicated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ff9aef49-133b-4a14-a43c-01356fc3213e
📒 Files selected for processing (15)
src/renderer/components/TitleBar.test.tsxsrc/renderer/components/TitleBar.tsxsrc/renderer/hooks/use-app-settings.test.tssrc/renderer/hooks/use-app-settings.tssrc/renderer/hooks/use-editor-persistence.test.tssrc/renderer/hooks/use-editor-persistence.tssrc/renderer/layouts/WorkspaceLayout.test.tsxsrc/renderer/layouts/WorkspaceLayout.tsxsrc/renderer/lib/tauri-keyboard-api.tssrc/renderer/stores/app-settings-store.test.tssrc/renderer/stores/app-settings-store.tssrc/renderer/stores/keyboard-shortcuts-store.test.tssrc/renderer/stores/sidebar-store.test.tssrc/renderer/types/settings.tssrc/shared/types/ipc.types.ts
gnoviawan
changed the title
Capture panel write payloads in the persistence queue, tighten close-request typing, and reuse shared shortcut callback types. This keeps rollback behavior deterministic and aligns the Tauri window API with async close coordination. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bring the branch up to date with dev while preserving global workspace panel persistence and close-flow synchronization. Align the merged persistence, terminal restore, and WorkspaceLayout tests so the #40 behavior remains covered. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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
Ctrl+Bfor the file explorer and add a dedicated configurable sidebar shortcutTest plan
src/renderer/hooks/use-app-settings.test.tssrc/renderer/components/TitleBar.test.tsxsrc/renderer/hooks/use-editor-persistence.test.tssrc/renderer/layouts/WorkspaceLayout.test.tsxsrc/renderer/stores/keyboard-shortcuts-store.test.tssrc/renderer/stores/sidebar-store.test.tssrc/renderer/stores/app-settings-store.test.tssrc/renderer/stores/file-explorer-store.test.tsbun run typecheckbunx lint-stagedCloses #40.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Tests