aethersdr / aethersdr/AetherSDR

[RFC] Name a button's keyboard shortcut in its tooltip, qualified rather than hidden when shortcuts are off

Open
#5,484 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C++
Stars
221
Forks
117
Avg merge
2d 7h
Merged PRs (30d)
299

Description

Preflight

  • I have read GOVERNANCE.md and confirmed this change requires an RFC — it is
    a UX behaviour change to what hovering a button shows.
  • I have searched existing issues and this RFC has not been proposed before.
    All 455 open issues fetched paginated to exhaustion and searched on
    shortcuts, tooltip, tooltips, discoverability, annotate,
    spacebar, PTT hold, KeyboardShortcutsEnabled, shortcutGuard. The
    near neighbours are #1539 (asks for new bindings and fixes to two specific
    ones), #2013 (arrow keys scoped to VFO focus), #4871 (a new binding),
    #4849 (the bridge's hover verb, not the UI), and #5351 (a field report
    from an operator whose spacebar PTT worked — the control case for this).
    None proposes annotating controls with their bindings.
  • I have not opened a PR for this change yet, and will not until this is
    approved.

Problem

A button never says which key works it. That is the whole of the problem this
RFC addresses, and it holds whether or not shortcuts are switched on.

MainWindow::registerShortcutActions carries 70 registerAction call sites
across the fourteen categories ShortcutManager::categories returns, nine of
them in TX. There is remapping UI — ShortcutDialog, with KeyboardMapWidget
drawing an actual keyboard — plus persistence, CSV export/import and conflict
checking. Space is already PTT (Hold) and T is already MOX Toggle.

Nothing in the running application says so. Hovering MOX showed no tooltip at
all — the button had none. Hovering TUNE showed "Start or stop tune carrier",
which describes what the button does and says nothing about a key. The only
surface naming any binding is a dialog behind View → Configure Shortcuts…,
which is where you look once you have already concluded that shortcuts exist.

What this RFC is NOT the fix for, and the correction that matters

An earlier draft of this document argued that surfacing was the cause of an
operator's report. That was wrong, and the correction is worth stating plainly
because it changes what this proposal is allowed to claim.

The operator asked:

"I would appreciate some keyboard shortcuts (for instance button space bar for
mox) — is any such thing foreseen?"

They already had them, and named as their example the exact key already bound to
the thing they wanted. But the reason pressing it did nothing is not that no
tooltip pointed at it. It is that KeyboardShortcutsEnabled defaults to
"False"
, shortcutGuard therefore refuses every handler, and
MainWindow::handlePttHoldShortcut gates Space the same way while still
returning true and consuming the keypress. Filed separately as #5483.

So the cause of that report is #5483, not this. This RFC is not the fix for
it, and turning the master switch on is deliberately not proposed here
— Space
would key the transmitter on every existing install the first time someone tapped
it, which is Principle VI's territory and the maintainers' call.

What survives, and what this RFC actually rests on, is narrower and independent:
even with shortcuts switched fully on, hovering MOX tells you nothing about
T.
The annotation is worth having on its own account, for an operator who has
already found the View menu and still cannot see which of seventy actions reaches
the button under their cursor. That claim does not depend on #5483 at all, and it
does not go away if #5483 is fixed.

The population that most wants a PTT key is also the least likely to go hunting a
View menu for it: an operator with a microphone in one hand, running SSB, who
wants the other hand off the mouse. A discovery mechanism that requires you to
already believe in the thing you are discovering is not one.

The operator's own proposal, verbatim:

"Would it not be an idea to add tooltip on hover over for buttons that have a
keyboard shortcut?"

Proposal

Annotate a widget's tooltip with the key currently bound to the action that
widget performs, so hovering MOX reads:

Toggle manual transmit on or off (T)

A widget opts in by carrying a dynamic property naming its action:

m_moxBtn->setProperty(ShortcutManager::kActionProperty, "mox_toggle");

ShortcutManager::applyShortcutTooltips(QWidget* root, bool shortcutsEnabled)
walks the tree from root, finds every widget carrying that property, and
appends the bound key. MainWindow calls it after rebuildShortcuts, on
bindingsChanged, from the Configure Shortcuts… handler, and from the master
toggle's own handler — so a remap or a toggle updates the buttons without a
restart.

The master toggle qualifies the key. It never hides it.

This is the substantive design decision in this RFC, and it reverses what was
built first.

