Skip to content

feat(theme): Phase 2 migrate app-wide stylesheet to tokens + applyAppTheme()#3090

Merged
ten9876 merged 2 commits into
mainfrom
auto/theme-phase2-app-stylesheet
May 25, 2026
Merged

feat(theme): Phase 2 migrate app-wide stylesheet to tokens + applyAppTheme()#3090
ten9876 merged 2 commits into
mainfrom
auto/theme-phase2-app-stylesheet

Conversation

@ten9876

@ten9876 ten9876 commented May 25, 2026

Copy link
Copy Markdown
Collaborator

Summary

Second consumer of the RFC #3076 Phase 2 theming infrastructure. Migrates src/gui/Theme.h — the central app-wide stylesheet applied to MainWindow + every floating window — from hardcoded hex to canonical token references.

Scope

Theme.h styles the entire base widget palette: `QWidget`, `QGroupBox`, `QPushButton`, `QComboBox` (further overridden by ComboStyle for finer control), `QListWidget`, `QSlider`, `QMenuBar`, `QMenu`, `QStatusBar`, `QProgressBar`, `QSplitter` — roughly 30 hex literals worth of theming across 4 call sites:

  1. MainWindow (applyDarkTheme)
  2. PanFloatingWindow ctor
  3. FloatingContainerWindow ctor
  4. MainWindow applet float window setup

What changed

  • Theme.h — new `appStylesheetTemplate()` returns a `{{token}}` template (35 placeholder substitutions across ~14 widget rules)
  • applyAppTheme(widget) wraps `ThemeManager::applyStyleSheet` so every top-level window registers in the reverse-map and gets free live re-theme on theme change
  • darkThemeStylesheet() retained as a `[[deprecated]]` back-compat shim that routes through `resolve()` — bypasses reverse-map registration but still themed correctly, so the rolling migration can pick off any remaining sites one at a time
  • All 4 call sites converted to `applyAppTheme(this)`

Canonicalisation map (per docs/theming/canonical-tokens.md)

Old hex New token
`#0f0f1a` / `#111120` / `#0a0a14` `color.background.0`
`#1a2a3a` / `#161626` `color.background.1`
`#203040` (background context) `color.background.1`
`#203040` (border context) `color.border.strong`
`#c8d8e8` `color.text.primary`
`#00b4d8` `color.accent`

Sub-perceptual visual shifts at the canonicalised sites — documented as the intentional design choice.

