Repository navigation
refactor(settings): resolve monitor wallpapers in rust - #766
Merged
Merged
Conversation
The fixed mode stores either one theme for every monitor or one entry per monitor, and the renderer rebuilt that union and the all pseudo-monitor by hand. A narrow command now returns the effective mapping and the mode a requested apply or stop writes.
The fixed store no longer interprets or builds the two wallpaper shapes: it asks the backend for the effective mapping and for the mode an apply or stop writes. The all-monitors expansion and the individual-map representative now live beside the command that tests them.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configuration
📒 Files selected for processing: 13
✨ Finishing Touches
Comment |
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
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.
Section 2.3 of the 2026-10-22 review:
store/fixed.store.tsbuilt themonitor_specific_wallpapersunion and the"all"pseudo-monitor by hand in JavaScript, while AGENTS.md says business logic lives in Rust.Rust now owns the mapping. A new
monitor_wallpapers.rsresolvesMonitorSpecificWallpapersinto an orderedVec<MonitorTheme>—"all"first,Allexpanded over every reported monitor, anIndividualmap yielding only its stored entries plus a synthetic"all"representative, and a monitor with no entry simply absent — and turns an apply or stop request into the nextWallpaperMode. One narrow command,resolve_monitor_themes_cmd, returns{ unified, themes, configured, mode }; nodwallcrate public API changed. Twelve unit tests pin exact values for each of those cases, including the empty map and random-to-fixed.The frontend no longer interprets the union:
fixed.store.tsasks the backend for the effective mapping and for the mode to write,configuredThemeIdis gone frommodels/config.ts, and the bindings were regenerated withbun run typegen.random.storeneeded no change — it only writes{ mode: "random", pool }and never built the union, so the review's reference to it was stale.Live-checked over CDP, because this changes what the app sends:
resolve_monitor_themes_cmdsaw{ monitorId: <Lenovo path>, themeId: "Catalina", unified: false }, thenapply_theme_cmdwrotemonitor_specific_wallpapers = { <Lenovo path>: "Catalina" };{ monitorId: "all", themeId: "Minya Konka", unified: true }, thenmonitor_specific_wallpapers = "Minya Konka"as a string.The UI showed applied and stop correctly in both cases, and the config file was restored afterwards.
Verified:
cargo testfor the whole workspace,cargo fmt --all -- --check, clippy clean in both profiles and in the shipped feature sets,bun run fix,bun run check,bun run test(93) andbun run build:vite.Known trade-off, left as-is deliberately:
configThemesis now a cache refreshed on load and optimistically on apply. If an apply fails and the user then clicks Discard in Settings, the cache stays stale until the next reload — the old code read the config directly at that point. It is an obscure failure-then-discard path and untested either way; worth a follow-up if it ever bites.Summary by CodeRabbit