fix(gui): collapse ESC panel space immediately when DIV toggles off (#3383). Principle VIII.#3393
Conversation
There was a problem hiding this comment.
Thanks @mvanhorn — clean, well-scoped fix.
LGTM on correctness. The diagnosis is right: setVisible() only posts a QEvent::LayoutRequest to the event queue, so a synchronous sizeHint() call right after still reflects the pre-toggle layout. QWidget::adjustSize() calls QLayout::activate() on the owning layout before reading the hint, which is the textbook Qt fix here. All three diversity call sites in src/gui/VfoWidget.cpp are addressed (DIV button toggled, setDiversityAllowed(false), SliceModel::diversityChanged), and no other behavior is touched.
A couple of small observations, not blockers:
- The 4th
resize(sizeHint())atsrc/gui/VfoWidget.cpp:4197(RADE info-row toggle) has the same shape —setVisible(on)onm_radeInfoRowfollowed immediately byresize(sizeHint()). The PR description is explicit about scoping this out, which is reasonable; just flagging it in case it's worth a separate ticket so the pattern doesn't drift back in via cargo-culting. - Scope/AppSettings/RAII: nothing to call out — no settings touched, no new allocations, no meter changes.
CI looks like the right place to verify since the change is a two-method swap and there's no FlexLib/protocol surface area. Nice tight fix.
🤖 aethersdr-agent · cost: $10.0959 · model: claude-opus-4-7
jensenpat
left a comment
There was a problem hiding this comment.
Reviewed: correct, idiomatic Qt fix (adjustSize flushes the posted LayoutRequest before sizing, unlike resize(sizeHint())). All three diversity call sites verified; RADE site correctly left untouched. CI green across platforms, commit signed & verified.
|
Appreciate the fast merge @jensenpat! Glad the ESC panel space collapses immediately now. |
|
Thanks for merging, @jensenpat. The ESC panel collapsing immediately on DIV toggle makes the GUI feel a lot tighter. |
…ethersdr#3383). Principle VIII. (aethersdr#3393) ## Summary Fixes aethersdr#3383. Toggling DIV off now collapses the ESC panel space immediately. The three sites that toggle `m_escPanel` visibility called `resize(sizeHint())` right after `setVisible()`, but `setVisible()` only posts a `LayoutRequest` to the audio tab's layout - it does not activate it synchronously - so `sizeHint()` still returned the stale pre-hide height and the panel kept a blank gap until something else (like clicking the speaker sub-tab) forced a reflow. `adjustSize()` activates the pending layout before computing the new size, which is the missing step. All three diversity-path call sites in `src/gui/VfoWidget.cpp` are swapped: the DIV button `toggled` handler, `setDiversityAllowed(false)` (non-dual-SCU reconnect path), and the `SliceModel::diversityChanged` model-sync handler. The DIV-on path benefits equally: the panel grows in one step instead of relying on a previously-posted layout pass. ## Context aethersdr#3383 reports the gap between the Med (AGC speed) row and the bottom of the panel after DIV is toggled off, with the layout only correcting after a tab switch. The @aethersdr-agent triage on the issue traced it to exactly these three call sites, and this PR follows that triage. The fourth `resize(sizeHint())` in the RADE info-row toggle is intentionally left as-is to keep this scoped to the diversity paths. ## Constitution principle honored Principle VIII - Evidence Over Assertion. Root cause and call sites come from the @aethersdr-agent triage on aethersdr#3383 plus Qt's documented `LayoutRequest`/`activate()` ordering (`QWidget::adjustSize()` activates the layout before reading the size hint). The diff is confined to those three sites. ## Test plan - [x] Reproduction steps documented if user-reported bug (repro and expected behavior from aethersdr#3383: DIV off collapses the AF/SQL/Med rows immediately; DIV on restores the full ESC panel height; rapid toggling settles correctly since every toggle now activates the layout before sizing) - [x] Existing tests pass (CI) - no test code touched; the CI build matrix exercises compilation of the two-method swap Not run against a real radio: no FlexLib or protocol surface is touched, and this machine has no Qt6 toolchain for a local GUI build, so compilation is delegated to CI. ## Checklist - [x] Commits are signed (`docs/COMMIT-SIGNING.md`) - [x] No new flat-key `AppSettings` calls - none added (Principle V) - [x] All meter UI uses `MeterSmoother` - no meter changes (Principle II) - [x] Documentation updated if user-visible behavior changed - the visible change is the bug fix itself; no docs reference the stale-gap behavior - [x] Security-sensitive changes reference a GHSA if applicable - not security-sensitive Co-authored-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
Summary
Fixes #3383. Toggling DIV off now collapses the ESC panel space immediately. The three sites that toggle
m_escPanelvisibility calledresize(sizeHint())right aftersetVisible(), butsetVisible()only posts aLayoutRequestto the audio tab's layout - it does not activate it synchronously - sosizeHint()still returned the stale pre-hide height and the panel kept a blank gap until something else (like clicking the speaker sub-tab) forced a reflow.adjustSize()activates the pending layout before computing the new size, which is the missing step. All three diversity-path call sites insrc/gui/VfoWidget.cppare swapped: the DIV buttontoggledhandler,setDiversityAllowed(false)(non-dual-SCU reconnect path), and theSliceModel::diversityChangedmodel-sync handler. The DIV-on path benefits equally: the panel grows in one step instead of relying on a previously-posted layout pass.Context
#3383 reports the gap between the Med (AGC speed) row and the bottom of the panel after DIV is toggled off, with the layout only correcting after a tab switch. The @aethersdr-agent triage on the issue traced it to exactly these three call sites, and this PR follows that triage. The fourth
resize(sizeHint())in the RADE info-row toggle is intentionally left as-is to keep this scoped to the diversity paths.Constitution principle honored
Principle VIII - Evidence Over Assertion. Root cause and call sites come from the @aethersdr-agent triage on #3383 plus Qt's documented
LayoutRequest/activate()ordering (QWidget::adjustSize()activates the layout before reading the size hint). The diff is confined to those three sites.Test plan
Not run against a real radio: no FlexLib or protocol surface is touched, and this machine has no Qt6 toolchain for a local GUI build, so compilation is delegated to CI.
Checklist
docs/COMMIT-SIGNING.md)AppSettingscalls - none added (Principle V)MeterSmoother- no meter changes (Principle II)