feat(desktop): add categorized wallpaper sections with theme-default first - #1882
Conversation
…first - Add WallpaperSection type and export Wallpaper interface from theme-store - Add themeDefaultWallpaperId store field, populated in keepTheme/restoreActiveTheme - Add getWallpapersBySection() returning Theme default / Built-in / Your wallpapers / Browse online - Rebuild WallpaperPicker with sectioned layout, badge for theme-default, empty states - Add theme-default pre-selection when current wallpaper matches theme's declared default - Keep animated slider controls and overlay toggle unchanged Refs: jaylfc#864
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: 51 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 (5)
✨ 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 |
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| } = useThemeStore(); | ||
| const wallpapers = getWallpapers(); | ||
| const sections = getWallpapersBySection(); | ||
| const themeDefaultId = themeDefaultWallpaperId[activeThemeId] || "graphite"; |
There was a problem hiding this comment.
WARNING: Duplicated themeDefaultId derivation can disagree with the store
This re-derives the theme-default id (themeDefaultWallpaperId[activeThemeId] || "graphite") independently of getWallpapersBySection, which already computes it (theme-store.ts:309-312). The two can diverge: the store resolves an unknown/missing themeDefaultId back to graphite for the section items (line 311-312 fallback), but isThemeDefault here keeps the raw string. If a theme's defaultWallpaperId is a non-existent id, the section silently shows graphite while the "Theme default" badge (line 157-159) can never appear because wallpaperId === themeDefaultId is compared against the invalid string. Prefer exposing the resolved theme-default id from getWallpapersBySection (e.g. as a field on the section, or a small getThemeDefaultWallpaperId() helper) so the badge and the section contents share one source of truth.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| const state = useThemeStore.getState(); | ||
| // The theme's declared default wallpaper id, or the global fallback. | ||
| const themeDefaultId = | ||
| state.themeDefaultWallpaperId[state.activeThemeId] || "graphite"; |
There was a problem hiding this comment.
SUGGESTION: Consider validating defaultWallpaperId against WALLPAPERS
themeDefaultId falls back to "graphite" only if the active theme has no declared default. If a theme declares a defaultWallpaperId that is not in WALLPAPERS (e.g. a typo in a custom/agent theme), themeDefaultWp silently falls back to graphite (line 311-312), but themeDefaultId still holds the invalid value. The component then compares the active wallpaperId against that invalid string, so the badge logic never matches. Either guard defaultWallpaperId when it is set (fall back to graphite only when the id is missing or unknown), or have the component consume the resolved id rather than re-deriving it.
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
SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by hy3:free · Input: 77.3K · Output: 5.7K · Cached: 179.1K |
2 participants
Summary
Refactors the WallpaperPicker to show categorized sections, with the active theme's default wallpaper pre-selected and shown first.
Changes
WallpaperSectiontype, exportedWallpaperinterface, addedthemeDefaultWallpaperIdstore field (populated inkeepTheme/restoreActiveTheme), addedgetWallpapersBySection()returning four sections: Theme default, Built-in, Your wallpapers (placeholder), Browse online (placeholder)Fixes #864 C1
Tasks: t_0d30f061 (kanban)
Tests: 36/36 pass (
npx vitest run)