fix(settings): persist theme/wallpaper across sessions + apply Desktop & Dock settings - #1611
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
✨ 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 |
|
|
…p & Dock settings (#1601, #1603) Wallpaper never survived reload because the "Classic" wallpaper's catalog id happens to be the string "default", which collided with the backend's own "nothing saved yet" sentinel value of the same string. use-session-persistence.ts explicitly skipped restoring wallpaper: "default", so picking Classic silently reverted to graphite on next login. Align the backend's default wallpaper value with the frontend's actual boot default (graphite) so "default" only ever means an explicit user choice, and always restore whatever the backend returns. Desktop & Dock settings (icon size, position) wrote to bare localStorage keys that nothing else in the app ever read - the Dock component only ever consumed dock-store's pinned list and a theme-driven variant, so the controls had no visible effect. Move iconSize/position into dock-store, persist them through the existing /api/desktop/dock round trip (same pattern already used for pinned apps), and wire the Dock/DockIcon components to actually render them. Docs-Reviewed: settings-persistence fix, new test files under apps/ only, no app-list/route surface change
ff2ff09 to
526da95
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } | ||
| style={{ | ||
| height: "var(--spacing-dock-h)", | ||
| [isLeft ? "width" : "height"]: "var(--spacing-dock-h)", |
There was a problem hiding this comment.
WARNING: --spacing-dock-h is a height token (64px in tokens.css) and is now reused as the dock's width when position === "left". That ties the vertical dock's width to the horizontal dock's height — if anyone ever resizes --spacing-dock-h (e.g. for compact top bars or accessibility scaling), the vertical dock will resize along the wrong axis and could clip large (w-14/h-14) icons. Define a separate --spacing-dock-w token (or compute width from iconSize) instead of aliasing height.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| } | ||
| style={{ | ||
| height: "var(--spacing-dock-h)", | ||
| [isLeft ? "width" : "height"]: "var(--spacing-dock-h)", |
There was a problem hiding this comment.
WARNING: Same issue as MacosDock.tsx:36 — --spacing-dock-h is being reused as the taskbar's width in left mode, coupling the vertical and horizontal dimensions through one token. A future change to dock height (or a compact mode) would silently change the vertical taskbar's width too, and at 64px the taskbar is only wide enough for small/medium icons — large (w-14) icons will visually butt against the dock edges with no padding. Consider a dedicated --spacing-dock-w token or deriving width from iconSize.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (13 files)
NotesThe fix for both #1601 (wallpaper persistence) and #1603 (Desktop & Dock settings) is sound: the new backend default ( Fix these issues in Kilo Cloud Reviewed by minimax-m3 · Input: 76.6K · Output: 14.7K · Cached: 1.3M |
…1601, #1603) (#1617) The prior fix (#1611) wired up debounced auto-save and a mount-time restore for desktop settings, but the restore fetches (theme, wallpaper, dock, windows, widgets) live in SystemShortcuts, which mounts as a sibling of LoginGate before its /auth/status check resolves. On a fresh login (right after a logout) that mount-time restore fires while still unauthenticated, gets 401'd, and — being a one-shot effect — is never retried once the user actually logs in. The in-memory stores are left at their defaults for the rest of that session, which is exactly the reported symptom. Add an auth-ready-store that LoginGate publishes to once it reaches its "ready" phase, and gate the restore effects on it (resetting when it drops back to false so a later re-login re-fetches). Also split the dock/wallpaper auto-save guards off the outer "restore started" flag onto per-field "restore settled" flags, since the old shared flag flipped true the instant the restore effect began rather than once its fetch actually landed — a slow GET could otherwise lose a race to the debounced auto-save PUT and get overwritten with the pre-restore default value.
1 participant
Summary
Fixes two reported settings bugs (reporter: mandresve), both root-caused to code that sits next to — but doesn't correctly reuse — the app's existing preference-persistence pattern (
get_preference/save_preference+useSessionPersistence).#1601 — Theme/wallpaper not persisted across sessions
The "Classic" wallpaper's catalog id is literally the string
"default".use-session-persistence.tstreatedwallpaper === "default"as "nothing saved yet" and skipped restoring it — because the backend's ownDEFAULT_SETTINGS["wallpaper"]was also the string"default", colliding with a real, selectable wallpaper. Picking Classic wallpaper saved correctly but was silently dropped on the next login, reverting to graphite.Fix: align the backend's default wallpaper value with the frontend's actual boot-time default (
"graphite", matchingtheme-store.ts'sDEFAULT_WP), so"default"never doubles as a sentinel. The restore path now just trusts whatever the backend returns.#1603 — Desktop & Dock settings have no effect
DesktopDockSection(Settings → Desktop & Dock) wrote dock icon size/position to barelocalStoragekeys (taos-dock-size,taos-dock-position) that nothing else in the app ever read. The actualDockcomponent only consumeddock-store's pinned-apps list and a theme-driven variant — the icon size/position controls were fully orphaned.Fix: move
iconSize/positionintodock-store(alongsidepinned), persist them through the existing/api/desktop/dockround trip viauseSessionPersistence(the same pattern already used for pinned apps), and wireDock/MacosDock/WindowsTaskbar/DockIconto actually render the chosen size and edge position.Test plan
cd desktop && npm run build— passes (tsc + vite build)npx vitest run(desktop) — 297 files / 2417 tests pass, including new coverage:use-session-persistence.test.ts— saved wallpaper (including the"default"/Classic id) is re-read on load; saved dock icon size/position restored; invalid values ignoredDesktopDockSection.test.tsx— settings controls write todock-store, notlocalStorageDockVariants.test.tsx— dock-store icon size/position flow through to the renderedDockdock-store.test.ts— newsetIconSize/setPositionactionspython3 -m pytest tests/test_desktop_settings.py tests/test_routes_desktop.py -q— 17 passed