feat(theme): Phase 2 pilot — migrate ComboStyle to applyStyleSheet + tokens#3087
Merged
Conversation
…tokens First real consumer of the RFC #3076 Phase 2 theming infrastructure. ComboStyle.h is the central styling helper for every QComboBox in AetherSDR — applyComboStyle() is called from 20+ sites (AppletPanel, RadioSetupDialog, PhoneCwApplet, RxApplet, StripEqPanel, WaveApplet, SpectrumOverlayMenu, etc.). What changed: - Hardcoded hex (#1a2a3a, #c8d8e8, #304050, #00b4d8) replaced with canonical token references: {{color.background.1}}, {{color.text.primary}}, {{color.background.2}}, {{color.accent}}. - QColor(0x8a, 0xa8, 0xc0) in the arrow PNG replaced with ThemeManager::color("color.text.secondary"). - applyComboStyle() now routes through ThemeManager::applyStyleSheet so every combo gets free live re-theme on theme change (registered in the widget-tracked reverse-map for the Phase 5 inspector). - SpectrumOverlayMenu.cpp's two direct calls to comboStyleSheet() converted to applyComboStyle() — the underlying helper is gone. - Arrow PNG cache key now includes the resolved text.secondary hex so each theme variation gets its own cached arrow without stomping. Tradeoff documented inline: applyComboStyle installs TWO connections on themeChanged — the reverse-map's automatic re-apply, plus an explicit refresh to regenerate the arrow URL (since the URL is computed at apply time, not a {{token}} placeholder). The double- apply is wasteful but small; Phase 5 could introduce a "computed token" mechanism if combos become a hot path. QPointer guards against widget destruction between the connection and the next themeChanged. Verified locally: Linux build clean (all 20+ call sites still resolve), binary launches without crash. No visible diff expected today — the resolved colours match the previous hardcoded values exactly. Visible payoff arrives when the user switches themes: every combo re-themes simultaneously without any per-call-site plumbing. Phase 4's Default Light theme will demonstrate this end-to-end. Stacked on top of #3085 (applyStyleSheet infrastructure) — when that lands the pilot rebases cleanly to a single-commit diff against main. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2c034f1 to
ced96f1
Compare
G6PWY-Chris
pushed a commit
to G6PWY-Chris/AetherSDR
that referenced
this pull request
Jun 22, 2026
…tokens (aethersdr#3087) ## Summary First real consumer of the RFC aethersdr#3076 Phase 2 theming infrastructure. Migrates [src/gui/ComboStyle.h](src/gui/ComboStyle.h) — the central styling helper for every QComboBox in AetherSDR — from hardcoded hex to canonical token references via \`ThemeManager::applyStyleSheet\`. ## Why ComboStyle as the pilot - **High leverage:** \`applyComboStyle()\` is called from 20+ sites (AppletPanel, RadioSetupDialog, PhoneCwApplet, RxApplet, StripEqPanel, WaveApplet, SpectrumOverlayMenu, …) — one function migrated themes them all. - **Small surface:** single 54-line header, easy to review and rollback. - **Covers both code paths:** stylesheet template + paint-code QColor literal (the arrow PNG), exercising both \`resolve()\` and direct \`ThemeManager::color()\` lookup. ## Migration details **Hex → tokens:** | Was | Now | |---|---| | \`#1a2a3a\` | \`{{color.background.1}}\` | | \`#c8d8e8\` | \`{{color.text.primary}}\` | | \`#304050\` | \`{{color.background.2}}\` | | \`#00b4d8\` | \`{{color.accent}}\` | | \`QColor(0x8a, 0xa8, 0xc0)\` | \`ThemeManager::color(\"color.text.secondary\")\` | **Routing:** \`applyComboStyle\` now calls \`ThemeManager::applyStyleSheet\` instead of \`combo->setStyleSheet\`. Combos get free live re-theme on theme change via the widget-tracked reverse-map (PR aethersdr#3085). **Arrow PNG cache:** filename now includes the resolved \`text.secondary\` hex so each theme variation gets its own cached arrow (\`/tmp/aethersdr_combo_arrow_<hex>.png\`). **SpectrumOverlayMenu.cpp:** two direct calls to \`comboStyleSheet()\` (the underlying-now-removed helper) converted to \`applyComboStyle()\`. ## Tradeoff documented inline \`applyComboStyle\` installs **two** connections on \`themeChanged\`: 1. The reverse-map's automatic re-apply (from aethersdr#3085) 2. An explicit refresh lambda that regenerates the arrow URL The double-apply is wasteful but small. The arrow URL has to be computed at apply time (it's a filesystem path that depends on the resolved colour), not a \`{{token}}\` placeholder. Phase 5 could introduce a "computed token" mechanism if combos become a hot path. QPointer guards the lambda against widget destruction between the connection and the next theme change. ## Verified locally - [x] Linux build clean (all 20+ call sites still resolve) - [x] AetherSDR binary launches without crash (offscreen QPA, 3s smoke test) - [x] No visible diff expected today — resolved colours match the previous hardcoded values exactly ## Stacking note This PR is based on \`auto/theme-phase2-widget-map\` (PR aethersdr#3085) because it consumes \`ThemeManager::applyStyleSheet\` which only exists there. When aethersdr#3085 merges, this rebases cleanly to a single-commit diff against main. ## Test plan - [ ] CI green on the dependent aethersdr#3085 first - [ ] CI green here after rebase - [ ] Visual smoke: launch app, open Radio Setup dialog, confirm every combo box (profile picker, RCA/ACC, mic source dropdowns) renders identically to v26.5.3 - [ ] After Phase 4 Light theme lands: confirm theme switch repaints every combo simultaneously without a restart 73, Jeremy KK7GWY & Claude (AI dev partner) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.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 real consumer of the RFC #3076 Phase 2 theming infrastructure. Migrates src/gui/ComboStyle.h — the central styling helper for every QComboBox in AetherSDR — from hardcoded hex to canonical token references via `ThemeManager::applyStyleSheet`.
Why ComboStyle as the pilot
Migration details
Hex → tokens:
Routing: `applyComboStyle` now calls `ThemeManager::applyStyleSheet` instead of `combo->setStyleSheet`. Combos get free live re-theme on theme change via the widget-tracked reverse-map (PR #3085).
Arrow PNG cache: filename now includes the resolved `text.secondary` hex so each theme variation gets its own cached arrow (`/tmp/aethersdr_combo_arrow_.png`).
SpectrumOverlayMenu.cpp: two direct calls to `comboStyleSheet()` (the underlying-now-removed helper) converted to `applyComboStyle()`.
Tradeoff documented inline
`applyComboStyle` installs two connections on `themeChanged`:
The double-apply is wasteful but small. The arrow URL has to be computed at apply time (it's a filesystem path that depends on the resolved colour), not a `{{token}}` placeholder. Phase 5 could introduce a "computed token" mechanism if combos become a hot path.
QPointer guards the lambda against widget destruction between the connection and the next theme change.
Verified locally
Stacking note
This PR is based on `auto/theme-phase2-widget-map` (PR #3085) because it consumes `ThemeManager::applyStyleSheet` which only exists there. When #3085 merges, this rebases cleanly to a single-commit diff against main.
Test plan
73, Jeremy KK7GWY & Claude (AI dev partner)
🤖 Generated with Claude Code