cleanup(theme): swap kGreenToggle #006040 for color.background.success (#3141) by nigelfenton · Pull Request #3195 · aethersdr/AetherSDR

nigelfenton · 2026-05-26T19:54:23Z

Closes #3141.

Summary

Mechanical token swap in CatControlApplet.cpp::kGreenToggle:

background: #006040 → background: {{color.background.success}}
Stale comment block about "no dark-success-background token" removed (the token landed in PR feat(theme): Phase 5 PR 1 — Theme Editor dialog (live colour editing + Save As) #3130 alongside the Theme Editor work).

Why

The literal #006040 was bit-identical to the new dark token color.background.success, so the swap is visually-identical on Default Dark. The original concern was light theme: the hardcoded saturated dark-green on a light surround was unreadable. With the token, light theme now resolves to the soft mint #c8e8d0 defined in default-light.json.

Diff

// Before
"QPushButton:checked { background: #006040; color: {{color.accent.success}}; "
"border: 1px solid {{color.accent.success}}; }";

// After
"QPushButton:checked { background: {{color.background.success}}; "
"color: {{color.accent.success}}; "
"border: 1px solid {{color.accent.success}}; }";

Verification

Built on Windows 11 (MSVC + Ninja + Qt 6) on top of aethersdr/AetherSDR main at 9f3f1c4c. Smoke-tested both docked and floating CAT Control variants in both bundled themes:

Theme	Variant	`:checked` background	Result
Default Dark	docked	`#006040` (token resolves to bit-identical value)	unchanged ✓
Default Dark	floating pop-out	`#006040`	unchanged ✓
Default Light	docked	`#c8e8d0` soft mint	legible ✓
Default Light	floating pop-out	`#c8e8d0`	legible ✓

Screenshots of dark :checked (proof of pixel-identical regression) and light :checked (the legibility fix) available — happy to attach to the description in a follow-up commit comment.

Acceptance checklist (from #3141)

#006040 literal removed from kGreenToggle in CatControlApplet.cpp
Stale "no dark-success-background token" comment removed
Dark theme renders identically (token is bit-identical)
Light theme :checked button is now legible

cc @ten9876 @jensenpat

73 Nigel G0JKN

🤖 Generated with Claude Code

aethersdr#3141) The token landed in PR aethersdr#3130 alongside the Theme Editor work. The dark value is bit-identical to the previous literal (#006040), so dark theme renders identically; light theme :checked now uses the soft mint #c8e8d0 instead of the unreadable saturated dark green on a light surround. Stale "no dark-success-background token" comment also removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

jensenpat

Thank you Nigel for theme fix

@ten9876

…rrides (#3198) Second control-type carve-out following the slider + knob pattern from [#3188](#3188). References the toggle-buttons bullet under "follow-up sweeps" in [#3184](#3184). ## Why tribes (and not a single namespace) Where sliders have one canonical visual identity (track + fill), toggle buttons carry *semantic* colour: the "ON" state communicates meaning — enable / activate / warning / generic-mode — not just a value. A single `color.toggle.background.checked` token can't represent all four colour tribes that exist in the current codebase (green-success, blue-accent, amber-warning, deep-blue-panel), so the carve-out splits the **checked-state** tokens across three tribes: ```cpp enum class ToggleTribe { Accent, Success, Warning }; ``` Unchecked + disabled state styling is shared across tribes. Only the **Accent** tribe additionally carries per-applet overrides through the v2 scope tree — Success and Warning are semantic and look identical wherever they live. ## Namespaces (16 new tokens, both bundled themes) ``` color.toggle.background ← {color.gray.800} (alias to primitive) color.toggle.foreground ← {color.gray.200} color.toggle.border ← {color.gray.700} color.toggle.background.disabled ← {color.gray.900} color.toggle.foreground.disabled ← {color.gray.600} color.toggle.border.disabled ← {color.gray.900} color.toggle.accent.background.checked ← {color.blue.700} (TX red / RX green / comp amber per applet) color.toggle.accent.foreground.checked ← {color.blue.500} color.toggle.accent.border.checked ← {color.blue.500} color.toggle.success.background.checked ← #006040 / #c8e8d0 color.toggle.success.foreground.checked ← {color.green.500} color.toggle.success.border.checked ← {color.green.500} color.toggle.warning.background.checked ← #5a3a0a / #f5e8d0 color.toggle.warning.foreground.checked ← {color.amber.500} color.toggle.warning.border.checked ← {color.amber.500} ``` Also adds **`color.background.warning`** primitive (dark `#5a3a0a` / light `#f5e8d0`) for tribe symmetry with the existing `color.background.success` (which was added in PR #3130). All aliases resolve **directly to primitives** (single-hop) so the `resolveAlias()` lookup against `m_primitives` succeeds — chained semantic→semantic aliases don't resolve through the current path, which would land the literal `{color.background.1}` string into QSS / `color()` callers and break rendering. Caught and fixed during test development. ## Cascade — root → applet → applet/<name> Per-applet overrides live under `scopes.applet.scopes.<name>.tokens` alongside the existing slider + knob overrides. Single-token override (`background.checked` only) matches the slider precedent of one token per applet: ```json "applet": { "scopes": { "tx": { "tokens": { ..., "color.toggle.accent.background.checked": "{color.red.500}" } }, "rx": { "tokens": { ..., "color.toggle.accent.background.checked": "{color.green.500}" } }, "comp": { "tokens": { ..., "color.toggle.accent.background.checked": "{color.amber.500}" } } } } ``` ## What lands | File | Role | |---|---| | `resources/themes/default-dark.json`, `default-light.json` | Token defs + per-applet overrides + new `color.background.warning` primitive | | `src/core/ThemeManager.cpp` | `seedBuiltinDefaults` extension (16 token seeds + 3 applet-scope overrides) so user themes forked before this PR still resolve correctly | | `src/gui/Theme.h` | `ToggleTribe` enum + `applyToggleButtonStyle(btn, tribe)` helper + template | | `src/gui/CatControlApplet.cpp` | Drop inline `kGreenToggle` constant, route through helper (Success tribe) — single source of truth going forward | | `tests/theme_manager_test.cpp` | Coverage for base + per-tribe + per-applet cascade | | `docs/theming/toggle-button-tokens.md` | Following the slider-knob-tokens.md shape with the migration pattern for the next sweep | ## Why no global QSS rule Unlike sliders, **this PR does NOT add a `QPushButton:checked` rule to `appStylesheetTemplate`**. Every checkable button in the codebase that currently has no explicit `:checked` styling would have suddenly acquired the Accent-tribe look — a subtle but real visual regression for ~30+ sites that aren't part of this sweep. Opt-in via the helper keeps the namespace landing isolated; the global QSS rule can land in the follow-up after per-site auditing. ## Verification (Windows 11, MSVC + Ninja + Qt 6.10.3, branched from `aethersdr/AetherSDR` main at `4ee99f78`) **Unit tests:** - 10/10 new toggle assertions pass — base token validity, per-tribe checked values, per-applet cascade for accent (tx=`#ff4d4d`, rx=`#4dd87a`, comp=`#ffb84d`), success + warning stable across applet scopes, `isOverriddenAt` distinguishes own override from inherited. - The 4 pre-existing failures on this branch (lines 452/454/505/644 — `importThemeFromFile` related) are also present on clean `main` at the same source lines (398/400/451/590 there). **Not introduced here.** Looks like a Windows-vs-CI environment difference around temp dir handling. Happy to investigate separately if useful. **Theme Editor inspection (live cascade):** | Scope | `color.toggle.accent.background.checked` resolved | |---|---| | `root` | `#0070c0` (blue, alias to `{color.blue.700}`) | | `applet/tx` | `#ff4d4d` (red, alias to `{color.red.500}`) — verified in Editor columnar view | | `applet/rx` | `#4dd87a` (green) — verified in Editor | | `applet/comp` | `#ffb84d` (amber) — verified in Editor; columnar view shows root `#0070c0` / applet `inherited` / comp `#ffb84d` | **CatControlApplet helper-driven refactor:** - Default Dark, `:checked` → `#006040` dark green background, `#4dd87a` text/border — identical to pre-PR. - Default Light, `:checked` → `#c8e8d0` soft mint background, `#1a8040` text/border — identical to post-#3195. - Docked + floating variants both verified. - Live theme-switch (Dark↔Light without restart) re-renders cleanly via the helper's `applyStyleSheet` registration. **Cross-test sanity:** ran the full test suite. Other test failures (`async_log_writer_test`, `CAT_Flex_test`, `CAT_TS-2000_test`, `ole_compound_file_test`, `rigctld_test`) are unrelated to theme work and present on clean main — not introduced here either. ## One intentional addition to flag The new helper template includes a `QPushButton:disabled` rule that the original `kGreenToggle` constant didn't have. In practice the Enable CAT button is never set disabled in `CatControlApplet`, so this is invisible there. The addition is intentional — gives every toggle that opts into the helper a consistent disabled appearance using the `color.toggle.*.disabled` tokens. ## Out of scope (follow-up sweep) - **Per-site QSS migration** — `AetherDspWidget`, `AetherialAudioStrip`, `AntennaGeniusApplet`, `AppletPanel`, `ClientChainApplet`, `ClientCompApplet` (bypass toggle), `ClientCompEditor`, etc. Each currently has its own inline `QPushButton:checked` stylesheet; multiple sites per file, each needs a tribe judgement before migration. Mirrors the punt on hardcoded slider sites in #3188. - **Multi-token per-applet overrides** — current override on `applet/tx` only touches `color.toggle.accent.background.checked`; `foreground.checked` and `border.checked` stay at the root blue, which on a red background reads visually inconsistent (red bg + blue border + blue text). Adding `red.700`/`green.700`/`amber.700` primitives, or per-applet overrides of all three checked tokens, is the next design step. Documented in the Out of scope section of `toggle-button-tokens.md`. - **Indicator-style toggles** — `QCheckBox::indicator` / `QRadioButton::indicator` are a different visual primitive, separate namespace + sweep. - **Hover / pressed state tokens** — same deferral as sliders. - **`color.toggle.panel.*` tribe** — `AppletPanel.cpp`'s deep blue `#0a3060` for tab-like toggles doesn't fit Accent / Success / Warning cleanly. A fourth `panel` or `muted` tribe may earn its keep in a later sweep. - **Global `QPushButton:checked` rule** in `appStylesheetTemplate` — per-site auditing first. cc @ten9876 @jensenpat @chibondking 73 Nigel G0JKN 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Co-authored-by: Jeremy Fielder <kk7gwy@aethersdr.com>

nigelfenton requested a review from a team as a code owner May 26, 2026 19:54

jensenpat approved these changes May 26, 2026

View reviewed changes

jensenpat enabled auto-merge (squash) May 26, 2026 19:56

jensenpat merged commit 1fdc033 into aethersdr:main May 26, 2026
4 checks passed

nigelfenton mentioned this pull request May 26, 2026

feat(theme): toggle button namespaces with per-tribe + per-applet overrides #3198

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cleanup(theme): swap kGreenToggle #006040 for color.background.success (#3141)#3195

cleanup(theme): swap kGreenToggle #006040 for color.background.success (#3141)#3195
jensenpat merged 1 commit into
aethersdr:mainfrom
nigelfenton:cleanup/theme-success-token-3141

nigelfenton commented May 26, 2026

Uh oh!

jensenpat left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

nigelfenton commented May 26, 2026

Summary

Why

Diff

Verification

Acceptance checklist (from #3141)

Uh oh!

jensenpat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants