Skip to content

fix(gui): collapse ESC panel space immediately when DIV toggles off (#3383). Principle VIII.#3393

Merged
jensenpat merged 1 commit into
aethersdr:mainfrom
mvanhorn:fix/3383-div-row-reflow
Jun 5, 2026
Merged

fix(gui): collapse ESC panel space immediately when DIV toggles off (#3383). Principle VIII.#3393
jensenpat merged 1 commit into
aethersdr:mainfrom
mvanhorn:fix/3383-div-row-reflow

Conversation

@mvanhorn

@mvanhorn mvanhorn commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #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

#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

  • Commits are signed (docs/COMMIT-SIGNING.md)
  • No new flat-key AppSettings calls - none added (Principle V)
  • All meter UI uses MeterSmoother - no meter changes (Principle II)
  • Documentation updated if user-visible behavior changed - the visible change is the bug fix itself; no docs reference the stale-gap behavior
  • Security-sensitive changes reference a GHSA if applicable - not security-sensitive

@mvanhorn mvanhorn requested a review from a team as a code owner June 4, 2026 15:56

@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.

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:

  1. The 4th resize(sizeHint()) at src/gui/VfoWidget.cpp:4197 (RADE info-row toggle) has the same shape — setVisible(on) on m_radeInfoRow followed immediately by resize(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.
  2. 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 jensenpat 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: 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.

@jensenpat jensenpat merged commit dcb58b0 into aethersdr:main Jun 5, 2026
5 checks passed
@mvanhorn

mvanhorn commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Appreciate the fast merge @jensenpat! Glad the ESC panel space collapses immediately now.

@mvanhorn

Copy link
Copy Markdown
Contributor Author

Thanks for merging, @jensenpat. The ESC panel collapsing immediately on DIV toggle makes the GUI feel a lot tighter.

G6PWY-Chris pushed a commit to G6PWY-Chris/AetherSDR that referenced this pull request Jun 22, 2026
…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>
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.

Bug: DIV slider row leaves empty space when DIV is toggled off; requires speaker-tab click to reflow

2 participants