fix(electron): apply persisted hide-app-icon setting at startup#28
fix(electron): apply persisted hide-app-icon setting at startup#28ryota-murakami merged 4 commits intomainfrom
Conversation
The hideAppIcon preference is persisted in renderer-side localStorage via redux-storage-middleware, but app.setActivationPolicy() is runtime-only and resets to 'regular' on every launch. The Settings UI showed the toggle as ON while the dock icon remained visible until the user toggled it again. Adds ElectronStartupSync mounted in the root layout to forward the persisted value to the main process via IPC after Redux hydration. The IPC handler is idempotent so firing on every selector change is safe.
Apply review feedback from /simplify: - Replace useIsElectron hook with isElectronEnvironment() called inside the effect. Avoids hauling ElectronLoginForm + Clerk hooks into the root layout chunk for web users; SSR-safe because effects are browser-only anyway. - Catch IPC promise rejection so a main-process regression surfaces in the console instead of being silently swallowed. - Drop the redundant typeof window guard inside useEffect. - Collapse duplicated true/false test cases into it.each, and add coverage for the not-in-Electron and missing-electronAPI guards.
Adds two regression tests surfaced during /review: 1. setHideAppIcon rejection — asserts the .catch handler logs the prefixed error to console.error. Without this test, removing the handler would not cause a CI failure. 2. hideAppIcon re-sync — asserts the effect re-fires when the Redux value changes after mount. Locks down the [hideAppIcon] dependency so a future refactor to [] (mount-only) is caught immediately. Skipped lower-value findings: partial electronAPI shape edge cases (existing undefined test covers optional chaining), afterEach cleanup (Vitest isolates JSDOM per file), JSDoc rewording (current text is accurate).
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
==========================================
+ Coverage 62.95% 64.03% +1.08%
==========================================
Files 13 17 +4
Lines 332 367 +35
Branches 81 83 +2
==========================================
+ Hits 209 235 +26
- Misses 114 123 +9
Partials 9 9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughIntroduces a new Changes
Sequence Diagram(s)sequenceDiagram
participant App as App Shell
participant Component as ElectronStartupSync
participant Redux as Redux Store
participant IPC as Electron IPC
participant Main as Main Process
App->>Component: Render & Mount
Component->>Redux: Select hideAppIcon value
Redux-->>Component: Return setting value
Component->>Component: Check isElectronEnvironment()
alt Electron Environment Detected
Component->>IPC: window.electronAPI.settings.setHideAppIcon(value)
IPC->>Main: IPC Message
Main-->>IPC: Acknowledge
IPC-->>Component: Promise resolved
else Not Electron / API unavailable
Component->>Component: Skip IPC call
end
Note over Component: useEffect runs on hideAppIcon change
Redux->>Component: hideAppIcon updated
Component->>IPC: Re-sync new value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 52 minutes and 33 seconds.Comment |
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
![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 the current code and only fix it if needed.
Inline comments:
In `@src/components/electron/ElectronStartupSync.tsx`:
- Around line 52-59: The current call to
window.electronAPI?.settings?.setHideAppIcon(hideAppIcon) only handles rejected
promises via .catch and misses when the IPC resolves to false; update the call
in ElectronStartupSync.tsx (the window.electronAPI?.settings?.setHideAppIcon
invocation with hideAppIcon) to inspect the resolved value (e.g., await or
.then(result => ...)) and treat a false result as a failure — log an error (or
handle appropriately) when result === false in addition to preserving the
existing .catch for thrown errors.
🪄 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: 985fa1d7-3072-4c66-a95e-9d8cb819d0c5
📒 Files selected for processing (3)
src/app/layout.tsxsrc/components/electron/ElectronStartupSync.test.tsxsrc/components/electron/ElectronStartupSync.tsx
The preload bridge for setHideAppIcon swallows thrown errors via try/catch and resolves to `false` instead of rejecting (see electron/preload.ts:1491- 1502). The previous .catch-only handler was therefore unreachable in the real failure path: any main-process regression during startup sync would be silently dropped. Switch to .then(ok => ...) + .catch chain so we log when the IPC resolves to `false` (the actual failure signal) while keeping the .catch as defense- in-depth in case preload behavior changes. Two new tests pin down the contract: - false-resolve must log the IPC-returned-false error - true-resolve must not log anything (guards against inverted check) Addresses CodeRabbit major finding on PR #28.
2 participants
Summary
app.setActivationPolicy()is runtime-only and resets toregularon every launch. The ReduxhideAppIconvalue persisted in localStorage viaredux-storage-middleware, but was never re-applied to the main process at startup, so the renderer UI and the actual dock state diverged.<ElectronStartupSync />component (src/components/electron/ElectronStartupSync.tsx) mounted inside<ReduxProvider>inapp/layout.tsx. It forwards the persistedhideAppIconvalue to the main process via the existingwindow.electronAPI.settings.setHideAppIconIPC after Redux hydrates from localStorage. Re-fires on Redux value change so the Settings UI keeps the dock policy in sync at runtime.Implementation notes
isElectronEnvironment()directly inside the effect rather than theuseIsElectronhook — avoids importing the heavy auth-form module (and its Clerk hooks) into the root layout chunk for web users, while staying SSR-safe because effects only run in the browser.[hideAppIcon]so the dock policy follows the Redux value at runtime, not just at mount.console.errorwith prefix[ElectronStartupSync] Failed to sync hideAppIcon:— swallowing them silently would mask main-process regressions during startup sync.electron/main.ts:1365is idempotent (re-applying the same activation policy is a no-op), so firing on every selector change is safe.Test plan
pnpm test— 58/58 unit tests pass (6 new inElectronStartupSync.test.tsx)hideAppIcon=trueto main process on mounthideAppIcon=falseto main process on mountwindow.electronAPIis undefinedsetHideAppIconrejects (locks down the.catchhandler)hideAppIconchanges after mount (locks down the[hideAppIcon]dependency)pnpm typecheck— cleanpnpm lint— clean (no warnings)hideAppIcon=truepersisted in localStorage → dock icon should be hidden immediately, no toggle requiredSummary by CodeRabbit
New Features
Tests