The first implementation stripped the annotation whenever View → Keyboard
Shortcuts was down, on the reasoning that a tooltip promising a key that does
nothing is a lie. It is a tidy argument and it produces an absurd result:
KeyboardShortcutsEnabled defaults to "False", so on a stock install — which
is every install until someone finds the switch — the mechanism built to make
shortcuts discoverable is itself invisible.
It only ever teaches the operators
who already found the switch, who are exactly the ones who did not need teaching.

#5262's binding doctrine already settles this, and it is not our invention.
Quoted there as a maintainer decision binding on every milestone:

Individual controls are never shown/hidden per radio. Every control renders:
dimmed/unavailablegreyed/inactivecolored/active.

A stripped annotation is the hidden state. A qualified one is unavailable
with a reason, which is the state the doctrine actually calls for:

master toggle MOX tooltip
on Toggle manual transmit on or off (T)
off Toggle manual transmit on or off (T — shortcuts disabled)

That single line tells the operator three things they cannot otherwise learn:
the key exists, it is not working, and the name of the thing that is
off
— which is the exact wording of the View menu item that turns it on. The
View menu becomes findable from the button, rather than the button being
findable only after the View menu.

It is worth being explicit that this makes the annotation useful precisely in the
default state, which is the reverse of the earlier design.

The rest of the rules, each of which exists because the naive version gets it wrong
  1. An action bound to no key is left completely alone, in either toggle
    state.
    Seven of the nine TX actions ship unbound — atu_start,
    tune_toggle, two_tone_tune, vox_toggle, speech_proc_toggle,
    dax_toggle, tx_monitor_toggle. There is no key to name and nothing to
    explain; (— shortcuts disabled) on a button that never had a binding would
    invent a shortcut. This is the common case, not a corner.
  2. Re-applying replaces the suffix; it never stacks. The un-annotated text is
    stashed in kBaseTooltipProperty on first use. Re-reading the live tooltip as
    the base — the obvious implementation — grows MOX (T) (T) (T) over three
    rebinds, and it also breaks the qualified↔bare round trip.
  3. A widget that rewrites its own tooltip keeps the annotation. TxApplet
    rewrites the TUNE tooltip in its tuneAvailabilityChanged handler and the ATU
    tooltip in updateAtuAvailability, on every availability change. A one-shot
    walk would lose the annotation from TUNE the first time the radio connects —
    precisely when the operator is looking at it. ShortcutManager installs
    itself as an event filter on each opted-in widget and re-annotates on
    QEvent::ToolTipChange. Call sites that set tooltips need to know nothing
    about shortcuts.
  4. A widget with no tooltip of its own gets the key alone. Better than
    nothing, but thin — so five TX buttons that had no tooltip were given one (see
    Implementation scope).
  5. A property naming an unknown action leaves the widget untouched. A typo,
    or a widget surviving the removal of its action, must not silently blank a
    tooltip.

Why a per-widget property and not a table in MainWindow. The declaration
lives with the widget, the idiom markTxKeying already establishes here, and
ShortcutManager.h's own comment on Action::keysTx records what happens
otherwise: "not a hand-maintained id list that drifts (#4057 review:
atu_start was missed)"
. A central table of button pointers is that list again,
with the same failure mode.

Cross-platform impact

The mechanism is QWidget, QVariant, QKeySequence and QEvent only. No
platform #ifdef is added, none removed, and nothing in it is conditioned on a
platform.

The rendered key text differs by platform by design, and that is the point of
using QKeySequence::NativeText.

  • macOSCtrl+M renders as ⌘M, and the other modifiers as ⌥ ⇧ ⌃,
    matching what is printed on the operator's keyboard. Verified: the test asserts
    the annotation is Base (⌘M) on this machine, and a deliberate regression to
    PortableText fails it with Base (Ctrl+M).
  • Linux and WindowsNativeText and PortableText coincide for these
    sequences, so both show Ctrl+M. The test detects that the two forms are equal
    and reports [SKIP] rather than pretending to have proved something.

Using PortableText would put the string Ctrl+M on a Mac tooltip beside a
keyboard with no key marked Ctrl in that position — the failure this proposal
exists to avoid, in miniature.

