[w5jwp]

Summary

Closes #6744. Part of #6239. The Icom RC-28 Button Mapping window
(RC28MappingDialog) moves from PersistentDialog to a CanonWindow
dialog (Kind::Dialog), as the AetherRX/AetherTX Settings and AGC-T
Calibration dialogs did:

• Chrome: no title bar, the canon ground, the canon header and the
  corner close button. It closes with the ✕, Close, Esc or Ctrl+W (⌘W).
  Like the other canon tool windows it installs FramelessResizer, so any
  edge resizes it, and it keeps its geometry under the existing
  RC28MappingDialogGeometry key. The rectangle the PersistentDialog
  saved is read once and rewritten as x,y,w,h, so the window reopens where
  the operator left it (centered on the main window the first time). The
  activity log's right-click menu keeps a canon panel. It no longer follows View →
  Frameless Window, so the Icom RC-28 Remote Encoder... menu action
  creates it directly instead of through showOrRaisePersistent(). The
  mappingFieldChanged wiring is unchanged.
• Title and entry point: unchanged: "Icom RC-28 Button Mapping",
  reached from Settings → Controllers → Icom RC-28 Remote Encoder....
• Look: it sits under the canon/rc28Mapping theme scope with one canon
  style sheet, using the same vocabulary as Radio Setup:
  • each section is a raised card with its title in cyan above it;
  • key labels (VID:PID, Path, Serial, Firmware, Pulses per step) are muted;
  • combo boxes, the slider and the buttons are canon controls, and the
    activity log is nested, with canon scroll bars;
  • the radio buttons and check boxes are the painted canon indicators
    (canonIndicatorRules());
  • the Pulses per step slider has no tick marks (a styled groove draws
    none); the number beside it shows the step.
• Status: the dot and status line are restyled from an rc28State
  property, with theme tokens for each state: muted when not connected,
  color.accent.success when connected and color.accent.warning when
  there are multiple devices. Every hard-coded colour literal in the file
  is gone.
• Layout: in the Encoder card, Pulses per step now sits below the
  two check boxes (Invert encoder direction, Auto-snap…) instead of
  between them.
• Size: minimum width 460 (was 420), which makes room for the canon
  margins. The minimum height is 683 px offscreen (the old dialog was 671),
  so it still fits a 1366×768 screen: card titles take an 18 px margin,
  cards are inset 8,2,8,4, and the activity log has a 48 px minimum that
  grows with the window.
• Accessibility (docs/a11y.md): the status line's accessible name
  carries its state ("RC-28 connection status: Not connected", the device
  name or the multiple-devices warning) and changes with it; the Pulses per
  step slider (now the buddy of its label, with a range description), the
  activity log and its Clear button carry accessible names.
• Shared canon scroll bars: the canon QScrollBar block that seven
  canon windows each carried a copy of is now one
  canonScrollBarRules() (src/gui/CanonScrollBarStyle.h), appended to
  the sheets of AetherModem, SpotHub, AetherGPS, Network Diagnostics,
  AetherMap, Radio Setup and this window. An offscreen render of both scroll
  bars under each window's old and new sheet matches pixel for pixel in
  Default Dark and Default Light.

Docs:

• usb-control-surfaces.md says how the window opens, resizes and closes, and lists
  the Encoder controls in their new order.