Out of scope (follow-ups)

  • `#000` text-on-accent: still hardcoded. Should become a new `color.text.onAccent` token for Phase 4 Light-theme contrast tuning.
  • `13px` font size: still hardcoded. Phase 2 follow-up should canonicalise font sizes (the existing `font.size.normal` is 12px which doesn't match the app baseline).

Verified locally

  • Linux build clean (no new warnings from this change)
  • AetherSDR binary launches without crash (offscreen QPA smoke test)
  • No visible diff expected today — resolved colours match the previous hardcoded values within sub-perceptual canonicalisation shifts

Test plan

  • CI green: build + check-paths + check-windows + CodeQL
  • Visual smoke: launch app, pop out a panadapter (PanFloatingWindow), open Radio Setup dialog — confirm no visible regression vs the previous build
  • When Phase 4 Default Light theme lands: confirm theme switch re-themes MainWindow + every floating window simultaneously without a restart

73, Jeremy KK7GWY & Claude (AI dev partner)

🤖 Generated with Claude Code

…Theme()

Second consumer of the Phase 2 theming infrastructure.  Theme.h's
darkThemeStylesheet() is the central app-wide stylesheet applied to
MainWindow plus every top-level floating window (4 call sites total):
PanFloatingWindow, FloatingContainerWindow, MainWindow itself, and
the applet float window inside MainWindow.

This stylesheet styles the entire base widget palette: QWidget, QGroupBox,
QPushButton, QComboBox (overridden by ComboStyle for finer control),
QListWidget, QSlider, QMenuBar, QMenu, QStatusBar, QProgressBar, and
QSplitter — roughly 30 hex literals worth of theming.

Migrated:

  - Theme.h now exports appStylesheetTemplate() returning a {{token}}
    template (35 placeholder substitutions across ~14 widget rules)
  - New applyAppTheme(widget) wraps ThemeManager::applyStyleSheet so
    every top-level window registers in the reverse-map and gets free
    live re-theme on theme change
  - darkThemeStylesheet() retained as a [[deprecated]] back-compat shim
    that routes through resolve() — bypasses the reverse-map registration
    but still produces a correctly themed stylesheet, so the rolling
    migration can pick off remaining sites one at a time
  - 4 call sites converted from setStyleSheet(darkThemeStylesheet()) to
    applyAppTheme(this) / applyAppTheme(widget)

Canonicalisation choices (per docs/theming/canonical-tokens.md):

  #0f0f1a / #111120 / #0a0a14   →  color.background.0
  #1a2a3a / #161626             →  color.background.1
  #203040 (background contexts) →  color.background.1
  #203040 (border contexts)     →  color.border.strong
  #c8d8e8                       →  color.text.primary
  #00b4d8                       →  color.accent

Sub-perceptual visual shifts at sites where close variants collapsed
(e.g. #111120 list bg → #0a0e14 background.0).  Documented as the
intentional design choice in the canonicalisation doc.

Out of scope (follow-ups):
  - #000 text-on-accent: still hardcoded.  Should become a new
    color.text.onAccent token for Phase 4 Light-theme contrast tuning.
  - 13px font size: still hardcoded.  Phase 2 follow-up should
    canonicalise font sizes (the existing font.size.normal token is
    12px which doesn't match the app baseline).

Verified locally: build clean, AetherSDR binary launches without
crash.  No visible diff expected today — resolved colours match the
previous hardcoded values within sub-perceptual canonicalisation
shifts.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ten9876 ten9876 requested a review from a team as a code owner May 25, 2026 00:41
CI build failed at link step for container_widget_test:

  FloatingContainerWindow.cpp.o: undefined reference to
    AetherSDR::ThemeManager::instance()
  FloatingContainerWindow.cpp.o: undefined reference to
    AetherSDR::ThemeManager::applyStyleSheet(QWidget*, QString const&)

Cause: FloatingContainerWindow.cpp now calls applyAppTheme() (from
Theme.h) which inlines a call to ThemeManager::instance().applyStyleSheet().
Three container_*_test executables include FloatingContainerWindow.cpp
but were missing ThemeManager.cpp + its logging deps from their
target_sources list.

Fix: add ThemeManager.cpp + LogManager.cpp + AsyncLogWriter.cpp to all
three container_widget_test / container_manager_test /
container_nesting_test target sources.

Verified locally: all three tests build clean and pass under
QT_QPA_PLATFORM=offscreen.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@ten9876 ten9876 requested a review from a team as a code owner May 25, 2026 00:49
@ten9876 ten9876 merged commit 5e889aa into main May 25, 2026
4 checks passed
@ten9876 ten9876 deleted the auto/theme-phase2-app-stylesheet branch May 25, 2026 01:08
ten9876 added a commit that referenced this pull request May 25, 2026
The Phase 2 mass migration in this PR rewrote setStyleSheet sites in
CwxPanel.cpp and HelpDialog.cpp to route through
ThemeManager::resolve().  Two test executables link those .cpp files
directly but didn't list ThemeManager.cpp + its logging deps, so they
failed at link with:

  undefined reference to AetherSDR::ThemeManager::instance()
  undefined reference to AetherSDR::ThemeManager::resolve(QString const&)

Fix: add ThemeManager.cpp + AppSettings.cpp + LogManager.cpp +
AsyncLogWriter.cpp to both test target source lists.  Same recurring
class of bug already fixed for container_*_test in #3090 — bot/PR
authors miss the downstream test-target link surface when a header
change adds a new function call across translation-unit boundaries.

Verified locally: both targets link clean, both tests pass under
QT_QPA_PLATFORM=offscreen.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ten9876 added a commit that referenced this pull request May 25, 2026
The Phase 2 mass migration in this PR rewrote setStyleSheet sites in
CwxPanel.cpp and HelpDialog.cpp to route through
ThemeManager::resolve().  Two test executables link those .cpp files
directly but didn't list ThemeManager.cpp + its logging deps, so they
failed at link with:

  undefined reference to AetherSDR::ThemeManager::instance()
  undefined reference to AetherSDR::ThemeManager::resolve(QString const&)

Fix: add ThemeManager.cpp + AppSettings.cpp + LogManager.cpp +
AsyncLogWriter.cpp to both test target source lists.  Same recurring
class of bug already fixed for container_*_test in #3090 — bot/PR
authors miss the downstream test-target link surface when a header
change adds a new function call across translation-unit boundaries.

Verified locally: both targets link clean, both tests pass under
QT_QPA_PLATFORM=offscreen.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…Theme() (aethersdr#3090)

## Summary

Second consumer of the RFC aethersdr#3076 Phase 2 theming infrastructure.
Migrates [src/gui/Theme.h](src/gui/Theme.h) — the central app-wide
stylesheet applied to MainWindow + every floating window — from
hardcoded hex to canonical token references.

## Scope

Theme.h styles the entire base widget palette: \`QWidget\`,
\`QGroupBox\`, \`QPushButton\`, \`QComboBox\` (further overridden by
ComboStyle for finer control), \`QListWidget\`, \`QSlider\`,
\`QMenuBar\`, \`QMenu\`, \`QStatusBar\`, \`QProgressBar\`, \`QSplitter\`
— roughly **30 hex literals** worth of theming across 4 call sites:

1. MainWindow (applyDarkTheme)
2. PanFloatingWindow ctor
3. FloatingContainerWindow ctor
4. MainWindow applet float window setup

## What changed

- **Theme.h** — new \`appStylesheetTemplate()\` returns a \`{{token}}\`
template (35 placeholder substitutions across ~14 widget rules)
- **applyAppTheme(widget)** wraps \`ThemeManager::applyStyleSheet\` so
every top-level window registers in the reverse-map and gets free live
re-theme on theme change
- **darkThemeStylesheet()** retained as a \`[[deprecated]]\` back-compat
shim that routes through \`resolve()\` — bypasses reverse-map
registration but still themed correctly, so the rolling migration can
pick off any remaining sites one at a time
- All 4 call sites converted to \`applyAppTheme(this)\`

## Canonicalisation map (per docs/theming/canonical-tokens.md)

| Old hex | New token |
|---|---|
| \`#0f0f1a\` / \`#111120\` / \`#0a0a14\` | \`color.background.0\` |
| \`#1a2a3a\` / \`#161626\` | \`color.background.1\` |
| \`#203040\` (background context) | \`color.background.1\` |
| \`#203040\` (border context) | \`color.border.strong\` |
| \`#c8d8e8\` | \`color.text.primary\` |
| \`#00b4d8\` | \`color.accent\` |

Sub-perceptual visual shifts at the canonicalised sites — documented as
the intentional design choice.

## Out of scope (follow-ups)

- \`#000\` text-on-accent: still hardcoded. Should become a new
\`color.text.onAccent\` token for Phase 4 Light-theme contrast tuning.
- \`13px\` font size: still hardcoded. Phase 2 follow-up should
canonicalise font sizes (the existing \`font.size.normal\` is 12px which
doesn't match the app baseline).

## Verified locally

- [x] Linux build clean (no new warnings from this change)
- [x] AetherSDR binary launches without crash (offscreen QPA smoke test)
- [x] No visible diff expected today — resolved colours match the
previous hardcoded values within sub-perceptual canonicalisation shifts

## Test plan

- [ ] CI green: build + check-paths + check-windows + CodeQL
- [ ] Visual smoke: launch app, pop out a panadapter
(PanFloatingWindow), open Radio Setup dialog — confirm no visible
regression vs the previous build
- [ ] When Phase 4 Default Light theme lands: confirm theme switch
re-themes MainWindow + every floating window 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant