Skip to content

fix(gui): availability registry keeps own tooltips. Principle VIII. - #6253

Merged
rfoust merged 5 commits into
aethersdr:mainfrom
on8st:fix/availability-registry-keeps-tooltips
Oct 12, 2026
Merged

rfoust merged 5 commits into
aethersdr:mainfrom
on8st:fix/availability-registry-keeps-tooltips

Conversation

@on8st

@on8st on8st commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #5859

Result

Supported controls keep their own tooltips and descriptions. While unavailable, the registry shows the reason on the tooltip and accessibility channel; when available again, it restores the owner's latest text. Inactive widgets append "Available, not currently active" to their own description.

Implementation and scope

The temporary text hold is stored on the control as {own, shown}, so recreating a registry while the reason is showing preserves the original text. Owner writes in either state are adopted at the next apply. An empty reason releases the hold instead of blanking the owner's text.

QAction tooltip ownership is detected through Qt's ActionChanged event, with temporary probe events and signals suppressed. An explicit tooltip remains explicit even when it equals the label. An action without an explicit tooltip keeps following its text, including after the owner clears the tooltip while unavailable. Existing signal blocking is preserved.

Files Purpose
src/gui/ControlAvailabilityRegistry.{cpp,h} Preserve and restore widget/action text, including explicit and text-derived action tooltips
docs/style/theme-style-guide.md Document the text-composition rule
docs/user/docs/accessibility.md Describe restored descriptions and inactive announcements for users
tests/control_availability_registry_test.cpp Socket-free restoration, owner-write, registry-recreation, action-rename, equal-label explicit-tooltip, clear-while-held, blocked-signal, probe-notification, and empty-reason coverage
tests/rx_applet_squelch_reconciliation_test.cpp Socket-free real-applet capability round trip with no slice

No new protocol, settings, CLI, or capability surface is added. Current main aa0dd0e4 was merged without dropping upstream changes; the three follow-up commits are SSH-signed and GitHub-verified.

Independent verification at 8266702aa

  • Both focused CTests pass: control_availability_registry_test and rx_applet_squelch_reconciliation_test.
  • Mutation checks: substituting main's pre-fix registry makes both original regression targets fail. Substituting the original PR helper at abb02d71 makes all five new follow-up assertions fail; they pass with this implementation. The unchanged squelch regression still passes against abb02d71, as expected.
  • macOS app build passes with RADE enabled and the ARM64 toolchain. Host/system/executable are ARM64; the Ninja graph contains no RNNoise x86 sources.
  • npm ci && npm run build passes in docs/user; the generated accessibility page contains the final wording.
  • Baseline and updated apps were driven offscreen against only DEMO-0001, using fresh isolated settings, explicit sockets, and TX disabled. Tooltip/description text survived the exercised clicks and disconnect. Every review instance was closed.
  • Fresh CI is tracked on this exact head. The final-head full CTest suite, screen-reader output, live-radio behavior, and Linux/Windows runtime were not independently tested locally.

Limits

An owner write made after a registry apply is covered or composed at the next apply, not immediately. RxApplet's applySqlModeVisuals() remains after refreshEngaged() because it also updates threshold and Auto SQL text. Its later squelch-button description write can temporarily omit the inactive phrase; its Off-mode slider tooltip still overrides the reason. These are existing ordering limits and remain outside this fix.

Interaction with #5484's proposed shortcut tooltip annotation has not been tested; its composition order still needs settling before that proposal lands.

ControlAvailabilityRegistry::applyOne wrote an empty tooltip, and a
state description, whenever a control was not Unavailable. Every
registered control therefore lost its own tooltip and accessible
description (QAction: tooltip and status tip) on the radios that
support it, and RxApplet re-set its squelch texts by hand afterwards.

The registry now holds a text channel only while it shows text of its
own there: the reason while Unavailable, and the "Available, not
currently active" announcement only on a widget with no description of
its own. Anything the owner wrote since the last apply is the
control's own text and is what comes back when the registry lets go;
an owner write while the reason shows is replaced by the reason at the
next apply. The hold is kept on the control as a dynamic property, so
a registry re-created while the reason shows (RxApplet and VfoWidget
rebuild theirs in setRadioModel) still restores the own text.

