[memory] Keep Add Memory visible in short memory drawers#4272
Conversation
There was a problem hiding this comment.
Clean, well-scoped fix. Moving Add Memory to layout index 0 with insertWidget(0, …) correctly keeps it above the mutually-exclusive empty-label/table pair (both stretch-1, button stretch-0), so it stays visible when a short panadapter clips the fixed 252×460 drawer. All widgets are parented, no AppSettings/QSettings touched, no lifecycle or null-pointer concerns, and the objectName + accessible metadata are a welcome addition for automation and a11y. The paired comment update in SpectrumOverlayMenu.cpp keeps the two files consistent. Nothing blocking.
Non-blocking notes
setAccessibleName("Add Memory")duplicates the button's own visible text — QAccessible already derives the accessible name from the button text, so this is redundant. Harmless, and defensible if you want it pinned for automation; just noting it.
Thanks for the thorough write-up and the multi-slice bridge proof — nice work.
🤖 aethersdr-agent · cost: $6.4411 · model: claude-opus-4-8
NF0T
left a comment
There was a problem hiding this comment.
Verified the fix directly against SpectrumOverlayMenu::toggleMemoryPanel()'s clamp logic (panelY = std::clamp(panelY, 0, qMax(0, parent->height() - panelH))) — on a short panadapter the panel pins to y=0 and Qt's standard child-clipping cuts off whatever's at the bottom of the fixed 252x460 drawer. Moving m_addBtn to layout index 0 is a correct, minimal fix; confirmed no positional/index-based lookups or tab-order behavior depend on its old position (Qt's default tab order follows widget creation order, not layout order, and the construction point is unchanged). The new objectName/accessibleName/accessibleDescription calls match the established pattern used at 150+ other call sites in this codebase.
CI green across all 7 checks. Commit signed.
One framing note, not a blocker: the PR body says this placement "follows the requested UX direction in #3871" — the issue's only concrete proposal on record is actually the opposite approach (an adaptive/shrinking panel keeping the button at the bottom), from a bot comment, and no maintainer weighed in on the issue. The top-placement is a reasonable engineering call on the author's part, just not something #3871 specifically asked for — worth a quick mental note, not something to hold the PR on since the maintainer-review label is still in place for final visual sign-off.
Non-blocking follow-up: the ANT panel (buildAntPanel()/toggleAntPanel()) has the same shape — no bottom clamp, 8 stacked rows, no scroll wrapper — and could clip its Save CSV button/sweep-range fields on a short panadapter the same way. Out of scope here since #3871 was Memory-drawer-specific, but worth a follow-up issue.
Approving — Tier 3 only (src/gui/MemoryBrowsePanel.cpp, src/gui/SpectrumOverlayMenu.cpp), no Tier 2/1 gate to satisfy.
Summary
Root cause
MemoryBrowsePanel is a fixed 252 x 460 drawer. Add Memory was the final child below the stretchable table, so a short application window or constrained multi-slice view clipped the bottom of the drawer and hid the action. The bottom placement also left the action visually detached beneath the list.
The fix preserves the existing drawer size, memory ordering, scrollbar, recall behavior, and per-pan slice routing. It changes only the layout order: Add Memory is now layout index 0, directly followed by the scrollable list.
Proof
The agent automation bridge reproduced the issue and verified the fix against the live radio at the reporter-style 679 x 494 window size. This was RX-only; no transmitter action was performed.
Before: the list is visible, but Add Memory is below the clipped window.
After: Add Memory remains visible at the top and is grouped directly with the list.
Two-slice proof: a temporary Slice B was added alongside Slice A; Add Memory remained visible at the same short height. Slice B was removed afterward and the bridge confirmed the radio returned to its original single-slice state.
Bridge assertions:
Testing
This placement follows the requested UX direction in #3871; the maintainer-review label remains appropriate for final visual approval.
Fixes #3871
👨🏼💻 Generated with OpenAI Codex (GPT-5.6 Sol) and tested by @jensenpat