Repository navigation
Conversation
|
After: 
|
![aethersdr-agent[bot]](https://avatars.githubusercontent.com/in/3286087?s=60&v=4){kind=small}
There was a problem hiding this comment.
Issue fit
Yes. #6758 lists nine asks and every one is in the diff: CanonWindow::Kind::Dialog with FramelessResizer (src/gui/ShortcutDialog.cpp:139-153), the canon/shortcuts scope, the ModemChrome canon sheet in place of the hex sheet, the map resolving tokens through its widget and sitting on a rounded card, all six QMessageBoxes gone (grep QMessageBox src/gui/ShortcutDialog.* is empty), Reset All opening on Cancel with a gold accept, CanonMessageBox::setDetailedText(), the map sized to its keys with a cap, and the Return-in-Filter data-loss fix. The platform file pickers are untouched, and the declared non-goals (registry, workflow, category colours) are respected.
Two claims I could check and which hold exactly:
- The key extent is not an estimate.
kKeysW = 22·1.08 + 1 = 24.76andkKeysH = 5.25·1.08 + 1 = 6.67(src/gui/KeyboardMapWidget.cpp:43-45) are the true maxima ofbuildLayout(): the numpad-/+/ENTcolumn atx = 22, w = 1is the rightmost edge, and bothy5row keys (h = 1) andENT(y = 4.25, h = 2) bottom out at 6.67. No key clips, and the old 23.5 × 6.5 × 1.08 divisor was over-reserving, which is the empty band the issue describes. - The screenshot was really retaken at the old density.
keyboard-shortcuts.pnggoes 1833×1333 → 1833×1400 — exactly1100×840 × 1.6667, against main's1100×800 × 1.6667. Same width, new height, 232 KB → 222 KB.
Coverage is at the right layer: shortcut_dialog_test and the canon_message_box_test addition are socket-free CTests driving widgets with injected events, which is where AGENTS.md's test-layer boundary puts GUI state. Both are registered in tests/tests.cmake:2368-2392 and shortcut_dialog_test joins AETHER_SETTINGS_CONSUMERS as the CMake contract requires (it compiles AppSettings through $<TARGET_OBJECTS:aether_test_settings> and isolates with TestSettingsProfile.h). The PR is straight about CI: neither test is in ci.yml's frozen per-PR ctest -R gate, so the green checks on 3ff4b31f (build, check-macos, check-windows, Static checks, Build docs site — all success) prove compilation, not the new assertions.
No socket tests. No QTcpServer/QUdpSocket/QLocalServer/bind/listen/connectToHost, no peer process, no Fake* device in either added or modified test; nothing registered gains network behaviour. Nothing to surface under #5254.
Scope
| File(s) | Change | Claimed? | Verdict |
|---|---|---|---|
src/gui/ShortcutDialog.{h,cpp} |
QDialog → CanonWindow, canon sheet, six boxes → CanonMessageBox, setReturnClicksDefault(false) |
Yes | In scope |
src/gui/CanonMessageBox.{h,cpp} |
setDetailedText() + kDetailsHeight, details panel and its scrollbar rules |
Yes (issue asks for it by name) | In scope — new shared-widget API, one caller; modest and inside the canon the maintainer already owns |
src/gui/KeyboardMapWidget.{h,cpp} |
heightForWidth + cap, widget-scoped tokens, rounded card, light-theme key labels, press-between-keys ignored |
Yes | In scope. The press-passthrough is the one item not in #6758's text — it comes from CanonWindow's documented contract ("the window moves by dragging any empty part of it", CanonWindow.h:16-17), and the PR body discloses it |
tests/shortcut_dialog_test.cpp, tests/canon_message_box_test.cpp, tests/tests.cmake |
New target + details-panel checks | Yes | In scope |
docs/style/{aethersdr-style-guide,dialog-patterns,theme-style-guide}.md |
Add the window and canon/shortcuts to the three lists |
Yes | In scope. Overlaps #6757/#6753/#6751/#6749/#6745, which all append to the same lines — whichever lands second rebases there |
docs/user/docs/keyboard-shortcuts.md, docs/user/screens.json, .../keyboard-shortcuts.png |
Button names, chrome, retaken shot, alt text | Yes | In scope |
Everything in the diff is explained by #6758 or by the canon contract. No unrelated file, no build/CI churn, no vendored tree, no CHANGELOG.md entry (correct — it must not have one). The - lines delete only the hand-rolled hex sheet, the six QMessageBox constructions and the per-widget applyStyleSheet calls; no guard, early return, confirmation or symptom-naming comment is removed. The one added confirmation gate (Reset All now opens on Cancel) is a safety improvement the issue asks for.
Preference check: every change to existing UI points at an authority — RFC #6226 / #6239 for the canon look, #6758 (labelled maintainer-review) for the map sizing and the Cancel default, and CanonWindow.h's own contract for the drag passthrough. Nothing here reads as smuggled taste. The two numbers that are the author's own pick rather than the issue's — kMaxSizingWidth = 1100 and the 1100×840 opening size — are both disclosed in the body as visible changes, so they are a maintainer visual call, not a violation.
Security audit (CodeGuard, required): ran, no findings. Every flagged class is a false positive and nine of the eleven listed files are not in this diff at all (the known "file list can be main's tip" case): CS-VLN-CMDI-AMP fires on C++ reference syntax (const QString&, &ShortcutDialog::importShortcuts); CS-VLN-CMDI-PIPE only on PanadapterStream.cpp:907/1574, which this PR does not touch, where | is a Qt flag OR and not a shell (the same idiom this PR adds at KeyboardMapWidget.cpp:318, Qt::AlignLeft | Qt::AlignTop); CS-VLN-CMDI-BACKTICK on backticks inside comments; CS-VLN-SSRF-HEX on colour literals (QColor(0xc0, 0xd0, 0xe0)); CS-VLN-SSRF-LOCALHOST/CS-PII-IP/CS-PII-EMAIL on RFC 5737 example addresses and you@example.com in screens.json alt text; CG-NET-001 on the pre-existing Flex VITA ephemeral-port binds from merged 2a6ee37ec. The one path worth real scrutiny — untrusted CSV error text reaching the new details panel — is clean: QPlainTextEdit::setPlainText has no rich-text mode, the siblings are forced Qt::PlainText including the header title, and ShortcutManager caps the input at 1 MiB / 4096 rows / 512-byte fields before any of it is joined.
Blockers
None.
Nits
All non-blocking.
src/gui/ShortcutDialog.cpp:484-489— on Cancel the Action combo keeps showing the action you just rejected. The early return skips theupdateSelectedKeyInfo()two lines below, so the binding is untouched (correct) but the combo still reads the new action, which is the opposite of what happened. It is pre-existing (main'sif (r != QMessageBox::Yes) return;behaves identically) and #6758 puts the workflow out of scope, so this is only worth taking because this is the PR that gives Cancel its name and adds the test that asserts "Cancel keeps it where it was" — and the test checks the manager, not the combo. One line:QStringLiteral("Reassign"), CanonMessageBox::Role::Accept)) { updateSelectedKeyInfo(); // the combo still reads the rejected action return; }
tests/shortcut_dialog_test.cpp:239-241— the minimum-size check covers height but not width.dlg.bodyWidget()->minimumSizeHint().height() <= dlg.minimumHeight()is there; the width twin is not, and width is the dimension the sibling canon migrations tripped on. #6757 had to raise its minimum from 780 to the layout's own because "an explicit minimum overrides the layout's own … so Track and Band would otherwise clip", and #6753 grew 420×260 → 500×320 for the same reason. Here the minimum staysmain's 950 while the canon sheet grows the type and the button padding. I make the slack about 20 px (950 − 2·kInset− 28 px margins = 920 body px against the map's 900 pxminimumSizeHint), so it very likely does fit — but that is my arithmetic plus your macOS grab at 950×740, and the clipping symptom is silent and font-dependent. One more line in the same block pins it:EXPECT_TRUE(dlg.bodyWidget()->minimumSizeHint().width() <= dlg.minimumWidth());src/gui/CanonMessageBox.cpp:100-105—setDetailedText()doesn't feed the accessible description, thoughsetInformativeText()three lines above does. The panel carriessetAccessibleName("Details"), so it is reachable, but a screen reader hearing the box open gets text + informative text and has to navigate to find the skipped rows. Deliberate is defensible (hundreds of lines is not something to read aloud on open) — worth a one-line comment saying so, since the asymmetry with its neighbour reads as an oversight.src/gui/KeyboardMapWidget.cpp:42vs:223— the key gap is now two constants.kKeyGap = 0.08f(line 42) driveskKeysW/kKeysH, whilekeyRect()keeps its own localconstexpr float gap = 0.08f. They must agree or keys clip against the extent the newheightForWidthreserves.keyRect()can usekKeyGap.tests/shortcut_dialog_test.cpp:266-337— the two pixel probes are font- and DPR-dependent. The disabled-Undocheck takes the row pixel furthest from the panel colour and allowsdistance <= 12, which is more robust than an exact match but still assumes the glyph has a near-opaque core; a platform with lighter hinting can blend past 12. Andlist.pixelColor(QPoint(list.width() - 4, int(rowH * 1.5 * dpr)))scales y by the device pixel ratio but not x — harmless at the offscreen DPR of 1, wrong on a HiDPI runner. Since these land infull-suite.ymlrather than the per-PR gate, a flake there costs more than it does here.src/gui/ShortcutDialog.cpp:547-554— thekeyPressEventoverride is now an emptyifand a comment. The PR touches it only to swapQDialog::forCanonWindow::; since the base call is all that is left, the override and itsif (ev->key() == Qt::Key_Escape && …) { /* Could use this … in the future */ }can both go.docs/user/screens.json:1958-1959— the manifest now mixes spellings. Moving this entry to "colored" to match the guide page is the minimal way to satisfyembed_screenshots.py --check, and the body discloses it, but 101 of the user-doc pages use "colour" to 17 that don't, andscreens.jsonis left with one "coloured" (line 3444) against two "colored". Fixingkeyboard-shortcuts.mdto British instead would have satisfied the same check and kept both files consistent. Purely a maintainer-taste call.
What I tried to break
Findings below are reasoned from the head checkout at /tmp/aetherclaude/pr-6759; I have no build, no test run and no way to drive the GUI, so nothing here was reproduced at runtime.
- "The map card drags the window" vs. the drag dying after the first pixel.
mousePressEventnow ignores a press off a key so it propagates (QApplication::notify's parent walk) up toCanonWindow::mousePressEvent— butmouseMoveEventstill callsev->accept()unconditionally with mouse tracking on, which looked like it would swallow every subsequent move and freeze the drag. It doesn't:FramelessMoveHelper::start()either hands off tostartSystemMove()(compositor-driven, no further Qt events needed) or callshandle->grabMouse()on theCanonWindowitself (FramelessMoveHelper.h:42), which re-routes the moves away from the map. The test'sQCoreApplication::sendEvent(map, &miss)does exercise the real propagation path, not a shortcut. canon/shortcutssilently falling back to the root palette.ThemeManager::lookupRawdoes not walk to the nearest existing ancestor — an unknown path drops straight tom_rootScope(ThemeManager.cpp:293-295), which would have made the whole re-skin a no-op. It is safe becausetheme::setContainer→registerDeclaredContainer→scopeOrCreatematerialisescanon/shortcutsas a child ofcanon(ThemeManager.cpp:252-278, 1905-1913), andm_declaredContainersis re-seeded on every theme load (:519), so the chain survives a theme switch. The theme JSON needs no new entry;canonalready carries the base-token overrides and the empty child inherits them.- Return no longer clicking anything.
CanonWindow::keyPressEventguards on the same "plain Enter" testQDialoguses andignore()s before the default-button dispatch (CanonWindow.cpp:108-123), andQLineEditleaves Return unaccepted, so the filter's Return does reach it.Clearreally is the firstautoDefaultbutton (clearBtnandresetBtnare still the only bottom-area buttons withoutsetAutoDefault(false)), which is why themainbug cleared the binding — so the author's mutation result is consistent. Numpad Enter is covered by the same clause. - The width cap leaving a new empty band, or clipping keys. Past 1100 px
heightForWidthpins the height,paintEventtakesmin(unitFromW, unitFromH)so the unit stops growing, andm_originXcentres — butm_originYstays atkMapPadding, so any over-allocated height would pool at the bottom. It can't: the map's vertical policy isPreferredat stretch 0 while the table is stretch 1, so the layout gives the map exactly itsheightForWidthand the surplus to the table. Under-allocation degrades gracefully (unit shrinks, keys centre). - Teardown and reentrancy through the six boxes. Every box is a
ScopedChildWidget, whose destructor deletes through aQPointerso a parent that dies insideexec()isn't double-freed, andconfirm()short-circuits onoperator boolbefore touchingclickedButton(), so the danglinggois never dereferenced.m_clickedis reset inshowEvent, so a reused box can't return a stale answer, and closing by ✕/Esc/⌘W leaves it null →confirm()false. What is not guarded is theresetAllBtnlambda continuing to usethisafterconfirm()returns, but that is the pre-existing modal-exec()pattern used across the codebase and only reachable at app teardown. - Scrollbar completeness.
ModemChromeships onlyQScrollBar:vertical(ModemChrome.cpp:338-351); both new sheets add the horizontal twin, so theNoWrapdetails panel and the action table are styled in both axes rather than falling back to the OS bar in one. - Untrusted text into the new panel — covered by the security audit above;
QPlainTextEdit+ the threeShortcutManagercaps close it. - Couldn't check: the author's own mutation runs, the local test passes, the light-theme and 950/1100/2000-px grabs, the
npm run buildandaudit_coloursnumbers, and whetherkMaxSizingWidth = 1100and the canon type sizes look right. Those are visual or build-time claims; the three greencheck-*jobs plus Static checks cover compilation and the static gates, and nits 2 and 5 are where I think the remaining risk sits.
Recommendation
Approve with nits — with the caveat that this is a canon-look change, so the maintainer still owns the visual calls the body flags (the 1100 px sizing cap, the larger canon type in the action table, and Reset All opening on Cancel).
The migration is complete and honest: every issue requirement is in the diff, the two measurable claims I checked (the key-extent constants and the screenshot density) are exact rather than approximate, the Return data-loss fix is real and the mutation story for it is self-consistent, and the test additions fail for the right reasons rather than asserting the implementation back to itself. No blockers, and the nits are all one-liners. Next step: take nit 1 and nit 2 (both single lines, and nit 2 closes the gap that bit #6757 and #6753 in this same series), then it is the maintainer's visual sign-off. Thanks for the unusually legible body — the "visible changes for the maintainer" list and the explicit "CI does not run these checks" note made this much faster to review.
🤖 aethersdr-agent · cost: $19.1571 · model: claude-opus-5
|
Cisco CodeGuard — static analysis of this PR (137 finding(s))
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: $20.3569 · model: claude-opus-5 |
3ff4b31 to
6c637a7
Compare
|
Thanks for the review. I took nits 1–6 in The branch is rebased onto 1. Cancel on Key Conflict leaves the combo on the refused action. Fixed. The Cancel path now calls 2. The minimum-size check didn't cover width. Added the width check. Both checks now also add the window's hairline
So the width headroom is 4 px offscreen, not the ~20 px you estimated. It fits, but only just, and it depends on the font. Mutation: a 945 px minimum fails the width check ( 3. 4. Two key-gap constants. 5. The pixel probes depend on font and DPR.
6. The empty 7. "colored" vs "colour" in Runs on Still for @ten9876, as the body lists: the 1100 px sizing cap, the canon type in the action table, Reset All opening on Cancel, and now whether the 950 px minimum should keep only 4 px of width headroom. |
…ons are CanonMessageBoxes. Principle VIII. Configure Shortcuts moves from a hand-styled QDialog to a CanonWindow dialog, following the AetherRX / AetherTX Settings dialogs: canon header and ground, its own theme scope (canon/shortcuts), the canon ModemChrome dialog sheet in place of the hard-coded hex sheet, and FramelessResizer so it still resizes from every edge. It stays modal over the main window, and the menu still rebuilds the shortcuts when it closes. Return in the filter field no longer presses Clear or Reset to Default (setReturnClicksDefault(false)). The keyboard map resolves its tokens through its own widget, so it takes the canon scope's backgrounds and accent, and sits on a rounded card. Category colours are unchanged. All six QMessageBoxes become CanonMessageBoxes: - Reset All asks with Cancel and a gold Reset All; Cancel is the default, so Return no longer resets every binding. - Key Conflict asks with Cancel and Reassign (the default, as Yes was). - Import's failure and result notices and Export's notices have one OK button. CanonMessageBox gains setDetailedText(): a read-only list, at most 180 px tall, to scroll and copy below the texts, in place of QMessageBox's detailed text. Import uses it for the parse errors, skipped actions and displaced bindings. The file pickers stay the platform's own. New shortcut_dialog_test covers the window's shape and scope, and that Reset All and Key Conflict change nothing unless their accepting button is clicked. canon_message_box_test gains the details panel. The style docs and the user guide list the window; the guide's screenshot is now stale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… and the controls below it get room to breathe. Principle VIII. The map took four sevenths of the spare height against the table, while its key size was set by the width, so an empty band sat below the keys. It now reports heightForWidth from the keys' exact extent (24.76 x 6.67 key units with gaps, not the old 23.5 x 6.5 estimate) and is laid out without stretch; the keys fill its card and centre if the height ever limits them. The rows below it are spaced a little more (10 px, 6 px more after the legend, and between each row's label pairs), and the table takes the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…8 px screen, and draws its combo arrows. Principle VIII. Review follow-ups on this branch: - The keyboard map's height grew with the window's width with no cap, so a wide or maximised editor squeezed the action table to two rows. Past 1100 px the keys stop growing and centre; the table keeps its height. - The minimum height returns to 700 px. The canon layout needs less than that, and 740 did not fit a 768 px screen's usable height. - A press between or around the keys is ignored, so it reaches the window and drags it, as the user guide says. - The Action and Category combos draw the canon down-arrow (comboArrowRule()), as the other canon windows' combos do. - CanonMessageBox's details panel styles its horizontal scrollbar. - An import with skipped or displaced actions says "with warnings" in its text, since a canon box has no warning icon. Tests: shortcut_dialog_test pins Return in the Filter field leaving the selected key's binding alone (on main it pressed Clear), the map's height, the table keeping its height past the cap, the 700 px minimum, and a press between keys reaching the window; answerNextBox closes a modal that is not a CanonMessageBox so the test fails instead of hanging. The details check in canon_message_box_test now asserts the 200 lines scroll. Each new check fails when its fix is reverted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the user guide shows the canon editor. Principle VIII. Unbound keys drew their labels in a fixed white over the theme's background fill, so under Default Light ESC, the F keys and every unbound letter were nearly invisible. They now use color.text.primary through the map's scope; bound keys keep white on their dark category fills. The user guide's keyboard-shortcuts screenshot is retaken from the demo simulator (DEMO-0001, isolated settings, offscreen, TX off) at the old image's density (QT_SCALE_FACTOR 1.6667, 1833 px wide), as capture_screenshots.py would; OCR finds nothing to redact. Its alt text says the map fills the upper part of the window rather than the top half, and the manifest entry takes the guide's US spelling, so embed_screenshots.py --check passes for the page. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…he canon menu rules the other canon windows carry. Principle VIII. The sheet gave QMenu its canon panel and a cyan selected item, but no rule for a disabled item, so an unavailable Undo, Cut or Copy drew in canon.inkSoft, the same ink as an available one, and read as clickable. The menu now carries the full canon block the other canon windows use: item padding, color.control.unavailable for disabled items, and a canon.line separator. shortcut_dialog_test opens the Filter field's standard menu on an untouched field and checks the panel is opaque and that the Undo row's glyph core is color.control.unavailable. Without the disabled rule it is #c4d4e8 (inkSoft) and the check fails. It also checks the two qWaitForWindowExposed results it was ignoring. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… the other canon windows' lists do. Principle VIII. Six of the seven canon windows on main draw a combo's drop-down list on canon.nested (SpotHub, Network Diagnostics, Firmware Install, AetherControl, AX.25 HF Packet Decode, Radio Setup) and their right-click menus on canon.raised. This window had its lists on raised; they now follow the convention, leaving its menu on raised. shortcut_dialog_test opens the category filter's list and checks an empty part of an unselected row is color.canon.nested; with raised it reads #0a121e and the check fails. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… own action, and the minimum width is pinned against the layout. Principle VIII. Cancelling Key Conflict returned before updateSelectedKeyInfo(), so the binding was kept but the Action combo still showed the action just refused. It now goes back to the key's own action. shortcut_dialog_test picks the conflicting action in the combo before it is refused and checks the combo reads PTT again; without the call it reads Mute and the check fails. The minimum-size check covers width as well as height, and both now add the window's hairline inset to the body's minimum: 946 x 595 against 950 x 700 offscreen, 930 wide on cocoa. A 945 px minimum fails the width check. The Undo-row probe checks that the glyph core lies on the line from the panel to color.control.unavailable, at least half way along, rather than within 12 of the ink, so lighter hinting cannot blend it out of range. Without the QMenu::item:disabled rule the core is #c4d4e8, 3.53 along and 19.6 off the line, and both checks fail. keyRect() uses kKeyGap, the gap the map's extent is computed from, instead of its own copy. The empty keyPressEvent override goes; CanonWindow's is the one that runs. setDetailedText() says why the details stay out of the accessible description. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
6c637a7 to
96adcb1
Compare
|
Rebased again onto
On the new base, |
Resolves the conflicts in the three docs/style/* canon-window lists, where main added FFTW Wisdom / canon/nr2Wisdom. Every list keeps main's entries and this branch's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Closes #6758. Part of #6239 and approved RFC #6226.
The shortcut editor (
ShortcutDialog, Settings → Configure Shortcuts...) moves from a hand-styledQDialogto aCanonWindowdialog, following the AetherRX / AetherTX Settings dialogs. Its sixQMessageBoxes becomeCanonMessageBoxes.It also fixes a data-loss bug on
main: with a bound key selected, Return in the Filter field silently cleared that key's binding. QDialog turned Return into a click on the first auto-default button, which is Clear.CanonWindow::Kind::Dialog, still modal over the main window: canon header and ground, corner close button, Esc / ⌘W close, and edge resizing throughFramelessResizer. The menu still opens it withexec()and rebuilds the shortcuts when it closes (MainWindow_Menus.cppis unchanged).canon/shortcuts(wasdialog/shortcut, which no theme file uses).ModemChromedialog sheet replaces the hard-coded hex sheet. On top of it:canon.nestedand the menu oncanon.raised, the pairing six of the seven canon windows onmainuse (3415c22c). The menu carries the same canon block as TX Band Settings and the other canon windows: item padding,color.control.unavailablefor disabled items, and acanon.lineseparator (583cda8c). Without the disabled rule, an unavailable Undo / Cut / Copy drew incanon.inkSoft, like an available item;comboArrowRule()), as the other canon windows' combos do (the old sheet hid it);setReturnClicksDefault(false): Return in a field presses no button, which is the fix above. A focused button still takes Return.KeyboardMapWidget):ThemeManager::color(this, …)), so the map takes the canon scope's backgrounds and accent. It is drawn on a rounded card; category colours are unchanged.heightForWidth, from the keys' exact extent: 24.76 × 6.67 key units with gaps, where the old estimate was 23.5 × 6.5). Onmainit took four sevenths of the spare height while its key size came from the width, leaving an empty band below the keys.color.text.primaryinstead of a fixed white, so they read under Default Light; bound keys keep white on their dark category fills.Role::Destructive). Cancel is the default, so Return no longer resets every binding; before, Yes was the default.mainit kept showing the refused one).ScopedChildWidgets, as in the CTR2 window, so the editor closing mid-question tears them down.CanonMessageBox::setDetailedText(): import reported its parse errors and its skipped and displaced actions inQMessageBox's detailed text, which can run to hundreds of lines. The canon box gains a read-onlyQPlainTextEdit(canonMessageDetails) on a nested card, at most 180 px tall, with canon scrollbars, hidden when empty. The text can be scrolled and copied, is plain text, and doesn't take the focus from the default button.docs/style/aethersdr-style-guide.md,dialog-patterns.mdandtheme-style-guide.mdlist the window and its scope.dialog-patterns.mddescribessetDetailedText().docs/user/docs/keyboard-shortcuts.mdnames the Reassign / Reset All buttons, says Return and Esc cancel Reset All, and says how the editor moves, resizes and closes.keyboard-shortcutsscreenshot is retaken from the demo simulator at the old image's density (1833 px wide). Its alt text says the map fills the upper part of the window. The manifest entry takes the guide's US spelling, soembed_screenshots.py --checkpasses for the page (14 other pages are out of step onmainand are left alone).Intent
After this merges, the shortcut editor and its questions look and behave like the other canon windows and follow the theme. Every control does what it did before, except Return in a field, which no longer clears a binding. Out of scope: the editor's controls and workflow, the shortcut registry, and the category colours.
Visible changes for the maintainer:
mainit cleared the selected key's binding).Constitution principle honored
Principle VIII — Evidence Over Assertion: each behaviour this PR claims has a check that fails when its change is reverted (see the test plan).
Test plan
cmake --build build)AetherSDR,shortcut_dialog_testandcanon_message_box_teston macOS arm64.DEMO-0001, isolated settings, offscreen, TX off): opened the editor, grabbed it at 950×740, 1100×840 and 2000×840. Nothing clips at the minimum; at 2000 wide the keys centre and the table keeps its rows.main) and are fixed here.shortcut_dialog_test20/20 offscreen and 5/5 on the native cocoa platform (Retina display).shortcut_dialog_test:CanonWindowin thecanon/shortcutsscope, with the canon header and the keyboard map;color.control.unavailable, at least half way along, so lighter hinting can't blend it out of range;color.canon.nested;CanonMessageBoxis closed, so a regression fails instead of hanging.canon_message_box_testgains the details panel: hidden when empty; shown and read-only; 200 lines scroll inside a panel no taller than 180 px; markup kept as text; the default button keeps the focus; hidden again when cleared.setReturnClicksDefault(true): the Return-in-Filter check (PTT's binding cleared);QMenu::item:disabledrule: the Undo glyph core reads#c4d4e8(canon.inkSoft), 3.53 along the line and 19.6 off it, instead of#3a4a5a;updateSelectedKeyInfo()from the Key Conflict Cancel path: the combo still reads Mute;canon.raised: its ground reads#0a121einstead of#0e1a2a.ci.yml's per-PRctest -Rsteps (the per-PR gate is frozen,docs/agents/tests-ci.md); they run post-merge infull-suite.yml. The local runs above are the evidence.Static checkssteps run locally pass: engine boundary, generated user-docs reference, test registration, frozen per-PR test gate, QSettings ratchet, theme seed.tools/audit_colours.py --compare-src <origin/main> --strict): unique colours 470 (+0), colour references 1786 (−25),setStyleSheetcalls 564 (−1), on the branch rebased ontobe216ed3.ShortcutDialog.cpphas no hex colour literals left.tools/check_a11y.pyonShortcutDialog.cpp,CanonMessageBox.cppandKeyboardMapWidget.cpp: no findings.main's Return handling (setReturnClicksDefault(true), which is plainQDialog::keyPressEvent) on this branch, not on amainbuild.Also checked:
cd docs/user && npm ci && npm run buildpasses locally (links and anchors).capture_screenshots.pywas not used for the retake. It runs this untagged shot only against a real radio. The grab reproduced its settings by hand instead: demo instance, isolated settings, offscreen, TX off,QT_SCALE_FACTOR1.6667,oxipng, and an OCR check for addresses and serials.Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls — use nested-JSON-under-one-key (Principle V)MeterSmoother(AGENTS.md convention) — n/a, no metersdocs/style/anddocs/user/docs/keyboard-shortcuts.md. NoCHANGELOG.mdentry.🤖 Generated with Claude Code