RxApplet's applySqlModeVisuals() after refreshEngaged() stays: it no
longer has to restore the tooltip, but it also re-derives the slider's
threshold description and the Auto SQL text, which change while the
registry holds the channel.

Tests: a pre-tooltipped, pre-described widget and QAction through
Available -> Unavailable -> Available, owner writes in both states, a
re-created registry, an action keeping its text-derived tooltip; and
RxApplet's squelch button texts surviving registration and a
capability round trip with no slice, a path where RxApplet re-sets
nothing.

Fixes aethersdr#5859

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st
on8st requested a review from a team as a code owner October 7, 2026 17:49

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 568521d1, against the PR head checkout. I could not build or run anything, so every finding below is reasoned from code, not reproduced at runtime; I say so where it matters.

1. Issue fit — yes

#5859 was re-titled on 7 Oct to "ControlAvailabilityRegistry::applyOne clears a control's own tooltip whenever it is not Unavailable", which is exactly this defect, and the thread already converged on it: the triage comment listed it as item 1 with the fix shape ("preserve the widget's own tooltip on the Available path instead of writing QString()", plus "add a test registering a widget that already has a tooltip"), and you offered to write it. The issue body is stale — it argues about zero call sites, which #5924 settled — but the title and the thread are the live ask, so Fixes #5859 is right. The maintainer may still want to read the issue body's "option 3 / header staging note" as separately closed: the header rewrite here covers it, since the staging note is moot once call sites exist.

The fix also lands the stronger half of the thread's ask. Your test asserts restoration across Unavailable→Available, not merely preservation across registration — the distinction you drew yourself in that thread ("those are different bugs and only the second is the one that bites"). The mechanism is wired correctly:

Requirement from the thread Where
Available path stops writing QString() over the widget's tooltip src/gui/ControlAvailabilityRegistry.cpp:36-59 (applyText), :147-151
Same treatment for the QAction branch :177-191
Test registers a pre-tooltipped widget and asserts restoration tests/control_availability_registry_test.cpp:296-371
Doctrine written down docs/style/theme-style-guide.md:185-187