• docs/style/* list Icom RC-28 Button Mapping among the canon windows,
  the ones that save their geometry and install the resizer, and
  canon/rc28Mapping among the scopes; theme-style-guide.md documents
  canonScrollBarRules().
• The touchpoint manifest counts one more core/ThemeManager.h includer
  (ui-support, no radio state).
• No screenshot in screens.json shows this window, so nothing goes stale.

Intent

After this merges, the RC-28 window is a canon dialog that follows the
theme, matching AetherControl and the other canon windows. Its controls,
saved settings and behaviour are unchanged. Out of scope: combo-box
drop-down arrows (Radio Setup's canon combo boxes have none either) and
any change to the HID dispatch path.

The maintainer picked the CanonWindow move (an unlisted #6239 pick, the
same answer as #6734) and the Encoder reorder. Contrast note: the
multiple-devices warning uses the shared color.accent.warning token, about
3.1:1 on canon raised in Default Light, under docs/a11y.md's 4.5:1; it is
a shared token, not changed here.

Constitution principle honored

Principle VIII (Evidence Over Assertion) covers the checks below. The
visual direction is RFC #6226's.

Test plan

• Local build passes (cmake --build build, macOS, HID enabled)
• Driven through the automation bridge with scratch settings
  (AETHER_SETTINGS_DIR), no radio connected. Opened from Icom RC-28
  Remote Encoder... and grabbed: the canon header, five section cards,
  canon indicators and controls, and the "Not connected" status.
• Lifecycle: opening the window twice gives one window; both the
  Close button and the bridge's close remove it, and the menu action
  creates it again.
• Theme: switched View → Theme → Default Light with the window
  open, then back to Default Dark; it re-themes live both ways, with
  canon indicators and controls in each.
• Accessibility: dumpTree reports the names RC-28 connection status: Not connected,
  Pulses per step, RC-28 activity log and Clear activity log; the
  bridge drove the slider by its accessible name to 4, and after a close
  and reopen it read 4 (then set back to 1).
• canon_window_test passes locally.
• New rc28_mapping_dialog_test (29 checks, registered where hidapi is
  found, socket-free, null encoder) passes locally. It covers: CanonWindow;
  FramelessResizer installed; a legacy base64 rectangle restores under
  RC28MappingDialogGeometry and a hide saves x,y,w,h; the log's
  right-click menu is opaque; the Pulses per step value fits "10"; the
  button-assignment combos read the saved field, write a change back and
  emit mappingFieldChanged; the minimum height fits 700 px; the status
  line's accessible name and warning color follow
  onMultipleDevicesDetected(); Esc, Ctrl+W / ⌘W, the corner button and the
  footer Close each destroy the WA_DeleteOnClose window, as the
  PersistentDialog did. It passes offscreen on Linux (800 px screen) as
  well as macOS.
  • Mutations: removing FramelessResizer::install, setGeometryKey, the
    QMenu rules, the stateful accessible name, the tighter margins, the
    warning rule or the Close wiring each fails a check.
  • CI does not run it before merge: it is not in ci.yml's frozen per-PR
    ctest -R steps, so the green checks prove it compiles; it runs in
    full-suite.yml.
• audit_colours, check_a11y, check_qsettings,
  check_engine_boundary and check_theme_seed pass;
  gen_touchpoint_manifest.py --check is up to date.
• Not exercised: a real RC-28 (the connected and multiple-devices
  status colours haven't been seen on screen), and the docs site build
  (npm run build; no Node on the build machine, so CI's Static checks
  are the docs gate).

Checklist

• Commits are signed
• No new flat-key AppSettings calls. The geometry stays under the
  existing RC28MappingDialogGeometry key, with CanonWindow's one-time
  conversion from the old format.
• Code is clean-room
• All meter UI uses MeterSmoother (n/a)
• Documentation updated: docs/user/docs/usb-control-surfaces.md,
  docs/style/*, docs/architecture/aetherd-touchpoints.md
• Security-sensitive changes: n/a

🤖 Generated with Claude Code

[w5jwp]

After:

[aethersdr-agent]

Issue fit

Partially. #6744 asks for five things and the diff delivers four of them cleanly: the canon chrome and header, the canon/rc28Mapping scope with every hex literal gone, the three-state status dot driven off a property, and the unchanged mappingFieldChanged / RC28Mapping / HidEncoderInvertDir behaviour. I verified the latter by reading the whole changed file, not just the hunks: populateActionCombo and all four setting writes are byte-identical, and no widget lost a connect.

The gap is #6744's chrome bullet read against canon: docs/style/dialog-patterns.md:276-277 says a tool window on CanonWindow "also installs FramelessResizer, since CanonWindow itself only moves," and this one doesn't (finding 2). And the issue itself is the author's own, opened 25 minutes before the PR and still labelled maintainer-review — it isn't a ruling, so it can't authorise the pick the PR needs (finding 1).

Two body claims I could test and which held: "Every hard-coded colour literal in the file is gone" — confirmed, grep for #[0-9a-f]\{6\} in RC28MappingDialog.cpp returns nothing. And tools/check_a11y.py src/gui/RC28MappingDialog.cpp → 0 findings, run against the head checkout.

Scope

File(s)	Change	Claimed?	Verdict
src/gui/RC28MappingDialog.{h,cpp}	PersistentDialog → CanonWindow, sheet to tokens, a11y names, Encoder row reorder	Yes	In scope — except the reorder, see finding 1
src/gui/MainWindow_Menus.cpp:592-622	direct construction + WA_DeleteOnClose instead of showOrRaisePersistent	Yes	In scope and correct; MainWindow.h:1770 is a QPointer, so this is safe
docs/style/{dialog-patterns,aethersdr-style-guide,theme-style-guide}.md	adds this window and canon/rc28Mapping to the canon lists	Yes	In scope, but it self-certifies the pick — see finding 1
docs/user/docs/usb-control-surfaces.md	open/close sentence, Encoder row order swapped	Yes	In scope; the row swap follows the code change
docs/architecture/aetherd-touchpoints.md	core/ThemeManager.h 183 → 184	Yes	In scope — the new #include "core/ThemeManager.h" is real; generator-consistent

Nothing in the diff is unexplained, no new public/protocol surface, no settings key added, no CHANGELOG.md entry. The one removal worth naming is deliberate and disclosed: geometry persistence (nit 1).

Findings

1. src/gui/dialog-patterns.md / this whole PR — needs a maintainer decision, not a review sign-off. docs/style/dialog-patterns.md:273-277:

Use CanonWindow only for windows the style guide covers: short-lived windows, and the tool windows the maintainer has moved to it (Waveforms, Network Diagnostics, …). … Other tool and workspace dialogs keep the persistent pattern above.

RC-28 Button Mapping is a persistent tool dialog, and it is not on that list. #6239's "Persistent tool dialogs (one PR per dialog)" bullet sits under the token-migration heading — surfaces whose widgets move from base tokens to color.canon.*, keeping PersistentDialog chrome — and the CanonWindow candidate list there is a separate, closed set. So "Part of #6239" authorises the re-skin half of this PR but not the chrome move; merging is the pick, because docs/style/aethersdr-style-guide.md:118 and dialog-patterns.md:238 are edited to state the maintainer already made it. The sibling PR #6735 discloses exactly this about itself ("this full CanonWindow move is not on #6239's CanonWindow lists … please don't merge before the pick"); this body doesn't, which is the only thing I'd ask the author to change about it.

Folded in here rather than listed separately, because it needs the same ruling: the Encoder reorder is a preference change, not a fix. src/gui/RC28MappingDialog.cpp:366 moves vbox->addLayout(sensRow) below m_autoSnapCheck, reordering working UI. Nothing cites the old order as a defect — #6744 just asserts the new one, and the body calls it "a small layout change made at the contributor's request" where the contributor is the PR author. Per GOVERNANCE.md that's the maintainer's call; it's cheap to drop from this PR if the answer is no, since it's one statement.

2. src/gui/RC28MappingDialog.cpp:186-190 — the window can no longer be resized at all, and canon asks a tool window on CanonWindow to fix that. The constructor sets a minimum width and the canon sheet, and nothing installs the resizer:

    setMinimumWidth(460);
    applyCanonSheet(bodyWidget(), [] {

CanonWindow is frameless with no caption, and FramelessResizer is what supplies edge resizing — dialog-patterns.md:276-277: "A tool window on it also installs FramelessResizer, since CanonWindow itself only moves." As a PersistentDialog this window had only a minimum width, so the operator could widen it; the Button Assignments combo boxes and the Activity log are exactly the content someone widens. AGC-T Calibration gets away without a resizer because it is short-lived; a controller-configuration window the operator keeps open while pressing buttons is the tool-window case the sentence is about. Fix is one line in the constructor:

    setMinimumWidth(460);
    FramelessResizer::install(this);

(plus #include "FramelessResizer.h"). If the intent really is a fixed-size dialog, say so in the body and the docs line, since that contradicts the sentence this PR is editing.

3. src/gui/RC28MappingDialog.cpp:70-73 — the Activity log's right-click menu will draw with no background. The sheet opens with a universal transparent rule:

QWidget {
    color: {{color.canon.inkSoft}};
    background: transparent;
}

Qt applies a widget's style sheet to its children, and QPlainTextEdit's standard context menu (Copy / Select All, available because the log is read-only but selectable) is parented to the edit — so QMenu matches that QWidget rule and loses its panel. This is the same defect the maintainer saw on #6735 ("the menu on the RF and Tune power fields drew no background at all"), and the canon dialog that already has a text view carries the rule for it — src/gui/Ax25HfPacketDecodeDialog.cpp:184-186:

QMenu {
    background: {{color.canon.raised}};
}

It's a regression this PR introduces: before, m_log carried only its own QPlainTextEdit sheet, so the menu was default-styled. AGC-T and AetherRX Settings don't hit it because neither has a text view. Add to kRc28Style, after the QPlainTextEdit block:

QMenu {
    color: {{color.canon.inkSoft}};
    background: {{color.canon.raised}};
    border: 1px solid {{color.canon.lineHi}};
}
QMenu::item:selected {
    background: {{color.canon.cyan}};
    color: {{color.canon.onAccent}};
}
QMenu::separator {
    background: {{color.canon.line}};
    height: 1px;
}

I could not open the menu — headless, no build — so this is reasoned from the cascade plus the #6735 precedent, not observed.

Nits (non-blocking)

• src/gui/RC28MappingDialog.cpp:186 — dropping geometry persistence is disclosed in the checklist, but it's a user-visible removal that costs one line to avoid: CanonWindow::restoreGeometryFromSettings reads a PersistentDialog-era base64 value and rewrites it as x,y,w,h, so setGeometryKey(QStringLiteral("RC28MappingDialogGeometry")); would carry the saved position over and retire the key instead of orphaning it. #6735 took that route for TxBandDialogGeometry. If centring is the deliberate choice, fine — but then the orphan key is dead state nothing will ever clean up.
• src/gui/RC28MappingDialog.cpp:353 — m_sensitivityValueLabel->setFixedWidth(18); was sized when the label carried font-size: 11px from kLabelStyle. The new sheet sets no font size for plain QLabel, so this label now renders at the app default and "10" may clip in 18 px. Font-dependent, so I can't measure it headless — either drop the fixed width for setMinimumWidth(18), or let the slider's value speak through the accessible name alone.
• src/gui/RC28MappingDialog.cpp:386-395 + MainWindow_Menus.cpp:599 — WA_DeleteOnClose means the Activity log's history no longer survives a close and reopen, where showOrRaisePersistent used to keep the instance alive and hidden. The placeholder ("while the dialog is open") makes this defensible, but it's a behaviour change the body lists as "unchanged".
• No test, as the body says. Not withholding a merge for it — the canon-dialog shape has a deterministic widget seam (canon_window_test exists; #6735 and #6733 both ship mutation-checked widget tests for exactly this migration), so a handful of checks — it's a CanonWindow, Escape/Ctrl+W/Close don't write a setting, the four combos still carry their saved values after a close and reopen — would be cheap and would have caught findings 2 and 3's class. Worth it if you take the resizer fix.

What I tried to break

• Lifetime, the obvious suspect in a showOrRaisePersistent → WA_DeleteOnClose swap. It holds: MainWindow.h:1770 is QPointer<RC28MappingDialog>, so the pointer nulls on destruction and the if (!m_rc28MappingDialog) guard at :597 re-creates rather than dereferencing freed memory. I checked the three readers outside the hunk — MainWindow_Controllers.cpp:1072, :3158, :3168 — and all three null-check before appendButtonEvent. The connect sits inside the creation branch, so reopening doesn't stack duplicate handlers, and it passes this as receiver context. Escape/Ctrl+W route through QDialog::reject(), which CanonWindow.cpp:129-131 documents as hiding without a close event — benign on either reading, because a hidden-not-deleted instance is simply re-show()n by the menu action.
• CodeGuard's CRITICAL CS-VLN-CMDI-AMP — false positive, and not even in this diff. MainWindow_Menus.cpp:1748 is const auto shortcutText = [this](const QString& id) { inside the Window-menu refresh lambda; the & is a C++ reference declarator, the "&Window" on the line above is a Qt mnemonic, and the whole file contains no QProcess, system(, popen or exec* sink. (security-audit (CodeGuard): no findings, same conclusion.) Dropped, not repeated as a finding.
• Token resolution, since a bad token silently paints nothing: every name the sheet uses — canon.{ink,inkSoft,muted,cyan,aqua,raised,nested,control,line,lineHi,onAccent} plus accent.success / accent.warning — exists in resources/themes/default-dark.json. canon/rc28Mapping as a container string matches the canon/agcCalibration precedent at AgcCalibrationDialog.cpp:247, and needs no theme-file entry, so the docs-only change to theme-style-guide.md is right.
• "Distrust green", specifically whether this file was compiled at all — the whole translation unit is behind #ifdef HAVE_HIDAPI. ci.yml:626-633 runs setup-hidapi.ps1 on Windows and :906 brews hidapi on macOS, so check-windows and check-macos (both green on 86b79cb) did compile it; the green is meaningful here rather than vacuous. "Build docs site" is also green, which covers the npm run build the author couldn't run.
• What I could not check: anything requiring a running GUI. The status-dot colours, the centring, the context-menu background in finding 3, and the label clipping in nit 2 are all reasoned from code and from canon, not reproduced. The author's own body already flags that the connected and multiple-devices colours have never been seen on screen.

Recommendation

Needs maintainer decision. The engineering is careful — correct QPointer lifetime, every literal gone, accessible names added, a clean docs trail — and I found nothing that will break a user at runtime beyond finding 3's cosmetic menu. But merging ratifies an unlisted CanonWindow pick plus a layout preference, which is yours to grant, and the PR body should say so the way #6735 does.

Concrete next step: if you take the pick, ask for the FramelessResizer::install(this) line and the QMenu block before merge — both are small, both are canon-cited, and together they're the difference between this window matching the other canon tool windows and merely looking like them. If you don't want the Encoder row reorder, that's a one-line revert independent of everything else.

Thanks for the thorough disclosure in the body — the "Not exercised" bullet naming the untested status colours and the absent test made this much faster to review honestly.

---

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

[aethersdr-agent]

Cisco CodeGuard — static analysis of this PR (1 finding(s))

• [CRITICAL] CS-VLN-CMDI-AMP — Vulnerability: CS-VLN-CMDI-AMP in src/gui/MainWindow_Menus.cpp /tmp/aetherclaude/pr-6745/src/gui/MainWindow_Menus.cpp:1748

_{Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.}

---

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

[w5jwp]

Thanks for the review. Addressed in a4777393:

Finding 2, resizer: fixed. FramelessResizer::install(this), as dialog-patterns.md:276 asks for a tool window on CanonWindow.

Finding 3, log menu with no panel: fixed. kRc28Style now has the same QMenu block as TX Band Settings (raised panel, lineHi border, cyan selection, control.unavailable for disabled items, line separators).

Nit 1, geometry: taken. setGeometryKey("RC28MappingDialogGeometry"), so the old base64 rectangle is read once and rewritten as x,y,w,h. The user doc now says the window reopens where you left it and resizes from any edge.

Nit 4, tests: added rc28_mapping_dialog_test (18 checks, registered under if(HIDAPI_FOUND) and in AETHER_SETTINGS_CONSUMERS, null encoder, socket-free). It covers:

• CanonWindow and the installed resizer;
• legacy geometry restoring under the key, and a hide saving x,y,w,h;
• an opaque log menu;
• the Pulses per step value fitting "10";
• the combos reading the saved field, writing a change back and emitting mappingFieldChanged.

Mutations: removing the resizer fails 1 check, removing the geometry key fails 2, and removing the QMenu rules fails 1. The test isn't in the frozen per-PR gate, so CI only compiles it. The PR body says so.

Two nits didn't hold up:

• Nit 2, the "10" label clipping: measured, and no change made. horizontalAdvance("10") is 14 px in the macOS system font (.AppleSystemUIFont 13 pt, cocoa) and 14 px offscreen, inside the 18 px fixed width. A mutant using 18 px passes the new width check, so the check stays as a guard and setFixedWidth(18) stays.
• Nit 3, Activity log lost on close: not a behaviour change. showOrRaisePersistent() already set WA_DeleteOnClose (MainWindow.h:2358), so ✕ / Close destroyed the dialog and its log before this PR too. Esc / Ctrl+W hide it through reject() and keep it, as before.

Finding 1 (the pick and the Encoder reorder) is the maintainer's call. The reorder is one addLayout line and easy to drop if it isn't wanted.

Local: check_a11y 0 findings, check_test_registration, check_ci_test_gate and check_build_options OK, and the colour ratchet against base is down (unique −6, references −27, setStyleSheet −24).

[w5jwp]

Correction to my reply above, on nit 3. I wrote that Esc and Ctrl+W hide the window through reject() and keep the Activity log. That's wrong on Qt 6.12. With WA_DeleteOnClose, QDialog::done() closes the window, so Esc, QKeySequence::Close and the corner button each destroy it, and the log starts empty on the next open.

The conclusion stands: this isn't a behaviour change. showOrRaisePersistent() set the same attribute (MainWindow.h:2358), so the PersistentDialog was destroyed on every close too. e53c2f3b adds a check for each path to rc28_mapping_dialog_test (now 21 checks).

[ten9876]

Reviewed head e53c2f3b4a6e4e685f97974b4e08d4527ffdd7d3.

Issue fit

The PR does what #6744 asks. RC28MappingDialog moves from PersistentDialog to a CanonWindow dialog under canon/rc28Mapping; every color literal gives way to canon tokens in one sheet with canonIndicatorRules(); the status has three token states; the controls, saved fields and mappingFieldChanged wiring are unchanged; and Pulses per step moves below the two check boxes.

Maintainer ruling: the triage on #6744 called the CanonWindow move an unlisted #6239 pick. I pick it, the same call as AetherControl and the multiFLEX Dashboard, and the same answer applies to #6734. The Encoder reorder is accepted as part of it. So this review judges the code only.

Scope

File / group	What it changes	Claimed?	Verdict
src/gui/RC28MappingDialog.{h,cpp}	CanonWindow, canon sheet, rc28Role / rc28State, resizer, geometry key, a11y names, Encoder reorder, min width 460	Yes	In scope
src/gui/MainWindow_Menus.cpp	Canon create, raise and close in place of showOrRaisePersistent(); mappingFieldChanged connected once per dialog	Yes	In scope
tests/rc28_mapping_dialog_test.cpp, tests/tests.cmake	New socket-free test under if(HIDAPI_FOUND), in AETHER_SETTINGS_CONSUMERS	Yes	In scope; see blocker 1
docs/style/*, docs/user/docs/usb-control-surfaces.md	Canon lists, scope list, window behavior, Encoder order	Yes	In scope
docs/architecture/aetherd-touchpoints.md	core/ThemeManager.h includers 183 → 184	Yes	In scope (ui-support, no radio state)

Everything in the diff is explained by the issue. There's no new settings key (the geometry stays under RC28MappingDialogGeometry), protocol verb or capability, and no CHANGELOG.md entry.

Blockers

1. rc28_mapping_dialog_test fails on Linux offscreen, the configuration Full Suite runs it in

tests/rc28_mapping_dialog_test.cpp:51-62: "geometry saved by the old dialog restores under its key" fails every time on Linux with QT_QPA_PLATFORM=offscreen. Three runs out of three gave 20 passes and 1 failure. The sizes, printed at that check:

DBG chosen 520x898 restored 520x818 minHint 488x818 screen 800x800

The test asks for minimumSizeHint().height() + 80 = 898 px, but the offscreen screen is 800 px tall. QWidget::restoreGeometry() fits the window to the screen, and the minimum size then holds it at 818, so restored.size() == chosen can't hold. It passes on a Mac only because the display is taller.

full-suite.yml runs the unfiltered ctest with QT_QPA_PLATFORM: offscreen in ghcr.io/aethersdr/aethersdr-ci, whose .github/docker/Dockerfile:38 installs libhidapi-dev. So HIDAPI_FOUND holds there, the target builds, and this check would turn main's Full Suite red after the merge. PR CI doesn't run it, which is why the checks here are green.

A fix that keeps the restore meaningful on any screen (needs #include <QScreen>):

    RC28MappingDialog restored(nullptr);
    // restoreGeometry() fits the window to the screen, so ask for a height the
    // screen can hold; the offscreen screen is 800 px tall.
    const int minHeight = restored.minimumSizeHint().height();
    const int screenHeight = restored.screen()->availableGeometry().height();
    const QSize chosen(restored.minimumWidth() + 60,
                       qMax(minHeight, qMin(minHeight + 80, screenHeight)));

2. The status line's fixed accessible name hides the connection state from screen readers

At src/gui/RC28MappingDialog.cpp:262, m_statusLabel->setAccessibleName("RC-28 connection status") replaces the label's text as its accessible name. On the merge base the label's accessible name was its text, so a screen reader heard the state. Read back through QAccessible::queryAccessibleInterface() on each build:

BASE status a11y name 'Not connected'
HEAD status a11y name 'RC-28 connection status' description ''

The running app's dumpTree agrees: ('QLabel', 'RC-28 connection status'). "Not connected", the device name and the "Multiple … detected" warning are now unreachable for a VoiceOver, NVDA or JAWS user, which is a regression against the Perceivable goal in docs/a11y.md and #4896. docs/a11y.md § "Live value updates" also asks text updates to fire NameChanged; nothing does.

Either drop the fixed name (the base behavior), or put the state in it and announce changes (needs #include <QAccessible>):

void RC28MappingDialog::setStatusState(const QString& state)
{
    for (QLabel* l : {m_statusDot, m_statusLabel}) {
        l->setProperty("rc28State", state);
        l->style()->unpolish(l);
        l->style()->polish(l);
    }
    // The label's text is the status, so it belongs in the name.
    m_statusLabel->setAccessibleName(
        QStringLiteral("RC-28 connection status: %1").arg(m_statusLabel->text()));
    QAccessibleEvent ev(m_statusLabel, QAccessible::NameChanged);
    QAccessible::updateAccessibility(&ev);
}

Both callers set the text before calling setStatusState(). The constructor's initial name would then be "RC-28 connection status: Not connected".

3. The window's minimum height grew from 671 to 818 px and no longer fits a 768-px screen

I measured the merge-base dialog and the PR's dialog in the same offscreen harness with the same fonts:

BASE minHint 364x671
HEAD minHint 488x818

The running app opens it 782 px tall (dumpTree: 'h': 782, 'w': 460). A 1366×768 laptop, still common at the operating position, has roughly 730 px available. The old dialog fit there. This one can't be shrunk to fit, so Close and the Activity card fall off the bottom of the screen.

The growth comes from the new margins and paddings: QGroupBox { margin-top: 24px; } over five cards, the canon header, the larger root margins, and the default font size replacing the removed font-size: 11px rules. Please bring the minimum back under about 700 px, for example by:

• tightening the card title margin;
• putting the cards in a QScrollArea as AetherGPS does;
• letting the 100 px fixed-height log shrink.

Then state the new minimum in the body.

Merge conflict with main

The PR is CONFLICTING. #6768 (multiFLEX Dashboard, merged at 00:29 UTC) edited the same canon-window lists in docs/style/aethersdr-style-guide.md, docs/style/dialog-patterns.md and docs/style/theme-style-guide.md. Those three files are the only conflicts, and each is an entry in the same list: keep both "the multiFLEX Dashboard" / canon/multiFlex and "Icom RC-28 Button Mapping" / canon/rc28Mapping.

Nits (non-blocking)

1. The slider loses its tick marks. src/gui/RC28MappingDialog.cpp:370-371 still asks for TicksBelow every 1, but once the sheet styles QSlider::groove:horizontal (:174-188), the style-sheet path draws no ticks. Rendered side by side, the base slider shows the ten ticks and the head slider shows none. Either drop the two tick calls or keep the steps visible another way.
2. The activity log's scroll bar is unthemed. It appears after about seven events. The sheet's QWidget { background: transparent } reaches it, but there are no QScrollBar rules, so it draws Qt's default arrows and outlined handle on the canon panel. The other canon sheets (Network Diagnostics, Radio Setup, AetherControl, SpotHub) each carry canon QScrollBar rules; copying one fixes it.
3. One check's label doesn't match what it does. tests/rc28_mapping_dialog_test.cpp:155-160 labels path 2 "the corner close button destroys the window", but the test calls dialog->close(). It never clicks the corner button or the footer Close (rc28MappingClose). Click one of them, or rename the check.
4. Light-theme contrast of the warning status. color.accent.warning on canon raised is about 3.1:1 in Default Light (the triage on #6744 has the numbers), under docs/a11y.md's 4.5:1 for 11 px text. It's a shared token, so it's not this PR's to fix, but the body could say so.
5. No test covers the status states. Nothing checks that connected and warning actually restyle the dot and line. onMultipleDevicesDetected() can be called on a null-encoder dialog; one check of the label's color after it would pin the rc28State path that has never been seen on screen.
6. Duplicate sheet rules. In src/gui/RC28MappingDialog.cpp:76-78 and :168-173, QLabel { background: transparent } and the background / color lines in the QCheckBox, QRadioButton rule repeat the leading QWidget rule; only spacing: 8px adds anything.

Verified vs. read

• Built and ran (Linux, Qt 6.12, lean shared-core build at this head): AetherSDR, rc28_mapping_dialog_test and canon_window_test. canon_window_test passes; rc28_mapping_dialog_test fails as in blocker 1. The other 20 checks pass: the log menu is opaque, "10" fits, the combos read and write the saved fields and emit mappingFieldChanged, and Esc, Ctrl+W and close() each destroy the window.
• Measurements and renders: the base and head numbers in blockers 2 and 3, and the slider and scroll bar in nits 1 and 2, come from throwaway targets that compiled the merge-base and head dialogs in the same harness. Those targets were removed afterwards.
• App driven through the bridge, offscreen, with isolated AETHER_SETTINGS_DIR and AETHER_AUTOMATION_NO_TX=1, never connected to a radio:
  • triggered Icom RC-28 Remote Encoder... twice: one rc28MappingDialog, visible, 460×782;
  • dumpTree shows the four accessible names;
  • a grab shows the five canon cards, the indicators and the "Not connected" status, all correct in Dark.
  • Instance closed afterwards.
• Static gates on the merge base and on the head give the same output for check_engine_boundary --strict, gen_touchpoint_manifest --check, check_capability_records --strict and check_command_plane --strict. The manifest diff is the one ThemeManager.h includer. check_test_registration, check_theme_seed and check_a11y (0 findings) pass.
• CI: green, but the earliest run on this head was created at 21:17 UTC. main has since moved to 09dc6267b, which includes #6768, and the PR now conflicts. CI never runs the new test before merge either way.

What I tried to break that held

• Combo drop-down frames. The automated pass suggested the transparent rule would leave them see-through. Not so: the container (QComboBoxPrivateContainer) grabs fully opaque (0 non-opaque pixels of 156,300).
• Lifecycle. Two opens give one window; mappingFieldChanged is connected once per constructed dialog; Esc, Ctrl+W and close each destroy it under WA_DeleteOnClose, the same as the showOrRaisePersistent() path did.
• Return key. Both push buttons are setAutoDefault(false), so a Kind::Dialog has no default button for Return to click.
• Geometry carry-over. The base64 → x,y,w,h conversion works when the requested size fits the screen. Only the test's choice of size is wrong (blocker 1).

Recommendation

Request changes. The re-skin itself is solid and the pick is made. Three things need fixing: the new test's screen-dependent check, which would turn main's Full Suite red; the status line's accessible name, which hides the state from screen readers; and the minimum height, which no longer fits a 768-px screen. Then rebase onto main for the three docs conflicts. The nits can ride along.

[ten9876]

Approved at dcec01202. Every finding from my review is fixed:

1. Test failed on a Linux display: the carry-over check now asks for a height the 800 px offscreen screen can hold (efda151d4). rc28_mapping_dialog_test passes 29/29 offscreen on Linux.
2. Status hidden from screen readers: the status line's accessible name now carries its state (efda151d4). The running app reports "RC-28 connection status: Not connected" through dumpTree, and a new check follows the name through the multiple-devices warning.
3. Window too tall for a 768-px screen: minimum height 818 → 683 px (the old dialog was 671); the app opens it 644 px tall. A new check requires 700 px or less.
4. Conflicts with main: resolved by merging main twice (c657c84e5, dcec01202), keeping every list entry.

Nits: the slider tick calls are gone; the activity log has canon scroll bars; the test clicks the corner and footer Close buttons; the warning state has a test; duplicate sheet rules are gone; the contrast note is in the body.

Maintainer decisions: the CanonWindow move and the Encoder reorder; tightening the cards to fix the height; an accessible name that carries the state; dropping the tick marks; merging main in rather than rebasing; and sharing one canonScrollBarRules() across the seven canon windows that carried identical copies. All seven render pixel-identical before and after in both themes.

Verified:

• Built the app and the 16 tests that compile the touched dialogs; 15 pass. radio_setup_pair_grid_test fails identically on this machine at main (a 14 px font overflow) and passes in CI's Full Suite, so it is unrelated.
• Each new guard was reverted on purpose and its check went red.
• Drove the app offscreen with isolated settings, never connected to a radio, in Dark and Light.
• Static gates match main, apart from one more scanned file (the new header).

Not verified: a real RC-28. The connected and warning colors have been seen only in the test harness.