Breadway/bos-settings
1
0
Fork 0
Code Issues Pull requests 2 Projects Releases Packages Wiki Activity Actions

Canonical webview accent mapping + live per-output / monitor-move theming #3

Open
Breadway wants to merge 0 commits from fix/per-monitor-theme into main
pull from: fix/per-monitor-theme
merge into: Breadway:main
Conversation 0 Commits - Files changed - +124 -87
Breadway commented 2026-09-01 16:30:17 +08:00
Owner
Copy link

Fixes two per-monitor colour-theming bugs in commands/theme.rs.

Finding #1 (High) — accent mapping — NEEDS MAINTAINER CONFIRMATION

render_theme_css hand-rolled the :root { --name: ... } block and mapped:

--accent    = color1
--red       = color1
--on-accent = ink_on(color1)
--on-red    = ink_on(color1)

So --accent == --red == --on-accent == --on-red, all set to pywal's color1 — the ANSI-red slot bread-theme reserves for errors/destructive actions. Every GTK app in the ecosystem (and bread_theme::color_pairs / css_custom_properties / stylesheet) maps accent = color4, red = color1. On a multi-monitor desktop the settings window accented in a visibly different hue from the bar, the lock screen and every other window on the same monitor, and the frontend's --red / --on-red destructive-button styling (delete/remove/forget buttons across ~10 views) was indistinguishable from a normal accent button.

Decision: go canonical (accent = color4)

render_theme_css now calls bread_theme::css_custom_properties(palette) + bread_theme::css_tokens() directly instead of a hand-rolled copy. Both are exported from the pinned bread-theme v0.7.4 (the code comment claiming otherwise referenced v0.7.1 and was stale). This:

  • makes --accent = color4, matching the rest of the desktop per monitor;
  • restores --red (color1) and --on-red as distinct tokens, so destructive actions read as destructive again;
  • removes ~60 lines of duplicated token-mapping that could (and did) drift from the contract;
  • as a bonus, css_tokens emits --font-family: 'Varela Round', sans-serif instead of the local copy's 'Varela Round, sans-serif' (one over-quoted family that dropped the generic fallback).

bos-ui-demos/settings-redesign.html has a marketing line ("Accent is the warm color from the image - not pywal color4") suggesting the color1 mapping was once deliberate. It contradicts the bread-theme token contract and the rest of the ecosystem, and it's only in a demo file, so I went canonical — please confirm this is the intended direction. If the warm-accent look is wanted, it should at minimum not share color1 with --red, and ideally be a documented deviation in bread-theme itself.

Finding #4 (Medium) — live updates

watch_and_emit watched only the shared theme.css, filtered to that exact filename. Consequences:

  • breadpaper set_on on a non-focused monitor writes only palettes/<out>.json + themes/<out>.css — no theme.css rewrite, so a settings window parked on that monitor kept stale colours;
  • dragging the window to another monitor never re-rendered.

Change

  • The directory watch is now recursive over the runtime bread/ dir (so it also sees palettes/ and themes/), and the filter accepts the shared theme.css or the per-output palette/CSS file for the monitor this window is currently on (bread_theme::output_palette_path / output_css_path). If the monitor can't be resolved it falls back to reacting to any per-output *.json / *.css write, so a change is never missed. .tmp.<pid> scratch files from bread-theme's atomic writes are ignored.
  • Added a WindowEvent::Moved handler that re-renders with the new output's palette when the window lands on a different monitor (compares against the last monitor so a drag doesn't spam re-renders).
  • Factored monitor_name / emit_current_theme helpers out of the duplicated logic.

Tests

cargo build --locked, cargo build --release --locked, cargo clippy --all-targets --locked -D warnings, cargo test --locked (64 tests) — all pass. No frontend changes. No Cargo.lock change (both bread-theme helpers already in v0.7.4). No .cargo/config.toml patch in this repo.

Note: origin/main is not cargo fmt --check clean (pre-existing, across ~12 files); this branch touches only commands/theme.rs and leaves it fmt-clean without reformatting the rest.

