fix(gui): reopen the Aetherial strip after minimize — Principle XI. - #5366
fix(gui): reopen the Aetherial strip after minimize — Principle XI.#5366crypticpy wants to merge 2 commits into
Conversation
Minimizing the Aetherial Audio Channel Strip made it unrecoverable from
its own button: no number of presses on the AetherVoice button (or the
chain applet's nub) would bring the window back, leaving the taskbar/Dock
as the only way to reach it.
Two Qt behaviours combine to cause it, and toggleAetherialStrip() was
written as if neither existed:
1. QWidget::isVisible() stays TRUE while a window is minimized, so
`if (m_aetherialStrip->isVisible())` sent a minimized strip down the
hide() branch.
2. QWidget::show() on a minimized window restores its SAVED state —
still minimized — so the follow-up press did not recover it either.
Both are now pinned against a real QWidget rather than asserted from
memory, on the offscreen platform where they reproduce.
Extract the decision into gui/WindowShowState.{h,cpp}:
* windowIsShowing(w) — visible AND not minimized.
* showAndRaiseWindow(w) — showNormal() when minimized, show() otherwise,
then raise + activate.
showNormal() stays guarded on isMinimized() because calling it
unconditionally would clear a Maximized or FullScreen window — the same
reason the net-reminder and tray raise paths in MainWindow_Nets.cpp guard
it (aethersdr#3918). The helper is where those two sites should eventually
converge; not touched here to keep this change to the reported bug.
The docked CWX and DVK panels use the same bare isVisible() shape but are
child widgets that cannot be minimized independently, so they are
unaffected and are left alone.
New regression test window_show_state_test drives a real QWidget through
hidden → shown → minimized → restored and runs the toggle exactly as
MainWindow does. Mutation-checked three ways, each failing a different
assertion:
* windowIsShowing() without the !isMinimized() term → 3 failures
* showAndRaiseWindow() with an unconditional show() → 2 failures
* showAndRaiseWindow() with an unconditional showNormal()→ 1 failure
(the aethersdr#3918 maximized guard)
Found in local field use on macOS 26.5.2 (25F84) / Qt 6.11.1; filed as aethersdr#5365.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015HjpgedubkqsxhbYrsTzMR
ten9876
left a comment
There was a problem hiding this comment.
Issue fit
#5365: the Aetherial strip button treated a minimized strip window as "already showing", so pressing it did nothing — the operator had to un-minimize by hand. The fix makes windowIsShowing() require !isMinimized() and adds showAndRaiseWindow() that un-minimizes before raising. Correct and directly on-target, extracted into a testable free function with a mutation-checked regression test (window_show_state_test) that pins both Qt behaviors and the #3918 maximize guard. Principle XI honored.
Scope
Clean — the helper, its one call site, the test. CHANGELOG.md correctly untouched. Preflight: no sockets, pure widget-state test.
Blockers
None.
Nits (non-blocking)
- Minimize-from-maximized reopens un-maximized (inline).
showNormal()clears both the Minimized and Maximized bits, so maximize the strip → minimize → reopen brings it back at normal size, not maximized — a corner of the exact reopen path this PR fixes, and one the test misses (case 7 only checks maximize is preserved when the window was never minimized).w->setWindowState(w->windowState() & ~Qt::WindowMinimized)clears only the minimized bit and preserves maximize. - The
isMinimized()/showNormal()/raisepattern now lives in three places (MainWindow_Nets.cpp:226-247has two copies for #3918). The body acknowledges convergence "eventually"; this helper takes aQWidget*and could absorb those net-reminder/tray paths now, so a future fix (like nit 1) lands once.
What was verified vs read
- Verified by me:
showNormal()clears the maximized bit (nit 1's mechanism, uncovered by the test); the helper resolves unqualified inMainWindow.cpp's namespace. - From the automated pass, verified: first-press-creates-then-shows still works; the mutation test genuinely pins the two Qt behaviors.
- Not run: no bridge session (the widget-state helper is unit-covered), tests read not executed — no CI yet, which is a merge gate.
| return; | ||
| // showNormal() only for a minimized window — see the header note on #3918. | ||
| if (w->isMinimized()) | ||
| w->showNormal(); |
There was a problem hiding this comment.
Nit (non-blocking) — showNormal() drops a prior maximize. It clears both the Minimized and Maximized bits, so minimize a maximized strip and reopen → it returns un-maximized. Clear only the minimized bit to preserve the pre-minimize size:
| w->showNormal(); | |
| if (w->isMinimized()) | |
| w->setWindowState(w->windowState() & ~Qt::WindowMinimized); | |
| else | |
| w->show(); |
Add a test row (maximize → minimize → reopen → still maximized); case 7 doesn't cover the minimized-from-maximized path.
… — Principle XI. Review of aethersdr#5366 pointed out that showNormal() clears the Maximized and FullScreen bits along with Minimized, so a strip that was maximized, then minimized, then reopened from its button came back at normal size. That is a corner of the exact reopen path the PR fixes. showAndRaiseWindow() now clears only Qt::WindowMinimized via setWindowState(), which is a pending state on a hidden widget and immediate on a visible one, so the isMinimized() branch goes away too. The same isMinimized()/showNormal()/raise sequence lived twice more in MainWindow_Nets.cpp (the net-reminder tune and the tray-message click, both aethersdr#3918). They carried the same latent un-maximize and now call the helper, so the fix lands once. Braces restored on the strip toggle to match the surrounding style. Test: window_show_state_test gains case 8 — maximize, minimize, toggle — and asserts the window is showing AND still maximized. Mutation-checked: the old showNormal() body fails only that final assertion. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FhDXan9Qe86EbypHmxzHUy
|
@ten9876 Both nits taken, pushed as 5a48797. Nit 1 — minimize-from-maximized. Nit 2 — the two copies in One style-only change of my own: the strip toggle in |
Ozy311
left a comment
There was a problem hiding this comment.
Issue fit
The fix resolves #5365. Independently reproduced the lockout in the actual pre-fix application and verified one-click recovery in the PR application on macOS 26.5.2 (25F84), arm64, Qt 6.11.1. The latest commit also addresses both points from the earlier maintainer review: restoring a minimized window preserves maximized/fullscreen state, and the two net-reminder raise paths share the helper.
Scope
| Files | Change and provenance | Verdict |
|---|---|---|
MainWindow.cpp, WindowShowState.{h,cpp} |
Correct the strip toggle and restore only the minimized state bit; described by #5365 and the review follow-up | In scope |
MainWindow_Nets.cpp |
Reuse the helper at both raise sites, explicitly requested in the earlier maintainer review and disclosed in the author's follow-up | In scope |
CMakeLists.txt, tests/tests.cmake, window_show_state_test.cpp |
Compile the helper and register socket-free Qt widget regression coverage | In scope |
No unrelated changes, new settings, protocol changes, dependencies, or changelog churn. The PR summary still describes the earlier showNormal() implementation and says the Nets sites are untouched; the follow-up comment accurately describes the final code.
Blockers
None found.
Nits — non-blocking
- Refresh the PR summary and test comments to describe clearing
WindowMinimizedand the two Nets callers; references to anisMinimized()guard are stale. - Add braces to the new helper's null guard and the test's toggle branches to match AGENTS.md's control-flow convention (inline suggestion).
Independent verification
Built both the unchanged merge base 7f3eb98cd2df426dc4381e2a2203a1d59f600d1a and exact PR head 5a48797deeeef0057e761a68c652111215cd5ac7 as Mac applications. Both used RelWithDebInfo, Qt 6.11.1, and ENABLE_ASR=OFF; optional ASR is unrelated to this window path. The sandbox's icon-generation step failed identically on both; rerunning the builds outside the sandbox succeeded without source changes.
The full application checks used separate settings stores, explicit per-instance automation sockets, offscreen Qt, and only DEMO-0001 (AetherSDR Demo), with transmission pinned off. The actual AetherVoice launcher was invoked through VfoWidget/Aetherial Audio Channel Strip; restoration was not forced through the bridge's restore verb.
Recorded strip state:
| Sequence | Pre-fix application | PR application |
|---|---|---|
| Open, minimize, press AetherVoice once | visible:false, windowState:"minimized" |
visible:true, windowState:"normal" |
| Press AetherVoice again on pre-fix build | visible:true, windowState:"minimized" — still inaccessible |
Normal hide/reopen behavior verified separately |
| Maximize, minimize, press AetherVoice | — | visible:true, windowState:"maximized" |
| Fullscreen, minimize, press AetherVoice | — | visible:true, windowState:"fullscreen" |
The PR application also passed ordinary hide/show toggles, hide/show while maximized, and close/reopen while maximized. Both owned demo instances exited normally after validation; final demo model state reported transmitting:false and txPower:0.
The registered window_show_state_test passed through CTest. Its 15 assertions also passed independently on native Cocoa with no skips. Three deliberate mutations were caught: removing the minimized predicate (4 failures), omitting the minimized-state clear (3 failures), and using unconditional showNormal() (3 failures). An additional socket-free driver passed 11 assertions covering hidden/minimized restoration, fullscreen preservation, repeated geometry-preserving toggles, and a parent-owned top-level window matching the strip's ownership.
Inspected callers, ownership, surrounding code, and intervening main changes; no stale-base overwrite or overlapping fix found. Local test-registration and engine-boundary checks passed (existing tracked boundary warnings only). No separately invocable automated code-review skill was available; a second code audit was performed and its conclusions checked against the patch and runtime evidence.
Review snapshot and limits
Trusted governance snapshot: 50d70ebe9fdc4d6f01171b5cb17479bc89537be2, manifest PASS; canonical constitution and root mirror match. Both PR commits have valid GitHub-verified signatures. Approval remains subject to the repository's human CODEOWNERS requirements: infrastructure for CMakeLists.txt, reviewers for source/tests.
This review demonstrates the actual strip path on Mac plus native Cocoa helper behavior. Native Dock stacking, actual OS tray notifications, and physical-radio operation were not exercised. The full local suite was not run; the author's historical full-suite failure claims were not independently repeated.
Verdict
Approve with the non-blocking notes above. CI is green for Linux, macOS, and Windows, and Static Checks is green. Inspected the completed job logs; the platform test steps ran and passed. The head and existing discussion were rechecked immediately before submission. No merge action is included in this review.
| if (!w) | ||
| return; |
There was a problem hiding this comment.
Non-blocking convention nit: AGENTS.md requires braces on all control flow. Please brace this guard (and the toggle branches in the new test).
| if (!w) | |
| return; | |
| if (!w) { | |
| return; | |
| } |
Summary
Fixes #5365.
Minimizing the Aetherial Audio Channel Strip made it unrecoverable from its own
button: no number of presses on the AetherVoice button (or the chain applet's
nub) would bring the window back, leaving the Dock/taskbar as the only way to
reach it.
Two Qt behaviours combine to cause it, and
toggleAetherialStrip()was writtenas if neither existed:
QWidget::isVisible()stays true while a window is minimized, soif (m_aetherialStrip->isVisible())sent a minimized strip down thehide()branch.
QWidget::show()on a minimized window restores its saved state — stillminimized — so the follow-up press did not recover it either.
The decision is extracted into
gui/WindowShowState.{h,cpp}:windowIsShowing(w)— visible and not minimized.showAndRaiseWindow(w)—showNormal()when minimized,show()otherwise,then raise + activate.
showNormal()stays guarded onisMinimized()because calling itunconditionally would clear a Maximized or FullScreen window — the same reason
the net-reminder and tray raise paths in
MainWindow_Nets.cppguard it (#3918).That helper is where those two sites should eventually converge; they are
deliberately not touched here, to keep this change scoped to the reported
bug.
The docked CWX and DVK panels use the same bare
isVisible()shape but arechild widgets that cannot be minimized independently, so they are unaffected and
are left alone.
Constitution principle honored
Principle XI — Fixes Are Demonstrated. New regression test
window_show_state_testdrives a realQWidgetthroughhidden → shown → minimized → restored and runs the toggle exactly as
MainWindowdoes. Mutation-checked three ways, each failing a differentassertion:
windowIsShowing()without the!isMinimized()term → 3 failuresshowAndRaiseWindow()with an unconditionalshow()→ 2 failuresshowAndRaiseWindow()with an unconditionalshowNormal()→ 1 failure(the Scheduled Event Doesn't Show Correct Slice Matching Tuned Frequency #3918 maximized guard)
Principle VIII — Evidence Over Assertion. Both Qt behaviours above are
pinned against a real
QWidgeton the offscreen platform, not asserted frommemory.
Test plan
cmake --build build) — clean, exit 0management in
MainWindow, independent of the connected backendwindow_show_state_testpasses; full localsuite run on this build tree shows only the two failures that reproduce
unchanged on clean
main(bridge_docs_check,hl2_state_restore_test),neither of which touches GUI window state
Checklist
docs/COMMIT-SIGNING.md) — GPG, GitHub reportsverified: trueAppSettingscalls — this change adds no settings(Principle IV)
MeterSmoother— N/A, no meter UI is touchedneeded; no document describes the old toggle behaviour.
CHANGELOG.mddeliberately untouched
Note on the claim protocol (AGENTS.md §Issue / PR Claim Protocol): assignee
changes are rejected for an account without write access to this repo, so the
Fixes #5365link is the visible claim on the issue timeline instead.