Skip to content

fix(gui): inset the status bar container so its click targets clear the frameless bottom resize band (#4886). Principle VIII. - #6018

Closed
aethersdr-agent[bot] wants to merge 1 commit into
mainfrom
aetherclaude/issue-4886
Closed

aethersdr-agent[bot] wants to merge 1 commit into
mainfrom
aetherclaude/issue-4886

Conversation

@aethersdr-agent

Copy link
Copy Markdown
Contributor

Summary

Fixes #4886

What was changed

fix(gui): inset the status bar container so its click targets clear the frameless bottom resize band (#4886). Principle VIII.

Files modified

  • src/gui/FramelessResizer.h
  • src/gui/MainWindow.cpp
 src/gui/FramelessResizer.h | 40 +++++++++++++++++++++++-----------------
 src/gui/MainWindow.cpp     | 20 ++++++++++++++++++--
 2 files changed, 41 insertions(+), 19 deletions(-)

Generated by AetherClaude (automated agent for AetherSDR)


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

…he frameless bottom resize band (#4886). Principle VIII.

FramelessResizer claims a 6px band at every edge of a frameless window and
its filter lives on the QWindow, so Qt runs it ahead of the receiver's own
event() — a press in the band is consumed as a resize before any widget
sees it. MainWindow's status bar is docked flush to the window's bottom
edge and m_statusBarContainer's QHBoxLayout had no bottom margin, so every
child with a vertically-Preferred/Minimum policy was stretched into that
band: the gpsStack button and all ~20 plain-QLabel click targets
(m_stationNickLabel, m_bandStackIndicator, addPanBtn, the TNF/CWX/ASR/DVK/
FDX indicators, m_paTempLabel, m_networkLabel, the TGXL/PGXL containers),
plus m_txIndicator's fixed 36x36 rect by ~1px.

Fixed with one bottom inset at the container rather than per-control
maximumHeight() caps: one inset covers every current child and every one
added later, whereas ~20 individual caps would drift as controls are
added. The inset is FramelessResizer::kDefaultMargin, a new named constant
replacing the bare `margin = 6` default, so the band width and the inset
that has to clear it cannot silently diverge (issue follow-on note 1).
MainWindow's own install() call uses the constant too.

Evidence (Principle VIII): measured offscreen under Qt 6.11 with a
throwaway probe replicating the status bar (fixed height 46, border-top
1px, the same stylesheet, a 36x36 fixed child and a two-row telemetry
stack). Before: nick label and GPS button bottoms sit 2px from the window
edge — 4px inside the band, which starts 6px up — and m_txIndicator 5px,
matching the ~4px/~1px figures from the issue's hitTest probe. After:
worst-case clearance is 8px, m_txIndicator 7px, so nothing lands in the
band, and the 36px indicator is repositioned rather than clipped (still
h=36). Cost is the AlignVCenter content shifting up ~3px uniformly across
the bar; worth an eyeball on a running instance before merge, which is the
one thing the offscreen probe cannot judge.

m_sizeGrip (16x16 at height()-17) sits entirely inside the corner band and
so has had dead drag handling for as long as the resizer has been
installed. Left alone — the resizer performs the same resize, making the
grip a visual affordance.

Also corrects the FramelessResizer header's bottom-edge note, which
documented #4886 as deferred.

No new external symbol references and no cross-TU motion, so no test
target link surface changed; MainWindow.cpp is listed only in GUI_SOURCES
and frameless_resizer_test's source list is untouched. FramelessResizer.cpp
and the new constant's use sites were clang -fsyntax-only clean (no cmake
in this environment for a full target build).

Blast radius: risk_score=0.237, 12 high-risk affected (top:
MainWindow::MainWindow, Ax25HfPacketDecodeDialog::Ax25HfPacketDecodeDialog,
MainWindow::wirePanadapter). All 91 upstream callers reach this header
through FramelessResizer::install; the change to it is value-identical
(kDefaultMargin == the former literal 6 default), so no adopter's band
width changes. The three top callers were read to confirm it: MainWindow
passes the margin explicitly, PersistentDialog (which Ax25HfPacketDecodeDialog
derives from) and the rest take the default. The MainWindow.cpp edit is a
layout margin inside buildUI() with no signature or symbol change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown

Accessibility check findings

This PR touches src/gui/ files with 4 accessibility issue(s).
Full details appear as inline annotations on the diff above.

Files scanned : 2
Files with findings: 1
Total findings : 4

See docs/a11y.md for the canonical
Qt patterns (setAccessibleName, QAccessibleValueChangeEvent,
QAccessibleInterface subclasses, etc.). Structural remediation
(keyboard navigation, custom QAccessibleInterface subclasses)
is tracked in #3288.

Findings are warnings only — the build is not blocked.

@aethersdr-agent aethersdr-agent Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Issue fit

Yes, for the reported symptom. #4886 asks that status-bar click targets stop short of the frameless bottom resize band, and explicitly says the fix belongs in the status-bar layout rather than in FramelessResizer. The diff does that, and it takes the better of the two shapes the issue thread proposed: one bottom inset on m_statusBarContainer's layout instead of ~20 per-control maximumHeight() caps. Both of the issue's follow-on notes are handled — note 1 (derive the inset from the resizer's margin, don't hard-code 6) becomes FramelessResizer::kDefaultMargin, and note 2 (m_sizeGrip) is deliberately left alone. The header's stale "deferred, tracked as #4886" paragraph is corrected in the same commit, which is the right place for it.

I confirmed the mechanism rather than taking the body's word for it: FramelessResizer installs its filter on QCoreApplication (FramelessResizer.cpp:41-43), and computeEdges() claims p.y() >= rect.height() - margin (FramelessResizer.cpp:109), so the precedence over the widget tree is structural, as claimed. m_statusBarContainer is the only widget added to the status bar (MainWindow.cpp:6307; no addPermanentWidget anywhere), so one inset really does cover every click target listed in the issue.

Everything below is reasoned from the code in /tmp/aetherclaude/pr-6018. I have no build, no GUI and no way to re-run the author's offscreen probe, so the pixel measurements in the commit message are the one class of claim I could not independently reproduce — I checked them for internal consistency instead.

Scope

File / group What it changes Claimed by title/body? Verdict
src/gui/MainWindow.cpp:1201 install(this, 6, …) → install(this, FramelessResizer::kDefaultMargin, …) yes (issue follow-on note 1) In scope — value-identical, no behavior change
src/gui/MainWindow.cpp:5738-5753 the bottom inset + its rationale comment yes, this is the fix In scope
src/gui/FramelessResizer.h:136-141 new kDefaultMargin, install() default now names it yes In scope
src/gui/FramelessResizer.h:101-119 rewrites the bottom-edge note from "deferred" to "fixed" yes In scope — a - block that names a symptom, but it is a deferral note being retired by the fix it deferred, not a guard removal
src/gui/FramelessResizer.h:132 usage example install(win, 6) → install(win, 8) no Cosmetic, explained by context (a 6 example now reads as the default and no longer illustrates "explicit margin"). Named here, not a blocker

No new public surface beyond kDefaultMargin, which is a named constant for a value that was already the compiled-in default — no adopter's band width changes. CHANGELOG.md correctly untouched. No settings, no capability, no protocol surface. No sibling implementations left behind: the twelve other FramelessResizer::install call sites are dialogs/floating windows, none of which the issue reports a flush-docked click target on.

Blockers

None.

Nits (non-blocking)

  • FramelessResizer.h:138 — the inset rule in the new comment is off by one for the left and top edges. computeEdges() uses p.x() <= margin and p.y() <= margin for left/top but p.y() >= rect.height() - margin for right/bottom (FramelessResizer.cpp:105-110), so the left/top bands are margin + 1 px wide (columns 0..margin) while right/bottom are margin px. "Must inset it by at least this much" is exact for the edge this PR fixes and one pixel short for the other two. Inline comment has a suggested wording. MainWindow's own left/right margins of 6 are unchanged by this PR, so this is a note for the next adopter who follows the comment, not a regression here.
  • The inset comes out of content height, and the diff's own arithmetic says the tallest child doesn't fit in what's left. 34px of content vs m_txIndicator's fixed 36×36 (MainWindow.cpp:6271) — the comment acknowledges this with "positioned, not clipped", and the commit message measures its clearance at 7px against a 6px band. That is 1px of slack for the one control the issue measured as the worst case, and the same halving would apply to the two-row 12px-font telemetry stacks (statusBarCompactLabelStyle, MainWindow.cpp:314-317) on any platform whose font metrics render them above 34px. It works as measured; it is the part most likely to be re-broken by an unrelated change to setFixedHeight(46), the 1px border-top, or the label font size. Worth a sentence in the comment saying the slack is thin and why, so the next person editing the bar height knows it is load-bearing.
  • The commit message's m_sizeGrip claim is inherited from the issue comment and is not quite right. 16×16 at (width()-17, height()-17) (MainWindow.cpp:3976) puts only its bottom and right 6px strips inside the band — its top-left ~11×11 is still live, so its drag handling has not been wholly dead. The decision (leave it alone; the resizer performs the same resize) is right either way, so nothing to change in the code.
  • No test, and I am not asking for one. tests/tests.cmake:5724-5725 states outright that "MainWindow is not linked into test targets", so there is no seam that asserts real m_statusBarContainer child rects against the real window rect. A probe replicating the layout — which is what the author built and discarded — would assert the fix back to itself. frameless_resizer_test already covers computeEdges() as pure arithmetic and correctly needed no change. Honest coverage boundary: the geometry half of this fix is verifiable only on a running instance.

Socket tests

None added, modified or removed. No QTcpServer/QUdpSocket/QLocalServer/bind/listen/connectToHost and no Fake* peer anywhere in the diff; no test sources are touched at all.

Other tools' findings

CodeGuard's five CG-PATH-001 hits are all false positives for this PR and I am not carrying them forward: MainWindow.cpp:9556 and :9668 are QLabel user-facing strings about FFTW wisdom generation, and :9787-9789 walk up from QCoreApplication::applicationFilePath() to the macOS bundle root with no external input. All five are pre-existing lines far from this diff (which touches :1201 and :5738-5753). No Copilot comments were present.

What I tried to break

  • "One inset covers every child." Grepped for other widgets in the status bar — addWidget(m_statusBarContainer, 1) at :6307 is the only one, no addPermanentWidget, so nothing clickable escapes the container. m_sizeGrip is a direct MainWindow child outside the container and is the one deliberate exception.
  • "Headroom: nothing gets squeezed." Grepped the whole status-bar construction region (:5736-6310) for setFixedSize/setFixedHeight/setMinimumHeight: m_txIndicator is the only fixed-height child, so the comment's "tallest content is the two-row telemetry stacks" is right about the flexible children. The fixed one is the nit above.
  • "Value-identical, no adopter's band changes." Read all twelve install() call sites: MainWindow and AetherialAudioStrip pass a margin explicitly (both 6), the other ten take the default. kDefaultMargin == 6 == the former literal default, so no adopter's geometry moves.
  • Deleted lines. Read the - block in the header as carefully as the + block: it is a deferral note and a measurement correction, not a guard, and the symptom it names is the one this PR fixes.
  • Off-by-one in the band itself. Walked computeEdges() per edge. Bottom clearance of 8px vs rows h-6..h-1 is genuinely clear; the asymmetry only bites the left/top edges, which is the first nit.
  • Could not check: the offscreen 2px→8px and 5px→7px measurements, and the ~3px upward shift of the AlignVCenter content across the whole bar. That shift is the one user-visible consequence and the commit says so honestly — it needs an eyeball on a running instance.

Recommendation

Approve with nits. The fix is the right shape at the right seam, it addresses both of the issue's follow-on notes, and the scope is exactly two files that the stated fix explains. Nothing I found rises above a nit: the left/top wording in the new comment is worth a one-line correction while the file is open, and the 1px of slack on m_txIndicator is worth documenting so a future bar-height change doesn't silently re-break it. Concrete next step for the maintainer: give the status bar an eyeball on a running frameless instance to confirm the uniform ~3px upward shift reads as intentional — that is the one claim no reviewer here can settle from the code.


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

public:
// Width (px) of the edge-resize band every adopter gets unless it passes
// its own `margin`. Named rather than a bare literal because adopters that
// dock a control flush to an edge must inset it by at least this much to

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This rule is exact for the bottom and right edges and one pixel short for the left and top. computeEdges() claims p.x() <= margin and p.y() <= margin but p.x() >= rect.width() - margin / p.y() >= rect.height() - margin (FramelessResizer.cpp:105-110), so the left/top bands are margin + 1 px wide — columns 0..margin inclusive — while right/bottom are margin px. An adopter that follows this comment literally and insets a left-docked control by exactly kDefaultMargin leaves its first pixel column still claimed by the resizer.

Doesn't affect this PR's fix (bottom edge, and the measured clearance is 8px), but the comment is the thing the next adopter will read.

Suggested change
// dock a control flush to an edge must inset it by at least this much to
// dock a control flush to an edge must inset it by at least this much to
// stay out of the band — one more than this on the left and top edges,
// whose bands are `margin + 1` px wide because computeEdges() tests them
// with `<=` (see FramelessResizer.cpp). See MainWindow's status bar,
// #4886 — a hard-coded 6 on both sides could silently diverge from this
// default.

Comment thread src/gui/MainWindow.cpp
// ~5px status-bar margins − 6px inset leaves 34px of content, and the
// tallest content is the two-row telemetry stacks at ~30px. Measured
// offscreen (Qt 6.11): worst-case clearance from the window edge goes 2px →
// 8px, and the fixed 36x36 m_txIndicator is positioned, not clipped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The headroom paragraph is doing something slightly awkward: it computes 34px of content and then immediately names a 36×36 child that doesn't fit in it. The commit message resolves this (measured clearance 7px against a 6px band), but that is 1px of slack for m_txIndicator — which is a real click target (installEventFilter(this) at :6277) and the control the issue measured as the narrowest miss at ~1px.

The mechanism generalises: any child whose natural height exceeds the post-inset content rect gets back roughly half its overflow in lost clearance, so the same applies to the two-row 12px-font stacks if font metrics on some platform/DPI push them past 34px.

Not asking for a code change — the measurement says it clears. Worth one more sentence here saying the slack is thin, so whoever next edits setFixedHeight(46), the 1px border-top, or statusBarCompactLabelStyle's font size knows those numbers are load-bearing for #4886 and not just cosmetic.

@aethersdr-agent

Copy link
Copy Markdown
Contributor Author

Cisco CodeGuard — static analysis of this PR (5 finding(s))

  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6018/src/gui/MainWindow.cpp:9556
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6018/src/gui/MainWindow.cpp:9668
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6018/src/gui/MainWindow.cpp:9787
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6018/src/gui/MainWindow.cpp:9788
  • [MEDIUM] CG-PATH-001 — Potential path traversal in src/gui/MainWindow.cpp /tmp/aetherclaude/pr-6018/src/gui/MainWindow.cpp:9789

Automated static scan by Cisco DefenseClaw CodeGuard on the changed files. Advisory — some may be false positives; the review above verifies them.


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

@ten9876 ten9876 self-assigned this Oct 4, 2026

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

Issue fit: Yes, in the exercised layout. #4886 reports status-bar controls extending into the frameless bottom resize band. The container inset fixes that overlap in the actual application, and the shared constant keeps its width synchronized with the resizer. The issue's separate size-grip behavior remains outside this change.

File/group Change Claimed? Scope verdict
src/gui/MainWindow.cpp:1201 Use the named 6px resize margin Yes, issue follow-on requirement In scope; value unchanged
src/gui/MainWindow.cpp:5738–5753 Add bottom inset to the single status-bar container Yes The requested fix; covers its child layout
src/gui/FramelessResizer.h:103–117 Update the former bottom-edge deferral explanation Related explanation In scope, but too much history remains
src/gui/FramelessResizer.h:132–149 Expose kDefaultMargin, use it as the default, change explicit-margin example to 8 Constant is required; example cosmetic In scope; no adopter's default changes

One commit, authored September 29; both changed files are disclosed. No settings, radio commands, new capability, test registration, guard removal, or changelog changes. The deleted header text describes a deferred symptom rather than removing its protection. statusBar()->addWidget(m_statusBarContainer, 1) is the sole status-bar insertion; the existing independent size grip is deliberately separate.

Blockers: none.

Non-blocking nits

  1. src/gui/MainWindow.cpp:5738–5752 and src/gui/FramelessResizer.h:103–117: shorten the new incident narrative and measurements to the current invariant. Current AGENTS.md asks for concise comments about what is true now; history and review evidence belong in the commit/PR. The MainWindow comment also says the filter is “on the window,” whereas FramelessResizer.cpp:41–43 installs it on the application. A suitable replacement above the layout call is:
        // Keep status-bar click targets above the bottom resize band (#4886).
        // Share the inset with FramelessResizer so the two widths stay aligned.
        // The fixed 36px TX indicator must also remain clear of that band.
  2. src/gui/FramelessResizer.h:138–141: “inset it by at least this much” is not universally sufficient for every edge. computeEdges() at FramelessResizer.cpp:106–109 includes coordinates 0..margin on left/top, so those bands occupy margin + 1 pixels; bottom/right occupy margin pixels. Qualify this guidance as applying to bottom/right. This PR's bottom inset is correct; it does not introduce the existing asymmetry.
  3. src/gui/MainWindow.cpp:5738–5753: platform/font headroom remains a verification limit. The fixed TX label clears the band by only one additional logical pixel on the tested configuration. No clipping occurred, but the full range of native styles, DPI configurations, optional peripheral stacks and macOS/Windows layouts was not exercised. No synthetic layout-copy test is requested: the actual running widget tree is the relevant seam.

Empirical verification and attempted falsification

Built exact head af8272071ac6a330a7df70f96a2a3fdfa19f94cd in an isolated scratch worktree (Release, GCC 16.2.1, Qt 6.11.2, CMake 4.4.3; existing CMake build, cmake --build ... --target AetherSDR -j4). Ran the actual application with QT_QPA_PLATFORM=offscreen, empty QT_QPA_PLATFORMTHEME, Fusion style, fresh AETHER_SETTINGS_DIR/XDG directories, explicit private bridge socket, and AETHER_AUTOMATION_NO_TX=1. whoami returned the owned PID/identity and "txAllowed": false; only DEMO-0001 was connected, with "model": "AetherSDR Demo" and "transmitting": false.

Used connect list, connect local serial DEMO-0001, connect wait, get radio, dumpTree, hitTest, and grab QStatusBar. Then rebuilt a negative control changing only the new bottom inset from 6 to 0. This is a behavioral control on the same head, not a separately built full merge-base checkout. It restores the entire behavioral change while keeping the application, settings, style and measurement procedure the same.

For the actual 1400×800 window and 46px bar:

Measured control Zero-inset control PR head
Add Panadapter, TNF, ASR, station label and other visible flexible click targets 2px bottom clearance; 4px inside resize band 8px clearance; no overlap
Fixed 36×36 Cancel transmit label 5px clearance; 1px inside resize band 7px clearance; no overlap; still 36px high
hitTest MainWindow 235 796 QLabel, objectName: addPanadapterButton Inner QWidget, no control at this band point

This reproduces the original overlap and removes it with the fix. Inspected both actual status-bar PNGs: the content moves upward as expected, and the fixed indicator remains intact. The active station text and two-row telemetry remain readable. Geometry/hit-test evidence establishes the overlap; this was not a native compositor drag test. Hidden GPS/peripheral controls were not individually enabled, so their future layout behavior is code-reviewed, not claimed as runtime coverage. Both owned instances were closed; the scratch source was restored to exact head. No operator process was driven and no transmit control was invoked.

No tests changed, and the exact diff introduces no socket-owning test or synthetic firmware peer. Existing pure resizer arithmetic was inspected but not presented as coverage of this layout change. The seven displayed GitHub checks are successful, including all four required checks, but the PR-triggered CI/Static Checks runs were created September 29 (2026-09-29T14:35:17Z), before current main's October 4 tip. That green is stale merge evidence, not a fresh full-suite result. The local application build passed; no broad suite was run.

Previous bot concerns were checked independently: the left/top wording and thin TX slack are real nits; the size grip is not entirely inside the corner band (its top-left portion lies outside), but this PR does not alter it. CodeGuard's cited path-handling locations and the accessibility warnings concern pre-existing code outside these hunks, not new behavior from the inset.

Recommendation: Approve with nits. The actual application plus zero-inset control demonstrates the load-bearing fix. Shorten the comments, qualify the general inset guidance, and obtain current-base CI and the maintainer's usual platform/layout check before landing.

— Jeremy KK7GWY + Codex

@ten9876 ten9876 removed their assignment Oct 4, 2026

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

Issue fit: yes. #4886 asks that status-bar click targets stop short of the frameless bottom resize band, and that the fix live in the status-bar layout. One bottom inset on the single status-bar container does that. I re-verified it on current main (below), not only on the September base.

What changed since the earlier reviews: the PR now conflicts with main (mergeable: CONFLICTING), and that is the one blocker. Squash auto-merge is still armed by aethersdr-agent (since 2026-09-29), so a rebase plus an approval merges it immediately.

Scope

File/group Change Claimed? Verdict
src/gui/MainWindow.cpp:1201 install(this, 6, …) → FramelessResizer::kDefaultMargin Yes (issue note 1) In scope, value unchanged
src/gui/MainWindow.cpp:5738-5753 Bottom inset on m_statusBarContainer's layout, plus a 15-line comment Yes In scope; the comment is nit 2
src/gui/FramelessResizer.h:136-141 kDefaultMargin constant, used as install()'s default Yes In scope
src/gui/FramelessResizer.h:101-132 Rewrites the "deferred to #4886" note; usage example 6 → 8 Implicit Main has since deleted this block (see blocker 1)

One commit, authored 2026-09-29. There are no test changes, settings, protocol or capability surface, or CHANGELOG entry. Socket preflight: no test sources touched.

Blockers

  1. It conflicts with main in both files. git merge-tree origin/main af827207 reports content conflicts in src/gui/FramelessResizer.h and src/gui/MainWindow.cpp. #6072 (comment-policy pass) condensed FramelessResizer.h, deleting the whole bottom-edge paragraph this PR rewrites, and moved and edited the MainWindow.cpp regions.

    When rebasing, don't re-introduce the deleted header paragraph: on main it no longer exists, and that was deliberate. The whole fix rebased onto main 85f2a1249 is three lines. I applied exactly this, built it, and drove it:

    // src/gui/FramelessResizer.h, replacing the install() declaration
        static constexpr int kDefaultMargin = 6;
        static void install(QWidget* window, int margin = kDefaultMargin, int topMoveReserve = 0);
    // src/gui/MainWindow.cpp (main:1170)
            FramelessResizer::install(this, FramelessResizer::kDefaultMargin, TitleBar::kHeight);
    // src/gui/MainWindow.cpp (main:5626)
        // Keep status-bar click targets above FramelessResizer's bottom resize band (#4886).
        hbox->setContentsMargins(6, 0, 6, FramelessResizer::kDefaultMargin);

Nits (non-blocking)

  1. src/gui/AetherialAudioStrip.cpp:127 still passes a literal 6 (install(this, 6, kTitleBarHeight)). The PR's own rationale for kDefaultMargin, that a hard-coded 6 "could silently diverge from this default", applies to it too. It's a one-token change.
  2. MainWindow.cpp:5738-5752: the 15-line comment narrates the measurement history. AGENTS.md §"Comments" asks for the current invariant only, and the earlier review's 3-line replacement still fits. The one-line version in the blocker fix above is enough.
  3. FramelessResizer.h:138: "inset it by at least this much" is exact for bottom/right but one pixel short for left/top. computeEdges() uses <= margin there (FramelessResizer.cpp). This is carried over from the earlier reviews and is still true on main.
  4. Two aethersdr-agent inline threads (FramelessResizer.h:138, MainWindow.cpp:5752) are unresolved. main requires conversation resolution, so they block the merge independently of this review.

Verified empirically, and what I tried to break

  • Re-measured on current main, not the September base. main has taken 20 commits through MainWindow.cpp since this branch, including new peripheral and status entries, so the earlier measurements described a status bar that no longer exists.

    • Setup: I built main 85f2a1249 plus the 3-line rebase above (Release), and drove it offscreen with isolated settings, the explicit socket aethersdr-pr6018 and AETHER_AUTOMATION_NO_TX=1.
    • whoami returned "txAllowed": false. I connected explicitly to DEMO-0001, and get radio returned "model": "AetherSDR Demo".
    • From dumpTree geometry on the 1400×800 window (status bar at y 754, height 46), I computed every visible status-bar descendant's clearance from the window's bottom edge:
    zero-inset control (main as-is) with the fix
    Flexible click targets (addPanadapterButton, TNF, ASR, station label, automationChip, separators) 2 px (4 px inside the 6 px band), height 41 8 px, height 35
    Fixed 36×36 "Cancel transmit" 5 px (1 px inside the band) 7 px, still 36 high
    hitTest MainWindow 235 796 QLabel addPanadapterButton container QWidget, no control

    The bug reproduces on today's main, and the fix clears it with 1–2 px of margin over the band. The zero-inset control is the same tree with only the inset reverted, rebuilt.

  • "One inset covers every child." On main, m_statusBarContainer is still the status bar's only widget, so all 28 visible descendants I enumerated sit inside it.

  • Not tested: a native compositor drag (offscreen geometry only), and hidden GPS or peripheral entries, which are reviewed from code.

  • CI is from 2026-09-29, before main's 2026-10-04 tip, and it describes a merge that no longer applies.

Recommendation: Request changes, for the rebase only. The fix itself is right and re-verified on current main. After the rebase, the only things left are resolving the two bot threads and the operator's approval, which merges it straight away because auto-merge is armed.

— Jeremy KK7GWY + Claude

@jensenpat

Copy link
Copy Markdown
Collaborator

Superseded by #6198

@jensenpat jensenpat closed this Oct 7, 2026
auto-merge was automatically disabled October 7, 2026 15:51

Pull request was closed

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.

MainWindow status bar: bottom-row controls get their clicks shadowed by the frameless edge-resize margin

2 participants