Fixes two per-monitor colour-theming bugs in `commands/theme.rs`. ## Finding #1 (High) — accent mapping — NEEDS MAINTAINER CONFIRMATION `render_theme_css` hand-rolled the `:root { --name: ... }` block and mapped: ``` --accent = color1 --red = color1 --on-accent = ink_on(color1) --on-red = ink_on(color1) ``` So `--accent == --red == --on-accent == --on-red`, all set to pywal's `color1` — the ANSI-red slot `bread-theme` reserves for errors/destructive actions. Every GTK app in the ecosystem (and `bread_theme::color_pairs` / `css_custom_properties` / `stylesheet`) maps `accent = color4`, `red = color1`. On a multi-monitor desktop the settings window accented in a visibly different hue from the bar, the lock screen and every other window on the same monitor, and the frontend's `--red` / `--on-red` destructive-button styling (delete/remove/forget buttons across ~10 views) was indistinguishable from a normal accent button. ### Decision: go canonical (`accent = color4`) `render_theme_css` now calls `bread_theme::css_custom_properties(palette)` + `bread_theme::css_tokens()` directly instead of a hand-rolled copy. Both are exported from the pinned `bread-theme` v0.7.4 (the code comment claiming otherwise referenced v0.7.1 and was stale). This: - makes `--accent = color4`, matching the rest of the desktop per monitor; - restores `--red` (`color1`) and `--on-red` as distinct tokens, so destructive actions read as destructive again; - removes ~60 lines of duplicated token-mapping that could (and did) drift from the contract; - as a bonus, `css_tokens` emits `--font-family: 'Varela Round', sans-serif` instead of the local copy's `'Varela Round, sans-serif'` (one over-quoted family that dropped the generic fallback). `bos-ui-demos/settings-redesign.html` has a marketing line ("Accent is the warm color from the image - not pywal color4") suggesting the `color1` mapping was once deliberate. It contradicts the `bread-theme` token contract and the rest of the ecosystem, and it's only in a demo file, so I went canonical — **please confirm this is the intended direction.** If the warm-accent look is wanted, it should at minimum not share `color1` with `--red`, and ideally be a documented deviation in `bread-theme` itself. ## Finding #4 (Medium) — live updates `watch_and_emit` watched only the shared `theme.css`, filtered to that exact filename. Consequences: - `breadpaper set_on` on a non-focused monitor writes only `palettes/<out>.json` + `themes/<out>.css` — no `theme.css` rewrite, so a settings window parked on that monitor kept stale colours; - dragging the window to another monitor never re-rendered. ### Change - The directory watch is now **recursive** over the runtime `bread/` dir (so it also sees `palettes/` and `themes/`), and the filter accepts the shared `theme.css` **or** the per-output palette/CSS file for *the monitor this window is currently on* (`bread_theme::output_palette_path` / `output_css_path`). If the monitor can't be resolved it falls back to reacting to any per-output `*.json` / `*.css` write, so a change is never missed. `.tmp.<pid>` scratch files from `bread-theme`'s atomic writes are ignored. - Added a `WindowEvent::Moved` handler that re-renders with the new output's palette when the window lands on a different monitor (compares against the last monitor so a drag doesn't spam re-renders). - Factored `monitor_name` / `emit_current_theme` helpers out of the duplicated logic. ## Tests `cargo build --locked`, `cargo build --release --locked`, `cargo clippy --all-targets --locked -D warnings`, `cargo test --locked` (64 tests) — all pass. No frontend changes. No `Cargo.lock` change (both `bread-theme` helpers already in v0.7.4). No `.cargo/config.toml` patch in this repo. Note: `origin/main` is not `cargo fmt --check` clean (pre-existing, across ~12 files); this branch touches only `commands/theme.rs` and leaves it fmt-clean without reformatting the rest.
Breadway added 1 commit 2026-09-01 16:30:17 +08:00 
Audit findings #1 and #4.

#1 (accent mapping): render_theme_css hand-rolled the `:root { --name }`
block and mapped --accent, --red, --on-accent and --on-red all to
pywal's color1 (ANSI red — the slot bread-theme reserves for errors),
so the settings webview accented in a different hue from every GTK app
on the same monitor, and destructive-action styling was indistinguishable
from accent styling. Replace it with a direct call to
bread_theme::css_custom_properties / css_tokens (both exported in the
pinned v0.7.4), which format the canonical color_pairs list: --accent =
color4, --red = color1. As a bonus css_tokens emits a correct
font-family list instead of one over-quoted family.

#4 (live updates): watch_and_emit watched only the shared theme.css, so
`breadpaper set_on` on a non-focused monitor (which writes only
palettes/<out>.json + themes/<out>.css) never reached a settings window
parked there, and dragging the window between monitors never re-rendered.
Watch the generated-theme dir recursively and re-render when the shared
sheet or *this monitor's* per-output files change; add a
WindowEvent::Moved handler that re-renders when the window lands on a
different monitor.
This pull request is broken due to missing fork information.
View command line instructions

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/per-monitor-theme:fix/per-monitor-theme
git checkout fix/per-monitor-theme

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git checkout main
git merge --no-ff fix/per-monitor-theme
git checkout fix/per-monitor-theme
git rebase main
git checkout main
git merge --ff-only fix/per-monitor-theme
git checkout fix/per-monitor-theme
git rebase main
git checkout main
git merge --no-ff fix/per-monitor-theme
git checkout main
git merge --squash fix/per-monitor-theme
git checkout main
git merge --ff-only fix/per-monitor-theme
git checkout main
git merge fix/per-monitor-theme
git push origin main
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: Breadway/bos-settings#3
No description provided.
Delete branch "fix/per-monitor-theme"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?