feat(theme): Phase 5 PR 1 — Theme Editor dialog (live colour editing + Save As)#3130
Merged
Conversation
…+ Save As)
First user-facing surface of the theming subsystem. Operators can now
edit any colour token in the active theme and ship a custom theme to
disk, all without touching a JSON file.
## What ships
View → Theme Editor… opens a modeless QDialog:
* "Editing: <active theme>" header so you always know what you're
mutating.
* Filter bar — type-to-match against the dotted token names.
* Sorted list of every colour token, one per row, with a 16×16
swatch and the current hex value. Click a row → QColorDialog
opens seeded with the current value. Accept the dialog → calls
ThemeManager::setColor() → emits themeChanged → every widget
registered through applyStyleSheet repaints on the next event-
loop turn. No restart required, no manual refresh.
* "Save As…" prompts for a theme name and writes m_tokens to
`~/.config/AetherSDR/themes/<name>.json` via
saveCurrentThemeAs(). The new theme is registered in the path
map, made active, and shows up in the View → Theme submenu
immediately (when that lands from #3129).
## ThemeManager additions
* `allTokenKeys()` — sorted enumeration of every loaded token,
used by the editor to populate its list.
* `setColor(token, color)` — in-memory mutation + themeChanged.
Hex round-trip so cssFragment() / resolve() pick up the new
value the same way they pick up loaded tokens. No-op on a
same-value write to avoid burning a repaint cycle.
* `setSizing(token, int)` — same idea for integer tokens (panel
padding, border widths). Not yet exposed in the editor UI;
will land in PR 2 when font + sizing get their own surface.
* `saveCurrentThemeAs(name)` — rebuilds nested JSON from the flat
dotted-key map (so on-disk shape matches the bundled themes),
writes to the user themes dir, registers + activates. Handles
QString / int / double / bool scalars + the existing
ThemeGradient type (with a full {type, angle, stops} round-trip
so the waterfall colormap and slice.dim tokens persist
correctly even though the editor's current pass-1 surface
doesn't let you edit them).
## What this PR explicitly does NOT do
* Inspector mode (click-on-widget to find tokens that paint it) —
Phase 5 PR 2. ThemeManager::tokensForWidget() already exposes
the reverse map populated by applyStyleSheet, so the wiring is
a few hundred lines once we settle on the latch/unlatch UX.
* Gradient editing for waterfall.colormap.{default,grayscale,…}
and slice.dim.{a..h}. The list skips those tokens via the
brush().gradient() probe so they don't render as
misleading-first-stop swatches. Phase 5 PR 3.
* Font / sizing editing — Phase 5 PR 2.
* Import (file picker for arbitrary JSON), drag-and-drop, theme
deletion, theme renaming — Phase 5 PR 4.
## Build verified clean
AetherSDR + all 320 targets including every test target.
## Phase status
✅ Phase 1: ThemeManager scaffolding + JSON loader
✅ Phase 2: gradient tokens + applyStyleSheet reverse-map + mass
migration
✅ Phase 3: paint-code migration + canonical-table closeout
✅ Phase 4: Default Light theme (#3129, in review)
🛠 Phase 5: editor UI — this PR is the first cut
73, Jeremy KK7GWY & Claude (AI dev partner)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Thanks Jeremy — really nice cut. Clean separation between the editor UI and the in-memory mutation API on ThemeManager, the WA_DeleteOnClose + destroyed-signal cleanup pattern matches the rest of MainWindow, and AppSettings/lcGui conventions are followed. A few things worth a look before merging:
1. Alpha channel is silently dropped on save (real bug)
ThemeEditorDialog::onTokenRowClicked opens the picker with QColorDialog::ShowAlphaChannel, but ThemeManager::setColor stores the value via color.name() — which defaults to HexRgb and strips alpha:
// ThemeManager.cpp
const QString hex = color.name(); // <- no alpha
m_tokens.insert(token, QVariant(hex));So users can pick a translucent colour, see the swatch update for the RGB component, and then have alpha silently quietly truncated on the next reload. Either drop the ShowAlphaChannel flag, or use color.name(QColor::HexArgb) and double-check the loader round-trips #AARRGGBB.
2. "Save As…" contradicts its own intent on bundled themes
The comment says:
Disallow overwriting a built-in (bundled :/themes/*.json) — that would silently shadow it from the user dir which is confusing.
…but the code just checks availableThemes() (which mixes built-ins + user themes) and offers Yes/No. If the user clicks Yes for a name like "Default Dark", we happily write ~/.config/AetherSDR/themes/Default Dark.json and shadow the bundled one — exactly the case the comment says we want to prevent. Either:
- track which themes are bundled (e.g. path starts with
:/) and refuse those outright, or - update the comment to match the implemented behaviour (warn + allow shadow).
3. The nested-JSON walk in saveCurrentThemeAs is more complicated than it needs to be
The current loop heap-allocates a new QJsonObject(child) per path component per token and hand-unwinds a stack of {parent, key} pairs. It appears to balance, but it's hard to convince yourself of that on a read, and silent breakage here would corrupt every saved theme. A small recursive helper that takes (QJsonObject& parent, const QStringList& parts, int idx, const QJsonValue& leaf) is ~6 lines and QJsonObject is value-typed so you don't need heap allocation at all. Not a blocker, but it's the riskiest code in the PR — worth simplifying while the surface is small.
Minor
QDir().mkpath(userDir)return is unchecked. The subsequentf.open(WriteOnly)will surface mkdir failures, so this is fine in practice, but a one-liner check would let you give a nicer error message than the generic "Could not write the theme file".- Nit:
m_themeLabel->setStyleSheet("QLabel { font-weight: bold; }")bypasses the token system — fine for a single bold label, but if you ever want operators to retheme it,applyStyleSheetwith a token-bearing template is the consistent path.
Deferred-scope list reads clearly; happy that gradient/sizing/font tokens are filtered out of the list rather than rendering misleading swatches. Phase split looks right.
73, AetherClaude
🤖 aethersdr-agent · cost: $6.9820 · model: claude-opus-4-7
…dits (SEGV)
Crash repro: open the Theme Editor, click any row, drag the QColorDialog's
gradient slider to a new colour, accept the dialog. Application segfaults
in QPixmapIconEngine::tryMatch via ThemeEditorDialog::updateRow().
Root cause — use-after-free:
1. User clicks a row → onTokenRowClicked() opens QColorDialog
2. User accepts a new colour → tm.setColor() → emits themeChanged
3. Signal delivery synchronously calls onActiveThemeChanged()
4. onActiveThemeChanged() called refreshTokenList() unconditionally,
which m_tokenList->clear() destroys every existing QListWidgetItem
5. Back in onTokenRowClicked(), the now-dangling `item` pointer hits
updateRow(item) → SEGV inside Qt's icon-cache lookup
The themeChanged signal fires for BOTH cases the editor cares about:
* User switched the active theme (Save As, View → Theme menu) —
needs a full rebuild because every token's value just changed
* User edited a single colour in-place via setColor / setSizing —
must NOT rebuild because the calling row updates itself
Distinguish the two by tracking the active-theme NAME the editor last
rendered against. Only do the wholesale rebuild when the name changes.
src/gui/ThemeEditorDialog.h
+ m_lastRenderedTheme member (QString)
src/gui/ThemeEditorDialog.cpp
* Constructor: seed m_lastRenderedTheme = activeTheme() right after
the initial refreshTokenList() so the first edit after the dialog
opens doesn't surprise-rebuild.
* onActiveThemeChanged(): always update the title (cheap), but only
refreshTokenList() if the active theme name actually changed.
73, Jeremy KK7GWY & Claude (AI dev partner)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
# Conflicts: # src/gui/MainWindow.cpp
Yesterday's PR #3110 darkening-workaround flow had me dropping a hand-fixed "Default Dark.json" into ~/.config/AetherSDR/themes/ to bypass the Qt resource rebuild. That file stuck around, and because ThemeManager::scanAvailableThemes() runs the user-dir scan after the built-in scan with an unconditional `m_themePaths.insert(name, full)`, the stale user-dir copy SHADOWED the bundled :/themes/default-dark.json. The stale copy predated PR #3122's waterfall-colormap restructure (flat single gradient → nested object with 5 named schemes). After it loaded: * color.waterfall.colormap = ThemeGradient (single, the old flat shape) * color.waterfall.colormap.{default,grayscale,blueGreen,fire,plasma} = MISSING from m_tokens The SpectrumWidget cache rebuild fell back to the black→white grayscale ramp for all 5 schemes — the dropdown still listed them but every selection rendered identical greyscale, which is exactly the symptom Jeremy reported. This commit: * Treats built-in names (paths starting with `:/themes/`) as RESERVED. A user-dir file with the same `name` field is logged with a clear warning and skipped, so the bundled version wins. * Comment documents the failure mode + points readers at "Save As under a new name" as the correct workflow for tweaked themes. The stale file itself was deleted out-of-band — this commit prevents the symptom from recurring on any other developer / user who happens to have an old user-dir Default Dark.json sitting around (anyone who followed yesterday's workaround in particular). Save As in the Theme Editor was already disallowing same-name overwrites via QMessageBox, so the editor itself can't manufacture this state. This guard catches the manual / migration case. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Repro: edit a custom theme in the Theme Editor → Save As "My Test Theme"
→ AppSettings stores ActiveTheme=My Test Theme. Later delete the
~/.config/AetherSDR/themes/My Test Theme.json file (intentionally or
accidentally — out-of-band cleanup, fresh install, sync skew, etc.).
Relaunch.
Pre-fix flow:
ThemeManager ctor:
seedBuiltinDefaults() ← scalar canonical tokens only,
NO color.waterfall.colormap.* gradients
setActiveTheme("My Test Theme")
→ m_themePaths.find("My Test Theme") returns end()
→ returns false silently
qCWarning(...): "failed to load theme My Test Theme — using
compiled-in defaults"
Result: m_tokens stays at seedBuiltinDefaults. The waterfall
cache rebuild reads tm.brush("color.waterfall.colormap.default")
etc., gets "missing brush token" for all 5 schemes, falls back to
the black→white grayscale ramp for every preset. Every scheme
renders identical grayscale despite the dropdown listing all 5 —
exactly what Jeremy hit after using the editor.
Other tokens (slice, accent, etc.) ARE in seedBuiltinDefaults so
the rest of the UI looks fine; only the waterfall is broken,
which makes the bug look much weirder than it is.
Fix: when the saved theme name is unresolvable, fall back to
"Default Dark" explicitly. The bundled JSON has all 5 colormap
gradients so the waterfall starts working immediately; setActiveTheme()
also writes "Default Dark" back to AppSettings so the recovery is
durable across the next launch.
If the bundled Default Dark itself fails to load (rebuild-resources
edge case), log a louder warning so future debuggers know it's not
a runtime issue.
src/core/ThemeManager.cpp
* Constructor's saved-theme fallback path now retries with
"Default Dark" instead of leaving m_tokens at the seed defaults.
* Comment block points at the waterfall failure mode so the next
person to read the code understands why "limp along" was wrong.
src/gui/SpectrumWidget.cpp
* Drop the temporary diagnostic qCDebug() from
rebuildWfStopsCacheFromTheme — it caught this bug; no need to
keep logging once per scheme on every theme switch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…TW wisdom
Three related fixes surfaced while testing the theme editor end-to-end.
## 1. Singular config dir (~/.config/AetherSDR/, not ~/.config/AetherSDR/AetherSDR/)
QApplication sets both organizationName and applicationName to "AetherSDR",
so QStandardPaths::AppConfigLocation resolves to ~/.config/AetherSDR/AetherSDR/
— a double-nested directory. AppSettings and the log dir already worked
around this by using GenericConfigLocation + "/AetherSDR" (see main.cpp
and AppSettings.cpp), but five other consumers had NOT been migrated and
were writing/reading from the doubled path:
src/core/AudioEngine.cpp — FFTW wisdom file
src/core/ChannelStripPresets.cpp — ChannelStrip.settings
src/core/FirmwareStager.cpp — firmware/ subdir
src/core/ThemeManager.cpp — user themes dir (scanAvailableThemes)
src/core/ThemeManager.cpp — user themes dir (saveCurrentThemeAs)
src/gui/MainWindow.cpp — signal_classifier.onnx model
Switching each to GenericConfigLocation + "/AetherSDR" puts all user
data under the singular ~/.config/AetherSDR/, matching AppSettings's
location. Existing files in the doubled dir need a one-time manual
migration (mv ~/.config/AetherSDR/AetherSDR/* ~/.config/AetherSDR/);
the app does not auto-migrate because the wisdom regen on first launch
is the only visible cost.
## 2. saveCurrentThemeAs round-trip data loss (gradient drop + prefix conflict)
The deep-walk serializer in saveCurrentThemeAs had two latent bugs that
only surfaced when an operator hit Save As:
* Prefix conflict — a flat scalar token (`color.accent` = "#00b4d8")
and a flat scalar with a longer key (`color.accent.bright` =
"#00c8f0") both exist at the same depth in the bundled themes.
Split-on-dots produced parts that overlapped, and whichever token
was iterated second clobbered the first depending on hash order.
The shorter scalar was silently dropped on every save.
* Gradient drop — `canConvert<ThemeGradient>()` returned false in
some metatype-registration timings, sending every waterfall.colormap.*
and slice.dim.* token to the `else continue;` branch. The resulting
JSON was missing all 5 colormap schemes + the slice dim block.
Both result in a "looks correct, loads broken" theme file. The combined
effect on @ten9876's Default-Dark-clone save:
* No `waterfall.colormap.*` tokens after load → SpectrumWidget's cache
fell back to a black→white grayscale ramp for every preset
* No `color.accent` scalar → button accents went transparent
Rewrites the serializer to emit a flat dotted-key object:
{"tokens": {"color.background.0": "#0f0f1a",
"color.accent": "#00b4d8",
"color.accent.bright": "#00c8f0",
"color.waterfall.colormap.default": {"type": "linear-gradient", ...},
...}}
Less pretty than the bundled themes' nested layout, but every token
round-trips perfectly with zero conflict surface. flattenTokens()
already accepts this shape (it just concatenates the prefix to each
key; flat dotted keys at the top level give the same flat m_tokens).
Future-Phase-5 polish can re-group into the bundled nested layout once
the editor is doing more than dump-and-reload.
Also switches the gradient detection from `canConvert<ThemeGradient>()`
to an explicit `userType() == qMetaTypeId<ThemeGradient>()` check, with
a defensive `qRegisterMetaType<ThemeGradient>()` call at the top of the
ThemeManager constructor so the metatype id is always valid.
## 3. Modeless FFTW wisdom progress dialog
NR2 wisdom generation runs on a background thread (QThread::create
inside enableNr2WithWisdom), but the progress dialog was
Qt::ApplicationModal — locking the operator out of the radio for the
many-minute wisdom run. Switched to Qt::NonModal + Qt::Tool +
WA_ShowWithoutActivating so the dialog floats above the main window
without claiming a taskbar entry or stealing focus. The Cancel button
and QDialog::rejected wiring keep their existing semantics.
## Build verified clean
All 320 targets including every test target.
73, Jeremy KK7GWY & Claude (AI dev partner)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Existing operators have user data sitting at QStandardPaths::AppConfigLocation:
Linux: ~/.config/AetherSDR/AetherSDR/
Windows: %LOCALAPPDATA%/AetherSDR/AetherSDR/
macOS: ~/Library/Preferences/AetherSDR/AetherSDR/
This release moves every consumer (FFTW wisdom, ChannelStrip presets,
firmware/, user themes, signal_classifier.onnx) from AppConfigLocation
to GenericConfigLocation + "/AetherSDR" — so files now live at the
singular ~/.config/AetherSDR/ (or platform equivalent), alongside
AppSettings and the log dir which already used that convention.
Without a migration, an existing operator's first launch of this build
would:
* Force a full FFTW wisdom regeneration (multi-minute, blocking pre
this PR; modeless after it)
* Lose ChannelStrip presets and firmware downloads until manually
moved
* Re-download the ONNX signal-classifier model (~85 MB)
This commit drops a one-shot migration block into main() right after
the QApplication is constructed (so QStandardPaths is usable) and
before AppSettings::load() or LogManager init (so subsequent subsystems
see the new layout). The block:
* Computes both paths. Bails immediately if they're identical
(e.g., if a future Qt resolves AppConfigLocation differently and
they coincide) or if the old dir doesn't exist (fresh install /
post-first-migration).
* For each entry at the top level of the old dir, moves it to the
new dir via QFile::rename(). Files AND directories both work —
on Linux/macOS/Windows rename() handles either as long as src and
dst share a filesystem (which they do by construction).
* Skips any entry whose target already exists — that path was
presumably populated by the new release before the operator ran
this build, so the new copy wins by being more recent. Logs the
skip so the operator can intervene if needed.
* After the moves, tries to rmdir the old directory. If anything
unrecognised remained (skipped collisions, files from other tools
we don't know about), the rmdir fails harmlessly and the old dir
stays put. Otherwise the empty doubled dir gets removed.
Idempotent: subsequent launches early-return because QDir(oldDir).exists()
returns false.
73, Jeremy KK7GWY & Claude (AI dev partner)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The AetherSDR config root was getting crowded with two unrelated log
streams sharing the same level as the actual settings files:
Application debug logs (LogManager-rotated, 50+ files easily):
aethersdr-YYYYMMDD-HHMMSS.log
aethersdr.log (symlink to latest)
SpotHub source feeds (one per data source, append-only):
dxcluster.log rbn.log wsjtx.log
spotcollector.log freedv.log pota.log
pskreporter.log
Now they each get their own subdir:
~/.config/AetherSDR/logs/ ← app debug logs
~/.config/AetherSDR/spothub/ ← per-source feeds
## Changes
src/main.cpp
* logDir = configRoot + "/logs"; (was configRoot)
* mkpath configRoot + "/spothub" so the SpotHub clients don't have
to each mkpath defensively.
* One-shot migration of existing root-level log files into the
new subdirs. Best-effort QFile::rename() with skip-on-collision;
removes the now-stale aethersdr.log symlink (LogManager will
recreate it at the new location).
src/core/LogManager.cpp
* Default activeLogFilePath() returns .../logs/aethersdr.log.
src/core/SpotCollectorClient.cpp
src/core/FreeDvClient.cpp
src/core/PotaClient.cpp
src/core/WsjtxClient.cpp
src/core/DxClusterClient.cpp
* logFilePath() returns .../spothub/<name>.log instead of root.
src/core/SupportBundle.cpp
* No change — already derives the log dir from
LogManager::logFilePath()'s absolutePath, so it follows the
LogManager change automatically.
## Migration
Runs alongside the doubled→singular config-dir migration in main.cpp.
Idempotent: subsequent launches early-return because the source files
no longer exist at the root.
73, Jeremy KK7GWY & Claude (AI dev partner)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Anticipated by the CAT-server PR (#3131): K5PTB had to hardcode `#006040` for the CAT-active :checked button background because the canonical taxonomy had `color.background.tx` (warm orange — TX indicator) but no green sibling for the "success / active" state. Adds the missing token in both bundled themes and the seedBuiltinDefaults fallback so #3131 (and any future widget that needs a green-active background tint) can use `{{color.background.success}}` instead of hardcoding hex. Values picked to match the existing accent-tier conventions: default-dark.json → #006040 (matches K5PTB's hardcode + the existing dim color.accent.success entry) default-light.json → #c8e8d0 (pale green, light enough that dark text reads on it — mirrors how background.tx is a warm light cream in the light theme) seedBuiltinDefaults → #006040 (matches the dark theme so the seed fallback path doesn't regress active widgets if the JSON load somehow fails) A follow-up tool-pass (tools/migrate_colours.py) can sweep CatPopoutWindow.cpp and CatControlApplet.cpp once #3131 lands to canonicalize the rest of the hardcoded styles in that PR (~8 more hex literals, all already-canonical token matches). 73, Jeremy KK7GWY & Claude (AI dev partner) Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 25, 2026
Closed
ten9876
added a commit
that referenced
this pull request
May 25, 2026
…e backdrop (#3148) ## Summary Closes the alpha-handling correctness gap that PR #3130's colour picker exposed, lays the inspector-coverage groundwork for custom-paint widgets, and adds a single-token "glass mode" backdrop that lets operators dial app translucency without flipping every widget at once. ## Alpha pipeline fix The colour picker returned `QColor` with `alpha < 255`, but every downstream step quietly stripped it: | Step | Bug | |---|---| | `setColor()` | `QColor::name()` defaults to `#rrggbb` — alpha lost at storage | | `gradientCssFragment()` | explicit `QColor::HexRgb` for stops | | `cssFragment()` | returned raw "#aarrggbb" — Qt SS parser ignores it | | `saveCurrentThemeAs()` | bare `.name()` on gradient stops on save | New helpers `colorToTokenString()` and `colorHexToCssFragment()` route all four: - Storage: `HexArgb` when alpha < 255, `HexRgb` otherwise (bundled-theme strings stay byte-identical) - Stylesheet emit: translates translucent hex to `rgba(r, g, b, a)` so Qt actually honours alpha - JSON round-trip: preserves the 8-digit form on save Theme Editor swatch now sits over a 2×2 checkerboard so translucent colours are visually distinguishable; hex label shows the 8-digit form when alpha < 255. ## `themeRegions()` infrastructure Per [RFC #3076](#3076): ```cpp struct ThemeRegion { QString token; std::function<bool(QPoint localPos)> hitTest; QString description; }; void declareWidgetRegions(QWidget* widget, const QList<ThemeRegion>& regions); QStringList tokensAtPoint(const QWidget* widget, const QPoint& localPos) const; ``` `tokensAtPoint()` returns just the tokens whose `hitTest()` matches the click point. Falls back to the coarse `tokensForWidget()` list when no region claims the point — guarantees the inspector always surfaces something useful. ThemeEditorDialog's inspector now routes through `tokensAtPoint()` with proper coordinate-space mapping up the parent chain. SpectrumWidget calls `declareWidgetTokens()` with its 25 painted tokens (background tiers, spectrum trace/peakHold/average/grid, all 8 slice colours + tx, waterfall.colormap, text tiers, accents). Sub-region splits between panadapter, waterfall, and slice triangles are a follow-up. ## Glass-mode backdrop A new `color.background.app` token paints MainWindow's full client area through a custom `paintEvent` override, with `Qt::WA_TranslucentBackground` enabled. Default is opaque (`#0f0f1a` dark, `#f5f5f8` light) — existing installs see zero visual change. But operators can now edit the token via Theme Editor and dial alpha down to watch the desktop show through wherever child widgets don't have explicit opaque fills. This is the unblocking architectural change for a future "glass" theme variant. ## Test plan - [x] Builds clean (`--target all`) - [x] Visual regression check — `color.background.app` defaulted opaque; no diff vs main when theme isn't edited - [x] Alpha picker: dialled `color.accent` down to ~50% opacity — Qt now honours rgba() - [x] Glass-mode A/B: alpha on `color.background.app` showed compositor desktop wallpaper through unclaimed regions (KWin / KDE Plasma X11) - [x] Inspector: clicks on the spectrum/waterfall area now report `SpectrumWidget: 25 tokens` (was: "no tokens registered") - [ ] Glass-mode audit: identifying the widgets that bleed desktop when they shouldn't — separate follow-up PR ## Follow-ups already on the roadmap - Glass-mode audit — add opaque fills to widgets that don't paint their own background (separate PR, open-ended scope) - SpectrumWidget sub-region splits via `declareWidgetRegions()` (panadapter / waterfall / slice triangles) - Gradient editor — in-dialog stop-strip widget for waterfall.colormap and slice.dim (PR 3b) - Phase 5 PR 4: font + sizing pickers, draft layer, delete/rename, reset-to-default 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4 tasks
jensenpat
pushed a commit
that referenced
this pull request
May 26, 2026
#3141) (#3195) 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 #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 ```cpp // 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) - [x] `#006040` literal removed from `kGreenToggle` in `CatControlApplet.cpp` - [x] Stale "no dark-success-background token" comment removed - [x] Dark theme renders identically (token is bit-identical) - [x] Light theme `:checked` button is now legible cc @ten9876 @jensenpat 73 Nigel G0JKN 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ten9876
added a commit
that referenced
this pull request
May 27, 2026
…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>
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
1 participant
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.
Summary
First user-facing surface of the theming subsystem. Operators can now edit any colour token in the active theme and ship a custom theme to disk, all without touching a JSON file.
View → Theme Editor… opens a modeless dialog with:
ThemeManager additions
Explicitly NOT in this PR (deferred to Phase 5 PR 2-4)
Build verified clean
Phase status
Test plan
73, Jeremy KK7GWY & Claude (AI dev partner)
🤖 Generated with Claude Code