Claude here, reviewing on Jeremy's behalf.

Welcome @Miriam-R-coder — first PR in the AetherSDR repo. Thanks for picking up #2574; making Profile Manager non-modal is a real UX improvement worth landing.

Verdict

The architectural change is right (modal → non-modal with QPointer + WA_DeleteOnClose), but the PR has a few specific issues that need fixing before it can land. CI is red (build and analyze (cpp) both FAILURE) and there are some style/API issues we should clean up.

Build failure — blocking

The CI build output flags this exact error:

src/gui/MainWindow.h:440: error: 'ProfileManagerDialog' was not declared in this scope
  440 |     QPointer<ProfileManagerDialog> m_profileManagerDialog;

MainWindow.h doesn't forward-declare ProfileManagerDialog. Existing forward declarations live around lines 67-88 in namespace AetherSDR:

namespace AetherSDR {
class ConnectionPanel;
class TitleBar;
class SpectrumWidget;
...
class WhatsNewDialog;
class CwxPanel;
class DvkPanel;

Fix: add class ProfileManagerDialog; to that block. That's the build-blocker.

AppSettings API style — needs fixing

The project's AppSettings::instance() returns a reference, not a pointer. The PR uses -> syntax:

AppSettings::instance()->value("ProfileManagerDialogGeometry").toByteArray();
AppSettings::instance()->setValue("ProfileManagerDialogGeometry", saveGeometry());

Should be . access. From src/core/AppSettings.h:25:

static AppSettings& instance();

And the usage convention from CLAUDE.md and every other call site in the project:

auto& s = AppSettings::instance();
s.setValue("Key", value);
const QByteArray geom = s.value("Key").toByteArray();

This would compile after the forward-declaration fix because instance() returns AppSettings&, but the -> syntax expects a pointer — so the PR will then hit a second build error: error: base operand of '->' has non-pointer type 'AppSettings'.

Include path

#include "AppSettings.h" should be #include "core/AppSettings.h" — every other call site uses the core/ prefix. Examples:

src/gui/MultiFlexDialog.cpp:#include "core/AppSettings.h"
src/gui/MemoryDialog.cpp:#include "core/AppSettings.h"

Header pollution — minor

#include <QCloseEvent> is now in ProfileManagerDialog.h, which means every translation unit that includes the header pulls in QCloseEvent. Better:

• Forward-declare class QCloseEvent; in the header
• Move #include <QCloseEvent> to ProfileManagerDialog.cpp where it's actually needed

Indentation typo

MainWindow.cpp line 6582 — the connect() line has 8 spaces of leading whitespace instead of the 4 used everywhere else in buildMenuBar(). Visible in the diff as:

-    connect(profileMgrAct, &QAction::triggered, this, [this] {
+        connect(profileMgrAct, &QAction::triggered, this, [this] {

Reduce to 4 spaces to match the surrounding style.

QByteArray ↔ AppSettings round-trip — needs verification

saveGeometry() returns binary QByteArray. AppSettings persists to XML. Whether QVariant::toByteArray() round-trips binary data through XML serialization correctly depends on AppSettings's implementation — most XML stores either base64-encode binary or mangle it. Worth checking by running the dialog, closing it, reopening AetherSDR, and confirming geometry actually restores. If it doesn't, you'll need to .toBase64() on save and QByteArray::fromBase64() on load.

I don't have the AppSettings.cpp source open to confirm whether it handles this — Jeremy can flag if base64 encoding is needed.

Coordination with PR #2580 (just merged)

#2580 merged a few hours ago (commit b94d0d48) which added setFramelessMode(bool) to ProfileManagerDialog. Your PR was branched before that landed and doesn't account for the new frameless chrome.

After rebasing onto current main, you'll see:

• ProfileManagerDialog now embeds a FramelessWindowTitleBar and has a custom outer layout
• setFramelessMode(bool) is called in the constructor based on AppSettings::FramelessWindow
• The dialog's geometry() includes the title bar height

The geometry persist/restore should still work — saveGeometry() captures the window geometry, not the inner layout — but worth testing that the restored dialog comes back at the correct size with the title bar visible.

Stale-code audit

• ✓ m_profileManagerDialog is a QPointer so it nulls automatically on WA_DeleteOnClose destruction. Next menu click correctly creates a fresh dialog.
• ✓ setModal(false) is redundant (show() is already non-modal by default unless setModal(true) was previously called) but harmless.
• ⚠ closeEvent is overridden but the dialog still gets deleted by WA_DeleteOnClose. Order is: closeEvent fires → user's override runs (saves geometry) → calls QDialog::closeEvent(event) → Qt schedules deleteLater. So save-before-delete works as written. Worth a one-line comment noting why the order matters.

Recommendation

Hold on merge until:

1. Add class ProfileManagerDialog; forward declaration in MainWindow.h:65-88 (the build blocker)
2. Replace AppSettings::instance()-> with AppSettings::instance(). in both ProfileManagerDialog.cpp sites
3. Fix the include path to "core/AppSettings.h"
4. Fix the indentation at MainWindow.cpp:6582 (8 → 4 spaces)
5. Verify the QByteArray round-trip through AppSettings works (or add base64 encoding if not)
6. Rebase onto current main (#2580 just added frameless chrome to this dialog) and confirm saveGeometry/restoreGeometry work with the new chrome
7. Confirm CI green after the changes

The architectural shape is right and the feature is wanted — this is just first-pass polish work. Happy to push the build-fix changes to your branch if that would help you get unblocked.

Thanks again for the PR. The non-modal Profile Manager will be a nice quality-of-life improvement once these are sorted.

73, Jeremy KK7GWY & Claude (AI dev partner)

Claude here, final review pass.

Status

Pushed merge commit `a3dbbfe6` syncing in the 3 commits that landed on main since the last touch (#2586 interlock notifications, #2593 dBm drag fix, the wallpaper merge). Clean auto-merge — no conflicts.

Final state of the PR

Six commits ending in the merge:

1. `bc52c00a` Add geometry restoration and saving — Miriam's original work
2. `e1e40145` Add ProfileManagerDialog pointer to MainWindow
3. `83ee1f77` Update profile manager dialog handling
4. `f8910791` Add closeEvent method to ProfileManagerDialog
5. `1785b684` First rebase + 9 review fixes (forward decl, AppSettings API, include path, base64, indentation, #2580 integration, etc.)
6. `3ec211b7` Adopt #2592 robustness (m_restoringGeometry guard + move/resize saving) + add docs/dialog-patterns.md + CLAUDE.md pointer

Post-merge audit

All recent main work preserved:

Built locally clean (only pre-existing unrelated warnings). All 5 CI checks green on the pre-merge HEAD; should re-run green after this push.

What this PR delivers, summarized

The Profile Manager non-modal conversion proper:

• `QPointer m_profileManagerDialog` slot in MainWindow
• Lazy-construct on menu click; `WA_DeleteOnClose` + `raise()` + `activateWindow()` on re-invoke
• Geometry persistence via base64-encoded `saveGeometry()` round-trip through AppSettings
• Save fires on `closeEvent` (canonical disk flush via `s.save()`) and on every `moveEvent`/`resizeEvent` (crash-resilient in-memory updates)
• `m_restoringGeometry` guard prevents recursive-save during `restoreGeometry()`
• #2580 frameless chrome integration — `setFramelessMode(framelessWindowEnabled())` on construct + wiring into `setFramelessWindow()` for runtime toggle propagation

Plus the infrastructure dividend:

• `docs/dialog-patterns.md` — canonical reference for the lazy-construct + non-modal + geometry-persist + frameless-chrome pattern, with a 9-row "common pitfalls" table (wrong/right + the PR that surfaced each one)
• CLAUDE.md points contributors at the doc before any `QDialog` work
• Bridge until the `PersistentDialog` base class (#2605) lands

Recommendation

Merge after Jeremy validates on his FLEX-8600:

1. Open Profile Manager → expected: non-modal, can interact with the radio behind it
2. Move + resize the dialog → expected: geometry persists across close/reopen
3. Close + reopen → expected: same position and size
4. App relaunch → expected: geometry survives (verifies the close-time `s.save()` flushed to disk)
5. Toggle View → Use frameless windows while Profile Manager is open → expected: chrome propagates correctly to the open dialog

Thanks to @Miriam-R-coder for the first PR (the architectural shape was right and gave us the seed to build on), and to @aethersdr-agent for the m_restoring + move/resize ideas (preserved on this branch via Co-Authored-By). Good collaborative outcome.

73, Jeremy KK7GWY & Claude (AI dev partner)

Approved — Profile Manager non-modal + dialog-patterns infrastructure.