Theme the tree-view rows so Theme Editor tokens stay readable (#5934). Principle VIII.

[aethersdr-agent]

Summary

Fixes #5934

What was changed

Theme the tree-view rows so Theme Editor tokens stay readable (#5934). Principle VIII.

Files modified

• src/gui/Theme.h
• src/gui/ThemeEditorDialog.cpp

 src/gui/Theme.h               | 22 ++++++++++++++++++++++
 src/gui/ThemeEditorDialog.cpp |  4 +++-
 2 files changed, 25 insertions(+), 1 deletion(-)

---

Generated by AetherClaude (automated agent for AetherSDR)

---

_{🤖 aethersdr-agent · cost: $3.9427 · model: claude-opus-5}

[aethersdr-agent]

Issue fit

Yes. #5934 opened as "font.family.freq is missing from the Theme Editor", and the thread (ve7atj's screenshot) redefined it: the token is there, the alternate rows are illegible. I re-derived the root cause independently rather than taking the body's word for it, and it holds: appStylesheetTemplate() had rules for QListWidget (Theme.h:128-133) but nothing for QTreeView/QTreeWidget, the Object list is a QTreeWidget with setAlternatingRowColors(true) (ThemeEditorDialog.cpp:248), and Qt only sets QPalette::AlternateBase from a stylesheet when alternate-background-color is present — otherwise the native palette wins while the text keeps color.text.primary. The fix reaches this dialog: applyAppTheme(this) at ThemeEditorDialog.cpp:162.

Scope

File / group	What it changes	Claimed by the issue?	Verdict
src/gui/Theme.h — QTreeView, QTreeWidget + ::item:selected	The reported defect, app-wide for every tree in an applyAppTheme() subtree	Yes	In scope
src/gui/Theme.h — QHeaderView::section	Themes all header views, tables included, not just tree headers	Partly — tree headers yes, table headers are a widening	In scope but undisclosed — nit 2
src/gui/ThemeEditorDialog.cpp:486	Replaces the hardcoded #607080 "inherited" foreground with color.text.secondary	Named in the triage comment, not in the PR body	In scope, and a measurable contrast win (below)

Nothing else in the diff. No settings keys, no capability surface, no CHANGELOG entry, no new tokens — all four tokens in the new block (color.background.0/1, color.text.primary, color.border.strong) are already used elsewhere in the same template, so there is no new resolution risk.

Blockers

None.

Nits (non-blocking)

• Two sibling surfaces still carry the identical defect. I walked every setAlternatingRowColors(true) site. The QListWidget ones were already covered; five tables live in dialogs that style themselves. Two of those style the table but never set alternate-background-color, so their alternate rows still fall back to native AlternateBase with light-grey text on Windows — exactly #5934:

  • src/gui/RadioHealthDialog.cpp:58-71 — setAlternatingRowColors(true) plus a themed sheet setting background + color only.
  • src/gui/NetworkDiagnosticsDialog.cpp:808 — alternating rows on; no alternate-background-color anywhere in that file.

  Not a reason to hold this PR (it fixes what was reported), but "fixes every themed tree at once" is true and "fixes this class of bug" is not. Worth a follow-up issue, or one line in each of those two sheets.

• Principle VIII is cited but the body carries no evidence. The template's test plan is absent entirely — no platform note, no before/after. The defect is specifically the Windows 11 native palette, which the authoring agent cannot have exercised. CI green (all 7 checks on 422e6dc, including check-windows and Static checks) proves it compiles, not that the rows are readable. Cheapest closure: ask @ve7atj to confirm on the build they screenshotted, in both Dark and Light.

• Inline: stale "inherited" foreground if the operator edits color.text.secondary itself (ThemeEditorDialog.cpp:486).

• Inline: QHeaderView::section reach (Theme.h:150).

What I tried to break

• Invalid brush from an unresolved token. QBrush(tm.color("color.text.secondary")) would paint garbage if the key were missing. It isn't: seeded at root in src/core/ThemeSeedGenerated.cpp:145 (#8ea8c0) and present in both bundled themes ({color.gray.400} dark, {color.gray.100} light). Survives.
• Is the "unreadable on light backgrounds" claim real, and does the new value regress Dark? I resolved the ramps and computed contrast both ways. Light (bg.1 #dde5ed): #607080 → 4.0:1, new #506070 → 5.08:1. Dark (bg.1 #1a2a3a): #607080 → 2.87:1, new #8ea8c0 → 5.93:1. The old value was worse on Dark, not just Light — the comment slightly misattributes it, but the change is an improvement in both. Survives.
• Per-widget sheets clobbered by the new app-level rules. Five files use trees. SettingsBrowserDialog (kDialogStyle, lines 58-62), RadioSetupDialog (755-759) and NetworkDiagnosticsDialog all carry their own, closer, equal-or-greater-specificity tree rules, so they are unchanged. NetworkDiagnosticsDialog.cpp:362-366 sets a per-item Qt::ForegroundRole for category rows, which wins over a view-level color:. Survives.
• Branch/expand indicators on newly-stylesheeted trees. The Theme Editor list is flat (addTopLevelItem only, ThemeEditorDialog.cpp:418), so there is no branch column to lose; the hierarchical trees elsewhere already ship their own ::branch rules (e.g. RadioSetupDialog.cpp:757). The one surface I could not reason to a conclusion is the non-native QFileDialog in ImageFileDialog.cpp:263-273, which is parented and so inherits these rules — worth a glance from anyone with a build.
• Hardcoded #000 on selection. Matches the existing QListWidget::item:selected precedent one line above, and black on accent (#00b4d8 dark / #0088b0 light) clears AA both ways. Not a finding.

All of the above is reasoned from the head checkout at /tmp/aetherclaude/pr-5938; I have no build and no GUI here, so nothing was reproduced at runtime.

Recommendation

Approve with nits. The root cause is correctly identified and independently confirmable from the code, the fix is minimal and sits in the canonical template where it belongs, and both colour changes measurably improve contrast in both bundled themes. Next step: have the reporter confirm the paint on Windows 11 in Dark and Light, and open a follow-up for the two tables in the first nit — no need to grow this PR.

---

_{🤖 aethersdr-agent · cost: $10.7536 · model: claude-opus-5}

[aethersdr-agent]

QHeaderView::section reaches further than "tree-view rows": it matches table headers too, app-wide within every applyAppTheme() subtree. I checked the blast radius — 15 files already define their own QHeaderView::section and are unaffected (their rule is closer and equal-specificity). The ones that newly change appearance are KiwiPublicReceiverPicker, MqttSettingsDialog, NetSchedulerDialog, SystemInfoDialog and the non-native QFileDialog in ImageFileDialog.cpp.

That's most likely an improvement, and tree headers genuinely need it — but it is a user-visible restyle of surfaces the issue never mentions. Please say so in the PR body so the maintainer is ruling on it knowingly rather than discovering it post-merge.

[aethersdr-agent]

Good change — I measured it and it improves contrast in both themes (Dark 2.87:1 → 5.93:1, Light 4.0:1 → 5.08:1 against background.1), and color.text.secondary resolves from the compiled-in seed, so there's no invalid-QColor path.

One consequence the old constant didn't have: this value is now baked into the item at populateRow() time. onActiveThemeChanged() (line ~1116) only calls refreshTokenList() when the active theme name changes; an in-place token edit repopulates just the row that was clicked. So editing color.text.secondary itself — in this very dialog, which is the one place people do that — leaves every other row's "inherited" cells painted in the previous colour until a scope switch or theme reload.

Cosmetic and self-correcting, not worth holding the PR. If you want it exact, having onActiveThemeChanged() also refresh when the edited key is color.text.secondary would close it.

[Ozy311]

The PR fixes the reported alternating-row defect in #5934, but introduces a selected-row readability regression for custom themes with a dark accent. Request changes for the new selection rule; the alternating-row and inherited-text changes can stay.

Scope

Change	Issue fit	Result
Theme.h: tree background, alternate background, text and border	Directly addresses the reporter's screenshot	Reproduced before/after; fixed
Theme.h: selected-item styling	Adjacent to the list fix	New dark-accent regression; inline finding below
Theme.h: generic header sections	Proposed in the issue thread, broader than the PR body's description	Reaches table headers too; existing discussion already covers this
ThemeEditorDialog.cpp: inherited foreground token	Proposed in the issue thread	Improves theme switching; existing live-edit refresh limitation remains

Blocker

1. P2 — Preserve readable selection with dark custom accents (Theme.h:146–149). The new rule forces black selected text on color.accent. With color.accent=#203040, the selected token name and value become almost invisible (nominal contrast 1.56:1). The same actual dialog on the merge base retains readable native selection. Removing only this new four-line selection block restores that selection while preserving the fixed alternating backgrounds. The inline suggestion applies that minimal correction.

Verification

Built the actual ThemeEditorDialog and its production theme/editor/settings dependencies in a standalone, socket-free harness on macOS, Qt 6.11.1, Fusion style, offscreen platform, isolated settings. Compared the two changed files from merge base 2859fdef with head 422e6dc5, and with a scratch variant deleting only the new selection block. This exercised the real dialog, not a reimplementation of the stylesheet.

• Default Dark alternate-row pixels: base #f7f7f7, head #1a2a3a; ordinary rows stay #0f0f1a. This reproduces the reported class of defect and its fix.
• Default Light rows on head: #f5f5f8 / #dde5ed. Dark → Light → Dark retained all 154 token rows, including font.family.freq.
• Dark custom accent: rendered the new dark-on-dark selected row, then demonstrated that deleting only the selected-item block restores readable native selection without losing the alternating-row fix.
• Confirmed the existing inherited-text refresh observation: editing color.text.secondary in place leaves existing inherited cells at their old brush; switching themes refreshes them.

This is a focused production-dialog reproduction, not native Windows verification or a full application/automation-bridge run. No radio connection or TX was involved. The seven checks succeeded for this head; Linux/macOS/Windows build steps actually ran. Those PR runs were created September 24 and do not validate the newer main tip. The full suite and native Windows visual confirmation were not run locally.

Existing non-blocking observations

I am not duplicating the active threads for header scope and inherited-cell live refresh. The PR body should also record the validation and broader header reach.

Snapshot: head 422e6dc54177cd7b7bf89b780bbd380d3d4f193e; merge base 2859fdef4c5121fe0d18f5d08f819ecbd9b33fc3; trusted current-main governance 33c0032c2b2d8ed9b74be3a5a80cf86c9930bbc2. Governance manifest PASS, required documents read, Constitution mirrors identical. Neither changed file nor the core theme/token resources changed between the merge base and current main. GitHub reports the commit signature verified / valid. Required human CODEOWNERS review remains separate.

— Ozy · Reviewed with Codex (GPT-6)

[Ozy311]

[P2] Keep selected rows readable when the theme accent is dark

With a custom theme whose color.accent is #203040, selecting a Theme Editor row now renders its name and value in black on that dark accent (1.56:1 nominal contrast). I reproduced this using the actual dialog at this head under Qt 6.11.1/Fusion/offscreen; the merge-base dialog kept readable native selection. The new literal also conflicts with the theme guide's “A contribution never introduces a new hardcoded colour literal.” The green CI checks do not exercise this rendering case. Removing this selection override preserves the alternating-row fix and restored readable selection in the same harness; alternatively introduce a proper selection-foreground semantic token with contrast validation. Minimal verified correction:

Suggested change

	QTreeView::item:selected, QTreeWidget::item:selected {
	background-color: {{color.accent}};
	color: #000;
	}

[ten9876]

The PR fixes the alternating-row readability defect clarified in #5934, but the added selected-item rule introduces another unreadable state for custom dark accents. Request changes on head 422e6dc54177cd7b7bf89b780bbd380d3d4f193e. I independently reproduced the earlier selection finding; the head has not changed since that review.

Scope

File/group	What changes	Claimed/explained?	Verdict
src/gui/Theme.h:140–145	Tree backgrounds, alternate backgrounds, text and border	Title/body and issue thread	Direct fix
src/gui/Theme.h:146–149	Selected tree item colours	Issue thread	In scope, but introduces blocker 1
src/gui/Theme.h:150–155	Generic header backgrounds, text, border and padding	Issue thread; absent from terse PR body	Also reaches tables; disclose scope
src/gui/ThemeEditorDialog.cpp:484–488	Inherited-cell foreground resolves a theme token	Issue thread	In scope; refresh limitation below

Everything in the diff is explained by the issue thread. The single commit has no unrelated authored-date outlier. No settings keys, public APIs, dependencies, removed guards, or CHANGELOG changes. The body has no scope checklist to contradict, but underdescribes the header and inherited-cell changes. The original row correction is a bug fix, not a visual preference; the broader header presentation should be made explicit for Jeremy's design review.

Blockers

1. P2 — Preserve readable selected text with dark custom accents (src/gui/Theme.h:146–149). The new rule forces color: #000 against arbitrary color.accent. With accent #203040, selecting font.family.freq in the actual production dialog renders black glyphs on that dark background (nominal contrast 1.56:1). The merge-base dialog retains readable native selection. Removing only this four-line rule restores that selection while keeping the alternating-row fix. This independently confirms the existing blocker rather than adding a new inline thread.

   Observed with Qt 6.11.2, Fusion, offscreen on Linux:

   merge base: selected blackPixels=0  bg=#308dc8
   PR head:    selected blackPixels=21 bg=#203040
   no-selection-rule mutation: blackPixels=0 bg=#308dc8
   

   Minimal correction: delete the new selection block. The resulting adjacency is:

        QTreeView, QTreeWidget {
            background-color: {{color.background.0}};
            alternate-background-color: {{color.background.1}};
            color: {{color.text.primary}};
            border: 1px solid {{color.border.strong}};
        }
        QHeaderView::section {
            background-color: {{color.background.1}};
            color: {{color.text.primary}};
            border: none;
            padding: 3px 6px;
        }

   Alternatively, introduce a properly themeable, readable foreground/background pair with appropriate design review. Copying an existing hardcoded list rule does not justify extending it: docs/style/theme-style-guide.md says, “A contribution never introduces a new hardcoded colour literal.” The blocker rests on the reproduced readability regression, not merely this convention.

Nits — non-blocking

• Live inherited-cell refresh (src/gui/ThemeEditorDialog.cpp:486, :540–552): existing cells retain a resolved brush when color.text.secondary is edited within the same theme. Actual dialog output: inherited before=#8ea8c0 after=#8ea8c0 token=#f12345 text=inherited. Refresh affected inherited cells when this token changes, without invalidating rows still in use. This improves on the old fixed brush but does not yet track live edits.
• Document the wider header reach and validation (src/gui/Theme.h:150–155): this selector also styles table headers; e.g. MemoryDialog.cpp:390 supplies table rules but no header override. Record that scope and before/after evidence in the body. Native Windows reporter confirmation remains outstanding.
• Shorten the historical comment (src/gui/Theme.h:134–139): retain the native AlternateBase constraint and issue reference; move the report narrative to the PR body.

What I tried to break / verification

• Built a socket-free harness using the actual ThemeEditorDialog, production theme/editor/settings dependencies and bundled resources. Compared merge base 2859fdef4c5121fe0d18f5d08f819ecbd9b33fc3, PR head, and a variant deleting only the new selection rule. No synthetic firmware peer or socket-owning test was introduced or executed.
• Original defect reproduced: Default Dark alternate background is #f7f7f7 on base, #1a2a3a on head, while text remains #c8d8e8. Default Light head alternates #f5f5f8 / #dde5ed. Both themes retain all 154 rows and font.family.freq. In this fixture other font rows occupy the unreadable alternate bands; the exact reporter's row ordering was not claimed reproduced.
• Read the sibling styles: Radio Setup, Network Diagnostics and Settings Browser tree-specific rules retain precedence. These conclusions are source-reviewed, not a claim that every sibling GUI was driven. No new lifecycle, persistence, threading or engine-boundary change was found.
• The separate code-review skill was unavailable; performed manual adversarial review plus an independent parallel scope audit. No PR tests were added, so there is no submitted regression test to mutation-check; the diagnostic selection-rule mutation above isolates causality.

Full AetherSDR Release build succeeded on Linux/Qt 6.11.2. Drove that exact PR-head binary through explicit socket /tmp/aethersdr-pr5938-review, offscreen, isolated settings/config and TX disabled. Connected explicitly to the simulator; never connected to the discovered live radio. Bridge evidence (relevant fields):

{"model":"AetherSDR Demo","serial":"DEMO-0001","connected":true,"transmitting":false}
{"action":"selectRow","class":"QTreeWidget","ok":true,"selectedRow":138,"selectedRowText":"font.family.freq","target":"ThemeEditorDialog/QTreeWidget"}
{"bytes":88401,"class":"ThemeEditorDialog","height":720,"ok":true,"path":"/tmp/pr5938-session/freq-selected.png","target":"ThemeEditorDialog","width":533}

invoke opened Theme Editor, switched Default Dark → Default Light → the isolated custom dark-accent theme, and selected the reported token. grab reproduced the black-on-dark selection in the full app as well. Closing/reopening the dialog survived and produced another successful capture. Disconnected the demo and used the owned application's Quit action; the instance exited. The full app was driven on head only; before/after and mutation evidence came from the focused production-dialog harness. The operator's existing app was left untouched.

All seven recorded checks passed for this head, but the pull_request CI runs were created 2026-09-24 12:26:59 UTC, before current main 8df6db2f (2026-10-04 18:26:40 UTC). Branch protection reports strict: false; those checks do not validate today's merge result. GitHub reports the commit signature verified/valid. No full test suite or native Windows visual run was performed. Recommendation: fix selected-row contrast, record validation and scope, then rerun checks on the updated branch. Approval remains the operator's decision.

[ten9876]

Issue fit: yes, for the alternating-row defect that #5934's thread clarified (the token was never missing; every other row was light text on the native near-white AlternateBase). The head is unchanged (422e6dc5) since the two earlier change requests, and it merges cleanly onto current main (git merge-tree against 85f2a1249: no conflicts). Neither changed file has moved on main except #6072's comment pass. One blocker remains, and I'm adding a canon citation to it.

Scope

File/group	Change	Claimed?	Verdict
src/gui/Theme.h:140-145	Tree background, alternate background, text, border	Yes	The fix
src/gui/Theme.h:146-149	QTreeView::item:selected → accent background with color: #000	In the issue thread only	Blocker 1
src/gui/Theme.h:150-155	QHeaderView::section: reaches every header, tables included	In the issue thread; not in the body	Nit 2
src/gui/ThemeEditorDialog.cpp:484-486	"inherited" foreground QColor(0x60,0x70,0x80) → color.text.secondary	In the issue thread	In scope, and an improvement

One commit, authored 2026-09-24. There are no settings, public surface, test or CHANGELOG changes. Socket preflight: no test sources touched.

Blockers

1. src/gui/Theme.h:146-149 adds a new hardcoded colour literal (color: #000). docs/style/theme-style-guide.md states the rule in its first sentence: "Every colour in AetherSDR resolves through a ThemeManager token. A contribution never introduces a new hardcoded colour literal." §3 adds: "No new hex/RGB literals in src/ UI code — not in QSS strings."

   CI is green only because the ratchet nets this to zero: the same PR deletes QColor(0x60,0x70,0x80), so audit_colours.py's total does not rise. That is a swap, not compliance.

   The runtime consequence is the one @Ozy311 and the 2026-10-04 review each reproduced independently. With a custom dark color.accent (e.g. #203040), the selected row is black text on a dark background (about 1.56:1), where the merge base kept readable native selection. I did not re-reproduce that dark-accent case. I'm relying on two independent pixel reproductions plus the canon sentence.

   Minimal fix: delete the four-line ::item:selected block. The resulting adjacency is in the 2026-10-04 review and is drop-in.

   Context for the operator, not a reason to keep the block: main already does the same thing in QListWidget::item:selected (Theme.h:90) and QMenu::item:selected (:167). A dark custom accent is therefore already unreadable in menus and lists app-wide. The durable fix is an on-accent text token used by all three selectors, which is a follow-up and not this PR's job. This PR shouldn't add a third instance.

Nits (non-blocking)

1. Three unresolved inline threads block the merge under main's required conversation resolution: Theme.h:149 (Ozy311), Theme.h:150 and ThemeEditorDialog.cpp:486 (aethersdr-agent).
2. Theme.h:150-155: QHeaderView::section themes every table header in the app, not just tree headers. That's probably right, but it isn't in the body. Please say so there, so the reach is searchable later.
3. Theme.h:134-139: the comment narrates the report ("the Theme Editor's Object list looked like it was missing rows"). Keep the native-AlternateBase constraint and the issue number, and move the rest to the PR body (AGENTS.md §"Comments").
4. Two sibling tables still carry the same defect on current main. Both set setAlternatingRowColors(true) but have no alternate-background-color (0 hits in either file): RadioHealthDialog.cpp:58 and NetworkDiagnosticsDialog.cpp:808. They're out of scope; a follow-up issue would cover them.

Verified empirically, and what I tried to break

• On current main, not only on the September base.
  • Setup: I built main 85f2a1249 plus this PR's diff (Release), and drove it offscreen with isolated settings, an explicit socket and AETHER_AUTOMATION_NO_TX=1.
  • Connected explicitly to DEMO-0001; get radio returned "model": "AetherSDR Demo" and "transmitting": false.
  • invoke "Theme Editor…" trigger then grab ThemeEditorDialog: in Default Dark, the Object list alternates background.0/background.1 with readable text.primary on both bands, which is the reporter's defect fixed.
• Did main already fix it some other way, making this redundant? No. main's Theme.h has no QTreeView/QTreeWidget/QHeaderView rule (only QListWidget, QMenuBar and QMenu selectors), and ThemeEditorDialog.cpp:484 still has the hardcoded slate.
• Does the ratchet catch the literal? It doesn't (net zero, above), which is why blocker 1 cites the guide rather than CI.
• Not tested: native Windows 11 (the reporter's platform), and the dark-accent selection, which rests on the two prior reproductions.
• CI is from 2026-09-24, before main's 2026-10-04 tip. The clean merge-tree is the current-base evidence.
• Auto-merge: squash auto-merge has been armed by aethersdr-agent since 2026-09-24, so approving after the fix merges it immediately.

Recommendation: Request changes, for blocker 1 only: drop the selection block. Everything else is ready.

— Jeremy KK7GWY + Claude

[chibondking]

@aethersdr: this PR is superseded by #6312, which fixes #5934 from my fork.

#6312 keeps what the three reviews accepted here and resolves the blocker:

• Kept: the QTreeView background, alternate-background-color, text and border (the actual fix), and the themed color.text.secondary for "inherited" cells.
• Dropped: QTreeView::item:selected { … color: #000 }, which was unreadable on a dark custom accent and added a hard-coded colour. Selection is left to the palette.
• Scoped: the header rule is now QTreeView QHeaderView::section, so table headers elsewhere don't change.
• Added: the "inherited" cells repaint on an in-place edit of color.text.secondary, which was this PR's review nit, plus a regression test in theme_manager_test (mutation-checked) and bridge before/after screenshots.

This PR's head hasn't changed since the change requests, so it can be closed in favour of #6312. That's the maintainers' call.

[ten9876]

Superseded by #6312 (merged as ba30526), which fixes #5934 with the tree-view row colours scoped so it avoids the selection and unscoped-header rules that held this one up. Closing.