Honest statement of what was and was not tested. Everything here was built
and run on macOS only — Darwin 25.6, Qt 6.8.3, Ninja, RelWithDebInfo.
Linux and Windows were not built and not run. The parity claim is a claim
about the code — no platform-specific constructs, no platform-specific Qt calls —
and not a claim about observed behaviour on those platforms. A maintainer
with a Linux or Windows box should treat it as unverified. Nothing was found that
is macOS-only; if something is, it was not found rather than shown to be absent.

Two smaller notes, stated as uncertainty rather than as findings:

  • Tooltip length. The qualified form is longer than the earlier design's, by
    the width of — shortcuts disabled, and it is the form a stock install shows.
    Tooltip wrapping and maximum width are platform style decisions, so a tooltip
    that fits on one platform may wrap differently on another. The longest string
    this can produce is "Tune carrier is unavailable in this mode through the
    current radio backend (F2 — shortcuts disabled)"
    , and only once tune_toggle
    has been bound by the operator. Not measured on any platform, and it is the
    one place where the strip-vs-qualify decision has a cost.
  • Screen readers. Only toolTip is touched. accessibleName and
    accessibleDescription are left exactly as they were on every widget, so
    nothing changes for assistive technology on any platform. If a maintainer wants
    the key in accessibleDescription too, that is a separate and defensible
    decision; it is not attempted here.

Alternatives considered

Do nothing — the shortcuts already exist. A serious option. The argument for
the change is that a button never names its key even for an operator who has
switched shortcuts on and read the dialog once. The argument against is that one
report is one report; the maintainers know the shape of their inbox and this
reporter does not. This RFC does not claim the report generalises.

Strip the annotation when the master toggle is down. This was built and is
rejected above, on #5262's doctrine. Recorded as an alternative rather than
silently dropped, because it is the intuitive design and a reviewer will think of
it: the tooltip does become momentarily "wrong" in the sense that the key does
not work — but the qualifier is what makes it right, and hiding it is what makes
the feature useless in the state every install starts in.

Make ShortcutDialog easier to find instead. Quite possibly the better fix,
and not exclusive with this one. The dialog is the complete answer — all seventy
actions, remappable, with a drawn keyboard — and it sits behind a menu you open
only if you already suspect the feature exists. A first-run hint, a title-bar
affordance or a Help entry would reach an operator who never hovers a button.

The comparison cuts both ways: a tooltip reaches the operator at the moment the
question occurs to them
, which a menu item cannot do; but a tooltip only ever
names a binding that exists
, and seven of the nine TX actions ship unbound, so
their buttons stay silent. A tooltip alone does not discharge the
discoverability gap.
It is the cheaper half, and it should not be mistaken for
the whole answer.

Turn shortcuts on by default. Arguably the larger half of the problem,
deliberately not proposed. See #5483 and the Problem section.

Show the key on the button face rather than on hover. Rejected on three
grounds. The TX buttons are setFixedHeight(22) at 48–52 px wide (m_procBtn,
m_daxBtn, m_monBtn, m_voxBtn) or QSizePolicy::Ignored in a shared row
(m_tuneBtn, m_moxBtn, m_atuBtn); MOX (T) does not fit and
Toggle speech processor (⌘⇧P) is not close. The face is already a state display
TxApplet swaps TUNE's text to TUNING... — so the key would vanish exactly
when the button is doing something. And with most actions unbound, most faces
would carry nothing, making the two that do look arbitrary.

Annotate accessibleDescription instead of toolTip. Rejected. Tidier for a
machine-readable fact, and it reaches nobody who reported this: the operator here
is sighted, using a mouse, hovering a button.

Generate the annotation once at construction. Rejected because it is wrong in
the common case, not because it is inelegant — TxApplet rewrites two of these
tooltips on every availability change, so a construction-time annotation is
destroyed the first time the radio connects.

Implementation scope

Six source files and one test, about two hundred lines added. No new
dependencies, no new build options, no public API removed. A branch exists and is
unpushed; it is offered as evidence that the scope figure has something
behind it, not as a fait accompli.

The mechanism — src/core/ShortcutManager.h, src/core/ShortcutManager.cpp:
applyShortcutTooltips(QWidget*, bool) (the tree walk and the filter install,
guarded by a property so repeated walks do not stack filters);
annotateShortcutTooltip(QWidget*) (private, the whole decision for one widget);
an eventFilter override that reacts to QEvent::ToolTipChange and never
consumes; four property-name constants.

