fix(theme): migrate hardcoded highlight and disabled-state colours to ThemeManager tokens. Principle IX.#3645
Merged
Conversation
… ThemeManager tokens. Principle IX. Fills three hardcoded-colour gaps left after aethersdr#3439 and aethersdr#3441. FreeDvReporterModel: removes kTxBg/kRxBg/kMsgBg/kHlFg constants and replaces their use sites with ThemeManager::instance().color(token). Adds a themeChanged->dataChanged{Background,Foreground} connection so highlighted rows repaint immediately on theme switch rather than waiting for the next 250ms highlight tick. Four new color.highlight.* tokens added to both theme files (values match the freedv-gui reference palette; color.background.tx is a chrome tint and could not be reused). RadioSetupDialog: migrates three QPushButton:disabled CSS blocks. Reboot Radio button uses new color.button.danger.*disabled tokens to preserve its intentional danger-red disabled appearance. Upload Firmware button corrects a prior partial migration that was using color.meter.bar.fill and color.background.1 as disabled-state stand-ins. Serial tab Open/Close buttons fix a silent bug: the style string contained {{token}} placeholders but was applied via plain setStyleSheet(), which does not run the template engine — tokens were silently discarded. Changed to applyStyleSheet() so tokens resolve and widgets participate in live theme switching. Six new color.button.* tokens added to both theme files. Fixes aethersdr#3446. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Reviewed the diff against the source. This is clean and well-scoped — thanks for the careful migration, @NF0T. 👍
Verified:
- All four
color.highlight.*and sixcolor.button.*token names used in C++ match exactly the JSON keys added to both theme files (scopes.root.tokens.color.{highlight,button}), and both files parse. Dotted keys (background.disabled,danger.background.disabled) follow the existingslider/toggle/knobconvention. - The consumed APIs all exist:
ThemeManager::color(const QString&), thethemeChangedsignal, andapplyStyleSheet. - The
themeChanged → dataChanged({BackgroundRole, ForegroundRole})connection in the constructor is correct and lifetime-safe (thisas context object), andindex(m_rows.size()-1, Col::Count-1)spans the right range. Replacing the four staticQColorconstants with livecolor(token)lookups indata()correctly closes the gap where a theme switch only repainted on the next 250 ms tick. - Serial-tab Open/Close move from
setStyleSheettoapplyStyleSheetso they now participate in live theme switching — consistent with how Reboot/Upload-Firmware already work. The remaining hardcoded hex in those templates is the non-disabled (normal/hover) state, which is out of this PR's scope; leaving it untouched is fine.
Non-blocking notes:
- Light-theme
color.button.background.disabledresolves to{color.gray.700}whileborder.disabledis{color.gray.600}— just confirming that's the intended ordering (700 darker than 600 for the fill behind a 600 border); reads fine, only flagging so it's a conscious choice rather than a swap. - The highlight tokens being identical across dark/light is acknowledged in the description and is reasonable — a future theme author can override per-scope.
No correctness, scope, or resource concerns. LGTM.
🤖 aethersdr-agent · cost: $6.2725 · model: claude-opus-4-8
ten9876
approved these changes
Jun 19, 2026
ten9876
left a comment
Collaborator
There was a problem hiding this comment.
Approving — clean, value-correct theming migration (Principle IX).
Verified:
- The high-visibility FreeDV row highlights are exact relocations (kTxBg #fc4500 → color.highlight.tx, kRxBg #379baf → .rx, kMsgBg #e58be5 → .message, kHlFg #000000 → .fg) — no visual change, just hardcoded → token.
- Token symmetry: all 10 new tokens defined in both default-dark.json and default-light.json; every token the two .cpp files reference resolves (no orphans).
themeChanged → dataChanged(Background/ForegroundRole)gives immediate repaint on theme switch.- Reboot (danger) disabled dark values are exact relocations preserving the red tint; serial Open/Close correctly moved
setStyleSheet → applyStyleSheetso tokens resolve + live theme switching works.
Minor/intentional: the generic button.*.disabled tokens introduce small deliberate changes on two low-visibility disabled states (fixing the semantically-wrong Upload Firmware tokens; standardizing serial buttons on the gray scale) — documented in the PR. CI green across all six. Nice work @NF0T.
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
2 participants
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
Follow-up to #3439 and #3441. Fills three hardcoded-colour gaps in the theme token system — the last surfaces in the FreeDV Reporter dialog and Radio Setup dialog that hadn't been migrated to ThemeManager.
FreeDvReporterModel — row highlight colours
QColorconstants (kTxBg/kRxBg/kMsgBg/kHlFg) and replaces their three use sites indata()withThemeManager::instance().color(token)calls.themeChanged→dataChanged{BackgroundRole, ForegroundRole}connection in the constructor so highlighted rows repaint immediately on a theme switch instead of waiting for the next 250 ms highlight tick.color.background.tx(#3a2a0e) is a chrome-panel tint used in 35+ surfaces and could not be reused here. The TX-row highlight (#fc4500) is a distinct high-visibility colour matching the freedv-gui reference palette, so new tokens were required for all four constants.RadioSetupDialog — button disabled states
QPushButton:disabledCSS blocks to tokens.buildRadioTab~line 601): uses newcolor.button.danger.*disabledtokens — preserves the intentional danger-red colouring in the disabled state.buildRadioTab~line 838): was using{{color.meter.bar.fill}}and{{color.background.1}}as disabled-state stand-ins (semantically wrong tokens from a prior partial migration). Now uses correctcolor.button.*disabledtokens.buildSerialTab~line 4054): hardcoded hex replaced withcolor.button.*disabledtokens;setStyleSheetcalls replaced withapplyStyleSheetso the template engine resolves tokens and the widgets participate in live theme switching.New tokens — 10 total across both theme files
color.highlight.tx#fc4500#fc4500color.highlight.rx#379baf#379bafcolor.highlight.message#e58be5#e58be5color.highlight.fg#000000#000000color.button.background.disabled#203040{color.gray.700}color.button.foreground.disabled{color.gray.500}{color.gray.500}color.button.border.disabled{color.gray.700}{color.gray.600}color.button.danger.background.disabled#2a1818#f0d8d8color.button.danger.foreground.disabled#8a6055#b08080color.button.danger.border.disabled#4a2828#d0a8a8Highlight colours are kept identical between dark and light themes — they are high-saturation reference palette colours from freedv-gui that provide adequate contrast with the black foreground on both backgrounds. A future theme author can override them via the token.
Fixes #3446.
Test plan
🤖 Generated with Claude Code