Test layer: both are socket-free CTest, the right layer (GUI state machine, injected capabilities). Neither adds a socket, peer process, or Fake* radio/amp/tuner class — I grepped both test files for QTcpServer/QUdpSocket/QLocalServer/bind(/listen(/connectToHost/Fake and got nothing. rx_applet_squelch_reconciliation_test is already registered (tests/tests.cmake:7467-7493) and the three new includes are headers, so no link change is needed.

One thing to know about the green badge: neither test is in any ctest -R filter in ci.yml (the only filtered steps are the DV, cross-needle, mac-NR and ASR ones). So the five green checks prove these files compile on all three platforms — which is real and is where _USE_MATH_DEFINES/NOMINMAX-class breaks would show — but they did not execute either new test. Your local 653/657 run is the only execution evidence, and I can't verify it.

2. Scope

File What it changes Claimed? Verdict
src/gui/ControlAvailabilityRegistry.cpp describe() → applyText() hold mechanism; widget + action text channels Yes In scope
src/gui/ControlAvailabilityRegistry.h registerWidget doc comment restated for restore semantics Yes In scope
docs/style/theme-style-guide.md +4 lines under the three-state table Yes In scope (see §4)
tests/control_availability_registry_test.cpp +78, the restoration cases Yes In scope
tests/rx_applet_squelch_reconciliation_test.cpp +35, real-applet round trip Yes In scope

Everything in the diff is explained by the issue. No new public surface: no protocol verb, no settings key, no capability field, no CLI flag, no bridge verb. The two dynamic-property keys (ControlAvailabilityRegistry.cpp:61-62) are new internal state on the controls, but they are private to this TU — I grepped the whole head checkout for aetherAvailability and those two lines are the only hits, so nothing else reads them and no QSS selector can be relying on them. No CHANGELOG.md entry, correctly.

On the - lines: the deleted describe() helper's comment cited #4896 and #5266/#5299 for why the reason rides both channels. The reasoning survives in condensed form at :144-146, and the #5266/#5299 history is still in theme-style-guide.md:189-196. No guard was removed — setEnabled(!unavailable) and the reason-on-both-channels invariant both still fire on the Unavailable path. Not a regression.

3. Blockers

None.

4. Needs maintainer decision (one item, and it is narrower than it looks)

src/gui/ControlAvailabilityRegistry.cpp:158-165 + docs/style/theme-style-guide.md:182

return entry.state == ControlAvailability::Inactive && own.isEmpty()
    ? QObject::tr("Available, not currently active") : QString();

The style-guide table immediately above your new prose still reads unconditionally:

| inactive | supported here, not engaged now | color.control.inactive | "Available, not currently active" |

That row encodes the #5262 M3a ruling, and your three lines at :185-187 qualify it three lines later. A reader hitting the table first gets the old rule. You flag this as the maintainer's call in the PR body, which is the right handling — I'm just locating it and sizing it.

It sizes small. I walked every current call site to find where today's behavior actually changes:

  • RxApplet.cpp:2100 / VfoWidget.cpp:6686 (m_sqlBtn) — applySqlButtonDescription() / syncSqlVisuals() already write an own description and already won on main. No delta.
  • RxApplet.cpp:2103 / VfoWidget.cpp:6689 (m_sqlSlider) — own description written by describeThreshold. No delta.
  • WfmApplet.cpp:275-295 — every engaged predicate is []{ return true; }, so these are never Inactive. No delta.
  • ReceiveCaptureAction.h:19 — a QAction; the inactive phrase never went to statusTip on either side.
  • RtlReceiverSettingsWidget.cpp:82 (m_dc) — the one site with a real engaged predicate, and it sets no accessibleDescription (only m_ppm gets an accessibleName, line 43), so own.isEmpty() is true and the phrase is still written.

So as far as I can tell the user-visible delta today is zero; the ruling matters for M3b conversions, not for 26.10.x. Worth stating in the body.

And the append alternative you offer needs no ruling at all. Appending the phrase to the widget's own description satisfies the table row verbatim while still restoring the own text, which is the whole point of the PR. I'd suggest making that the default and letting the maintainer decline it — it turns a doctrine amendment into a pure bug fix. If they prefer the conditional form, the table row at :182 should pick up the qualifier so the section doesn't read two ways.

5. Nits (all non-blocking)

  • src/gui/ControlAvailabilityRegistry.cpp:177-184 — the action-tooltip restore freezes a stale derived default if the action's text changes while the reason is held. The comment says:

    // An action with no tooltip of its own reports its text; clearing first
    // keeps that default instead of freezing a copy of the text.

    That holds only when the text is unchanged across the hold. Walk it: action text &Tune, no explicit tooltip, so own is captured as "Tune". While Unavailable the owner calls setText("&Stop"). On restore, set("Tune") does setToolTip(QString()) → toolTip() is now "Stop" → "Stop" != "Tune" → setToolTip("Tune"). The action is left with an explicit, wrong tooltip and no longer follows its text. Your plain case at tests/control_availability_registry_test.cpp:363-370 renames after restore, so it doesn't reach this. Narrow — I found no production action that renames while gated — but the comment overstates the guarantee. Worth either narrowing the comment or recording "had no explicit tooltip" as a third hold element instead of the derived string.

  • src/gui/ControlAvailabilityRegistry.cpp:36-59 — nothing releases a hold on destruction. RxApplet.cpp:2078-2079 does delete m_filterAvailability; and VfoWidget rebuilds likewise, so a registry can die while a control is Unavailable: the control keeps the reason, the disabled state, and an orphaned hold property. A later registry recovers it (your second case at :336-344 proves that), but setRadioModel(nullptr) builds no registry at all and the control stays dimmed with a reason that no longer applies. Pre-existing — main left the same stuck state — but now that own is stored, releasing in the destructor is a few lines, where before there was nothing to release to.

  • src/gui/ControlAvailabilityRegistry.cpp:44 — an empty-but-non-null reason silently reinstates the bug. The release sentinel is want.isNull(), so a caller passing QStringLiteral("") (non-null, empty) makes want non-null and the registry holds an empty tooltip over the control's own — exactly the #5859 defect, through the front door. The header says reason is REQUIRED but registerWidget only rejects a null widget/predicate. Every current call site passes a real tr() string, so this is hardening, not a live defect; an early if (reason.isEmpty()) return; or a comment naming the sentinel would close it.

  • The PR body's applySqlModeVisuals() paragraph covers the button, not the slider. You write that the call "no longer has to restore the squelch tooltip". For m_sqlBtn that's right — applySqlButtonDescription() has the stateOf(...) == Unavailable guard (RxApplet.cpp:1443-1447), and VfoWidget::syncSqlVisuals() has the matching sqlReasonOwned guard (VfoWidget.cpp:6451-6453). The slider has no such guard: RxApplet.cpp:3259-3260 runs refreshEngaged() then applySqlModeVisuals(), and in SqlMode::Off the slider is Unavailable (its predicate is supportsSquelch && m_sqlMode != SqlMode::Off) while the case SqlMode::Off: arm at :1424-1428 writes

              m_sqlSlider->setToolTip(
                  "Squelch threshold. Increase to require a stronger signal "
                  "before audio opens.");

    unconditionally and last. So the slider's reason ("Enable squelch in a supported receive mode to adjust its threshold") never reaches its tooltip. Identical on main — the owner won there too — so not a regression and not this PR's job. But the new rule makes it permanent rather than incidental: the owner's write becomes the recorded own, so restoration correctly gives back the generic text and the reason is never shown. Worth a sentence in the body so a later reader doesn't take "the registry now owns this correctly" to mean the slider's reason is reaching anyone.

  • The own-text heuristic is the #5484 collision surface. applyText infers "anything ≠ what I last wrote is the owner's text". #5484's proposed QEvent::ToolTipChange filter re-annotates tooltips after the write, so the suffix would be adopted as own and then re-appended on the next restore. You disclose that the two are untested together; I'd name it as the thing to settle before #5484 lands, since the triage comment on #5859 already asked for a composition order and this PR is now the incumbent.

  • docs/style/theme-style-guide.md:233-235 (adjacent, untouched) still reads "M3a introduces the mechanism; those production consumers and their second pushes have not yet migrated (M3b)." Stale since #5924. Out of scope — flagging only because you're in this file.

6. What I tried to break and could not

  • Null/empty round-trip through the dynamic property. QStringList{QString(), reason} → QVariant → toStringList(): a widget with no prior tooltip stores a null own, and the release path's set(own) clears correctly. hold != next compares with QString::operator==, which treats null and empty as equal, so a null↔empty flip just skips a redundant property write. No stuck state.
  • Registry re-creation mid-reason, which is the design's whole risk. Traced :296-345 by hand: the inner second registry reads hold.at(1) == current, recovers own, and releases to the owner's latest text rather than adopting the reason. The outer registry is still connected to the same model but refreshEngaged() emits nothing, so it isn't re-entered. The test's final assertion is the one that would fail on a wrong hold, and it checks meanwhile (the write made while unavailable), not the original — the harder of the two.
  • Duplicate and overlapping registrations. RxApplet.cpp:3362 and VfoWidget.cpp:5932 register m_filterUnavailable from a function that re-runs on mode change; the widget is deleted and rebuilt each time and pruneDead() runs first in applyAll, so no dangling entry. Two registries over the same widget share one property key, but with equal own and equal reason the writes are idempotent. RxApplet's and VfoWidget's m_sqlBtn are distinct objects, not one widget seen twice.
  • Owner-write ordering in both directions. If RxApplet's own capabilitiesChanged handler runs before the registry's lambda, its stateOf guard still sees the stale Inactive and writes the own description — which the registry then correctly records as own and covers with the reason. If it runs after, the guard sees Unavailable and returns. Both orders land in the same place; I could not construct a connection order that strands the reason or loses the own text permanently.
  • The inactive-announcement change against every live call site. Enumerated in §4 — five sites, four with no behavioral delta and one (m_dc) where the phrase is still written. I looked specifically for a site that would silently stop announcing today and did not find one.
  • Lifetime. applyText binds current to the temporary returned by w->toolTip(); the binding lives for the full call expression, so no dangling reference. Entry holds QPointers and pruneDead() precedes iteration.
  • The tests against a broken fix. The registry test's first new assertion fails on main at registration (main's Available path writes QString() with a real tooltip present), and the meanwhile/second-registry assertions each pin a distinct part of the hold, so none of them passes against an emptied applyText. I could not find a check that asserts the implementation back to itself. Not reproduced — I have no build.
  • Could not check: anything requiring execution or a display. No Linux/Windows run, no screen reader, no confirmation that your 653/657 figure or the five mutation results are as described. The CodeGuard section and the Copilot section in my brief were both empty, which means no findings or no scan — I'm not reading either as a clean bill.

7. Recommendation

Approve with nits, with the inactive-announcement question routed to the maintainer. The defect is real and was already triaged as item 1 on #5859; the fix is well-shaped, the hold-on-the-control choice is the right one given that RxApplet/VfoWidget delete and rebuild their registries, and the tests finally cover the case the old fixture was structurally blind to. All five files are explained, nothing public was added, and no guard was removed.

Concrete next step: offer the append form of the inactive announcement (phrase appended to the control's own description). That satisfies theme-style-guide.md:182 verbatim and removes the only item on this PR that needs a ruling — if the maintainer prefers the conditional form you've written, the table row should pick up the qualifier. The nits are all optional; the action-rename one and the empty-reason sentinel are the two I'd actually spend lines on. Thanks for tracing this from your own triage thread through to a test that fails for the right reason.


🤖 aethersdr-agent · cost: $9.6850 · model: claude-opus-5

Review on aethersdr#6253. The inactive announcement is now appended to a
control's own accessible description instead of being written only on
a control without one, so the three-state table's "Available, not
currently active" holds verbatim and no doctrine change is needed. The
style-guide lines under the table say so.

An action with no tooltip of its own is now held as a null own text, so
releasing the hold makes it follow its text again. Before, the derived
text was captured, and an action renamed while unavailable (&Tune to
&Stop) was left with an explicit, stale "Tune" tooltip.

An empty reason now releases instead of holding an empty tooltip over
the control's own text.

Tests: the inactive append (with and without a closing full stop, and
an owner write while inactive), an action renamed while unavailable,
and an empty reason. The squelch reconciliation test expects the
appended form on the Inactive squelch button.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@on8st

on8st commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review. Bad news first. The append form has one gap. An owner that rewrites its description right after the registry applies drops the inactive phrase until the next apply. RxApplet does exactly that for the squelch button: refreshEngaged() and then applySqlModeVisuals(), which writes the button's own description when it is not Unavailable. So after a squelch-mode change to Off, the button reads its own description without the phrase until the next capability or engaged refresh. The reason has the same "next apply, not immediately" behaviour, as the body already says. I did not change RxApplet's order in this PR. Its Unavailable guard needs the refreshed state first.

The inactive announcement, now the append form (your concrete next step). Done in abb02d71. While Inactive, "Available, not currently active" is appended to the control's own accessible description: "own. Available, not currently active", or after a space when the own text already ends in ., ! or ?. A control with no description of its own gets the phrase alone, as before. The own description comes back without the phrase when the control turns Active. So the three-state table row holds verbatim, and the three lines under it now say the phrase is appended. This PR no longer asks for a doctrine ruling. If the maintainer prefers the conditional form (the phrase only on a control without a description), it is a one-line change. In that case the table row should get the qualifier.

The action tooltip that froze a stale default (nit). Confirmed by test, not just by reading. On 568521d1 an action &Tune renamed to &Stop while unavailable came back with an explicit "Tune" tooltip and stopped following its text. Now an action with no tooltip of its own is held as a null own text, so letting go clears the tooltip and the action follows its current text again. The new test case renames while unavailable, then renames again after it is restored. One edge: an explicit tooltip that equals the action's derived text is treated as "no own tooltip". It looks the same until the text changes.

The empty-but-non-null reason (nit). Done. An empty wanted text now lets go instead of holding, so an empty reason leaves the control's own tooltip in place rather than blanking it. The control is still disabled. New test case.

Not done, and why:

  • Releasing holds in the destructor: the stuck state after setRadioModel(nullptr) already exists on main. Releasing the text alone would leave the control disabled and dimmed with its own tooltip, which is a half-fix. I left it out to keep this PR a pure bug fix and can file it separately if wanted.
  • The slider's reason never reaching its tooltip in SqlMode::Off: as you say, it is the same on main and not this PR's job. I will add a sentence to the body.
  • [RFC] Name a button's keyboard shortcut in its tooltip, qualified rather than hidden when shortcuts are off #5484: agreed, the composition order needs settling before it lands. I will name it in the body as the open item.
  • theme-style-guide.md:233-235 (M3a/M3b): it is in this file, but it is not a one-line correction. RxApplet, VfoWidget, WfmApplet and the RTL settings use the registry now. The Calibration page and DemoApplet that the sentence names still do not, so part of it is still true. I left it alone.

Tests. Full ctest on macOS (Qt 6.12.0, offscreen), head abb02d71 on 121340c0: 653 of 657 pass at -j8, 654 after the serial rerun. control_availability_registry_test and rx_applet_squelch_reconciliation_test pass. The three remaining failures fail on clean main on this machine too, each with only its known check lines: control_receive_test (#6248), peripheral_auth_dialog_test (#6008) and unified_title_bar_test (#6224). wdsp_channel_test failed once at -j8 ("the Q-only leg reported an error") and passed serially. It has failed the same way at -j8 on other commits here, including a main control run. This PR touches no WDSP file. Our harness reports RESULT: RED pr6253-review abb02d71 because #6248 is not on its waiver list; I checked each failing line by hand.

Red on mutation (scratch commits, never pushed, full suite each):

  • Inactive phrase only when the control has no description of its own (the previous form): RED. Two registry checks fail ("inactive appends its announcement to the control's own description", "an owner write while inactive gets the announcement appended once"), and so does squelchButtonTextsSurviveAvailability.
  • Action tooltip captured as the derived text (the 568521d1 behaviour): RED. Two registry checks fail ("an action renamed while unavailable reports its new text", "and it keeps following its text afterwards").

Not verified: the empty-reason case was not mutation-tested. No Linux or Windows run, no screen reader, and the app was not launched. As you note, CI does not run either test.

73, ON8ST

@Ozy311 Ozy311 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of abb02d71, the one commit since the aethersdr-agent review at 568521d1.

The one item that needed a ruling no longer does. The inactive announcement is now appended to the control's own description (withInactivePhrase(), ControlAvailabilityRegistry.cpp:67-78) instead of being used only when the control has none. That satisfies the style-guide table row ("Available, not currently active") verbatim, and the prose under the table now says the same thing (theme-style-guide.md:187). The review suggested this as the version that needs no maintainer decision.

Disclosed behaviour change. Controls that have their own description now announce the phrase while Inactive. One example is the squelch button with squelch off: rx_applet_squelch_reconciliation_test now expects own + ". Available, not currently active". That is the M3a ruling being applied as written, not a preference, but a screen-reader user will hear it, so it may be worth a line in the PR body.

Two of the nits are fixed with tests:

  • An action renamed while unavailable. The hold now records the action's explicit tooltip (null when it only reports its text), through explicitToolTip(). On release it therefore follows the new text instead of freezing the old one. The new check renames &Tune to &Stop while the action is unavailable and expects Stop, and then Go after a further rename.
  • An empty reason. The release sentinel is now isEmpty(), not isNull(), so QStringLiteral("") can no longer blank the control's own tooltip. A new check covers it.

I traced the append path across repeated applies. While the phrase is shown, current == hold[1], so own comes from the hold and the phrase is never appended twice. An owner write in the meantime becomes the new own and gets the phrase once; the "owner write while inactive" check pins that. The branch merges cleanly with current main. Not run: either test, since both need the core build.

Still open from the earlier review, and fine as follow-ups: releasing the hold in the registry destructor, and how this combines with #5484's tooltip annotation. Nothing new from me.

73, K6OZY


Generated by Claude Code

Detect stored QAction tooltip ownership through ActionChanged while suppressing the probe notifications, and normalize owner writes when releasing a held channel. Preserve explicit tooltips equal to the label and let cleared tooltips continue following later labels.

Add socket-free regressions for both cases, signal blocking, and transient notification suppression. Document restored descriptions and inactive announcements in the user accessibility guide. The new assertions fail against the original PR helper and pass with this change.

@rfoust rfoust left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at 8266702aa against current main aa0dd0e4. This solves the narrowed #5859 tooltip-restoration defect, including Available -> Unavailable -> Available restoration, owner writes, and registry recreation. The user-guide omission and both QAction ownership edge cases from my review are fixed.

Scope

Files Change Verdict
src/gui/ControlAvailabilityRegistry.{cpp,h} Preserve/restore control-owned text; preserve explicit and derived QAction tooltip ownership In scope
docs/style/theme-style-guide.md State the text-composition rule In scope
docs/user/docs/accessibility.md Document restored descriptions and inactive announcements for users In scope
tests/control_availability_registry_test.cpp Restoration, owner-write, recreation, explicit/derived tooltip, blocked-signal and probe-notification regressions In scope
tests/rx_applet_squelch_reconciliation_test.cpp Real-applet own-text capability round trip In scope

Everything in the diff is explained by the issue and the review fixes. No new protocol/settings/CLI surface, unrelated restyle, changed default, removed safety guard, or changelog entry.

Blockers

None remaining.

Nits

None outstanding from this review. Explicit tooltips equal to the label now remain explicit. Clearing a held tooltip releases it to the current text-derived default and it continues following later renames. The capture probe suppresses temporary ActionChanged events/signals and preserves existing signal blocking.

Verification

  • Independent macOS ARM64 build passed with RADE enabled. Host/system/executable are ARM64 and the Ninja graph contains no RNNoise x86 sources.
  • Both socket-free focused CTests passed: control_availability_registry_test and rx_applet_squelch_reconciliation_test.
  • Mutation proof: substituting main's pre-fix registry made both original regression targets fail. Substituting the original PR helper (abb02d71) made all five new follow-up assertions fail; they pass with the reviewed fix.
  • npm ci && npm run build passed for docs/user; checked the generated accessibility page contains the final wording.
  • Offscreen baseline and updated app bridge sessions used fresh isolated settings, explicit sockets, and only the built-in demo. Selected get radio fields: connected: true, model: "AetherSDR Demo", serial: "DEMO-0001", transmitting: false. Squelch tooltip/description text survived the exercised clicks and disconnect. The disclosed RxApplet owner-write ordering limitation remains unchanged. All review instances were closed.
  • No screen-reader, live-radio, Linux runtime, or Windows runtime validation is claimed. The demo cannot prove hardware capability convergence. No separate automated review pass was available.

What I tried to break

Text restoration after unavailable state, owner writes in either state, registry recreation mid-hold, repeated inactive composition, action rename before/after release, explicit text equal to the default, clearing while held, blocked signals, transient probe notifications, and empty reasons. The focused tests survived, and the inverted implementations failed as described.

Approved after the final push and fresh required CI passed on this exact head. No administrator bypass is requested.

@rfoust
rfoust merged commit 8207725 into aethersdr:main Oct 12, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ControlAvailabilityRegistry::applyOne clears a control's own tooltip whenever it is not Unavailable

3 participants