Skip to content

[memory] Keep Add Memory visible in short memory drawers#4272

Merged
NF0T merged 1 commit into
aethersdr:mainfrom
jensenpat:aether/fix-3871-memory-button-top
Jul 17, 2026
Merged

[memory] Keep Add Memory visible in short memory drawers#4272
NF0T merged 1 commit into
aethersdr:mainfrom
jensenpat:aether/fix-3871-memory-button-top

Conversation

@jensenpat

Copy link
Copy Markdown
Collaborator

Summary

  • move Add Memory to the top of the Memory drawer so it remains reachable when a short panadapter clips the drawer
  • keep the action immediately above the same scrollable memory list so it reads as the list's primary action
  • give the action a stable automation object name and explicit accessibility description

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.

Before: Add Memory clipped below the list

After: Add Memory remains visible at the top and is grouped directly with the list.

After: Add Memory visible above 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.

After with Slice A and Slice B

Bridge assertions:

  • resize requested and applied: 679 x 494
  • Memory drawer opened through the production Memory button
  • memoryAddButton resolved as a QPushButton with the expected Save the current slice tooltip
  • before, after, and two-slice screenshots captured successfully
  • all proof image URLs verified HTTP 200

Testing

  • cmake --build build -j8
  • memory_recall_policy_test
  • memory_field_values_test
  • memory_csv_compat_test
  • accessibility checker on both changed GUI files: 0 findings
  • git diff --check

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

@jensenpat
jensenpat marked this pull request as ready for review July 16, 2026 19:05
@jensenpat
jensenpat requested a review from a team as a code owner July 16, 2026 19:05
@jensenpat jensenpat changed the title Keep Add Memory visible in short memory drawers [memory] Keep Add Memory visible in short memory drawers Jul 16, 2026

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

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

Comment thread src/gui/MemoryBrowsePanel.cpp
@jensenpat jensenpat added help wanted Extra attention needed priority: low Low priority labels Jul 17, 2026

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

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.

@NF0T
NF0T merged commit 28f0a47 into aethersdr:main Jul 17, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

help wanted Extra attention needed priority: low Low priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

In minimized View, Memory button to Add Memory is not visible

2 participants