The wiring — four call sites. MainWindow_Shortcuts.cpp after the existing
rebuildShortcuts call, plus a bindingsChanged connection; and in
MainWindow_Menus.cpp, both the Configure Shortcuts… handler and the Keyboard
Shortcuts
master-toggle handler. buildUI() and buildMenuBar() both run
before registerShortcutActions(), so the applets exist and
m_keyboardShortcutsEnabled is loaded when the first walk happens.

The widgets that opted in — seven, all QPushButton, all in the TX
category, chosen because the TX controls are what the operator was reaching for:

widget file action ships bound to
m_moxBtn TxApplet.cpp mox_toggle T
m_tuneBtn TxApplet.cpp tune_toggle
m_atuBtn TxApplet.cpp atu_start
m_voxBtn PhoneApplet.cpp vox_toggle
m_procBtn PhoneCwApplet.cpp speech_proc_toggle
m_daxBtn PhoneCwApplet.cpp dax_toggle
m_monBtn PhoneCwApplet.cpp tx_monitor_toggle

On a stock install exactly one of them shows anything — MOX, reading
"Toggle manual transmit on or off (T — shortcuts disabled)". The other six are
unbound and stay bare. That is a modest return for the mechanism and it is stated
plainly rather than dressed up: the annotation is worth having because it is
right whenever it says anything, not because it says a lot on day one.

Two changes go beyond pure annotation and should be reviewed as separable.
Both exist because rule 4 — a widget with no tooltip gets the bare key — produces
a genuinely bad tooltip reading just T.

  • Five buttons that had no tooltip were given one: m_moxBtn, m_voxBtn,
    m_procBtn, m_daxBtn, m_monBtn. In every case the text is the widget's own
    existing accessibleDescription, verbatim. Nothing new was written; text
    already authored for the screen reader was made visible to the mouse.
  • The ATU button's available-state tooltip was empty and is now populated.
    TxApplet::updateAtuAvailability set an explanatory tooltip when the ATU is
    unavailable and cleared it to "" when available; with atu_start bound, the
    empty base would leave a bare key glyph with no indication what it does.

If a maintainer wants the annotation without those, they come out cleanly.

What was deliberately NOT tagged. Better seven buttons correctly than sixty
sloppily.

  • two_tone_tune — no widget invokes it. The nearest surface is the TUNE
    right-click menu, whose two QActions set the mode of the next TUNE press
    rather than toggling two-tone. Tagging it would mean tagging the wrong thing.
  • ptt_hold (Space) — no widget. It is momentary and there is no momentary
    PTT button; m_moxBtn is a latching toggle bound to a different action, so
    annotating it with Space would be a lie about what MOX does. This is the
    unsatisfying one:
    Space-for-PTT is the exact binding the operator asked about
    by name, and it is the one binding this cannot surface. It remains reachable
    only through ShortcutDialog — the strongest argument in this document for the
    "make the dialog easier to find" alternative.
  • The remaining sixty-one sites — not surveyed widget by widget. Many are
    frequency and band operations with no single owning button, and a sweep done at
    speed is how a wrong action id lands on a button that does something else. The
    mechanism is designed to be extended one widget at a time by whoever owns it.
  • TunerApplet::m_tuneBtn and SpeApplet::m_tuneBtn — both read as TUNE
    buttons; neither performs a ShortcutManager action.
  • The three tooltips that hand-name a keyHelpDialog's two find buttons
    and FreeDvReporterDialog::m_bandRadio — checked for the duplication risk and
    left alone. None of those keys is a ShortcutManager action. No tooltip
    anywhere in src/ is currently built from a QKeySequence.
Tests

New target shortcut_tooltip_test in tests/tests.cmake, 21 assertions,
running under QT_QPA_PLATFORM=offscreen. It needs a QApplication for
QWidget, so it cannot join shortcut_manager_test, which is a
QCoreApplication test.

It covers: a bound action produces the suffix; an unbound one does not; three
applies produce one suffix; a rebind replaces it and clearing removes it; a
widget with no tooltip gets the bare key; an unknown action id is untouched; a
widget without the property is untouched; the root is walked and not only its
children; an out-of-band setToolTip is re-annotated and does not stack; the
master toggle qualifies rather than strips, in both directions and through the
ToolTipChange path
; an unbound action stays silent in both toggle states;
and the display string is NativeText rather than PortableText.

Each was proved red before green by deliberately regressing the implementation
and rebuilding:

  • Reverting to the strip behaviour fails the three re-pointed toggle assertions,
    each reporting the bare base where the qualified form was expected. The two
    unbound-action assertions pass under both, which is the invariant they exist to
    pin.
  • Removing the event-filter installation fails "out-of-band setToolTip is
    re-annotated, not clobbered"
    .
  • Switching to PortableText fails the NativeText case with Base (Ctrl+M).
  • Removing the "is this still what we wrote?" guard does not fail an assertion —
    it crashes with a stack overflow, because that guard is also what
    terminates the re-entrant setToolTipToolTipChangesetToolTip loop.
    Worth knowing before someone tidies it.

Full ctest -j4: 357 of 357 pass, 52 s, four skipped for unrelated platform
reasons.

Verified in the running application

Launched under AETHER_AUTOMATION=1 per docs/automation-bridge.md, on an
isolated settings store migrated from the operator's own profile, and read back
with dumpTree. With View → Keyboard Shortcuts unchecked — the shipped
default:

MOX transmit                  toolTip = 'Toggle manual transmit on or off (T — shortcuts disabled)'
Tune                          toolTip = 'Start or stop tune carrier'
ATU tune                      toolTip = 'Start automatic antenna tuner'
VOX voice-operated transmit   toolTip = 'Toggle voice-activated transmit'
Speech processor              toolTip = 'Toggle speech processor for compression'
DAX digital audio             toolTip = 'Toggle DAX digital audio exchange'
TX monitor                    toolTip = 'Toggle sidetone monitor of transmitted audio'
ATU memories                  toolTip = None

MOX names its key and says why it is inert; the six unbound buttons correctly say
nothing; ATU memories, which carries no property, is untouched — that is what
"a widget that did not opt in" looks like from outside.

invoke "Keyboard Shortcuts" trigger drops the qualifier:

MOX transmit                  toolTip = 'Toggle manual transmit on or off (T)'

and a second trigger restores it with the base intact:

MOX transmit                  toolTip = 'Toggle manual transmit on or off (T — shortcuts disabled)'
Known risks, in the order a reviewer should worry about them
  1. The event filter recurses if its guard is removed. setToolTip sends
    QEvent::ToolTipChange synchronously, re-entering the annotator. Termination
    depends on kAppliedTooltipProperty being written before setToolTip.
    Verified by disabling the guard, which crashes with a stack overflow rather
    than failing an assertion. The ordering is commented at the call, but it is
    load-bearing.
  2. A widget built after the walk is never annotated. applyShortcutTooltips
    is a walk plus a signal connection, not a registry. Every applet touched here
    is constructed in buildUI(), before registerShortcutActions(), so it is
    correct today — but a lazily-constructed panel would need its own call and
    nothing would warn whoever wrote it. This is the weakest part of the
    design.
  3. The qualified string is longer, and it is the default state's string. See
    the tooltip-length note under Cross-platform impact. Not measured anywhere.
  4. "shortcuts disabled" goes through tr(); the bracket form around it does
    not.
    The suffix is assembled as "%1 (%2)" outside tr(), so a locale
    wanting different brackets, or an RTL locale wanting the key first, cannot
    express that. Deliberate — a translatable format string invites a translator
    to drop a placeholder — but it is a limitation.
  5. Three dynamic properties per opted-in widget hold state that is invisible
    unless you know their names. Seven widgets today; the cost is nothing, the
    obscurity is real.
  6. Not verified on Linux or Windows. Repeated here because it belongs in a
    risk list.
  7. The master-toggle state is latched, not observed. applyShortcutTooltips
    stores its shortcutsEnabled argument so the ToolTipChange re-annotation
    honours the same answer. All four call sites pass the live value today, and
    there is no signal on ShortcutManager for the master toggle to connect to
    instead — adding one would be the tidier fix and is not attempted here.

🤖 Generated with Claude Code

https://claude.ai/code/session_014TtnKQu6QGrSeBirGeAsAs

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading ShortcutManager, MainWindow::registerShortcutActions and rebuildShortcuts, and the TxApplet handlers that rewrite tooltips. Trace the proposed widget property and update entry points, then review the mentioned annotation test. Done means bound keys appear with native text, disabled shortcuts are qualified rather than hidden, unbound or unknown actions remain unchanged, and repeated updates do not stack suffixes.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
desktop
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.