Add frameless support for popout dialogs#2580
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the app’s existing frameless-window infrastructure (custom title bar + edge resizer) to additional “popout” dialogs and addresses a Qt/macOS geometry-restore edge case that could cause newly opened dialogs to appear at the top-left corner.
Changes:
- Add frameless-window chrome support (shared
FramelessWindowTitleBar+FramelessResizer) to Panadapter Layout, MIDI Mapping, multiFLEX Dashboard, and Profile Manager dialogs. - Fix geometry-restore ordering by only restoring geometry after toggling
Qt::FramelessWindowHintwhen the dialog is already visible (avoids macOS placement issues on first show). - Make “TX Band Settings” modeless and prevent duplicate instances by raising the existing dialog; also hooks it into global frameless toggles.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/gui/ProfileManagerDialog.h | Adds frameless toggle API and members for custom title bar/layout margin adjustments. |
| src/gui/ProfileManagerDialog.cpp | Implements frameless chrome/layout and visibility-guarded geometry restore. |
| src/gui/PanLayoutDialog.h | Adds frameless toggle API and members for custom title bar/layout margin adjustments. |
| src/gui/PanLayoutDialog.cpp | Implements frameless chrome/layout, resizer install, and size adjustment for fixed-size dialog. |
| src/gui/NetworkDiagnosticsDialog.cpp | Updates frameless toggle to restore geometry only when already visible. |
| src/gui/MultiFlexDialog.h | Adds frameless toggle API and members for custom title bar/layout margin adjustments. |
| src/gui/MultiFlexDialog.cpp | Implements frameless chrome/layout and visibility-guarded geometry restore. |
| src/gui/MidiMappingDialog.h | Adds frameless toggle API and members for custom title bar/layout margin adjustments. |
| src/gui/MidiMappingDialog.cpp | Implements frameless chrome/layout and visibility-guarded geometry restore. |
| src/gui/MemoryDialog.cpp | Applies the same visibility-guarded geometry restore and adds missing braces for clarity/safety. |
| src/gui/MainWindow.h | Tracks TX Band Settings dialog instance to prevent duplicates and support frameless toggles. |
| src/gui/MainWindow.cpp | Guards geometry restore in setDialogFramelessMode, adds single-instance TX Band Settings window with frameless chrome. |
| src/gui/DxClusterDialog.cpp | Applies visibility-guarded geometry restore and adds missing braces for clarity/safety. |
| src/gui/AetherDspDialog.cpp | Applies visibility-guarded geometry restore and adds missing braces for clarity/safety. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
309d9d4 to
6ac9850
Compare
ten9876
left a comment
There was a problem hiding this comment.
Claude here, reviewing on Jeremy's behalf.
Verdict
Merge-worthy. Two distinct improvements bundled cleanly; built clean locally. One stale-code finding worth filing as a follow-up — three more dialogs in the same bug class were not fixed by this PR.
What the PR does, in two parts
Part 1 — Bug fix: skip setGeometry(geom) when not visible
The pre-PR pattern was:
const QRect geom = geometry();
const bool wasVisible = isVisible();
setWindowFlags(flags);
setGeometry(geom); // ← runs even on first constructionOn macOS, setGeometry(geom) during the hidden-construction phase ends up capturing the pre-show default position (top-left), so Qt's normal placement logic (centering on parent, screen, last position) doesn't run. The dialog opens flush to the upper-left corner.
The PR's fix is the right shape:
if (wasVisible) {
setGeometry(geom); // only on runtime toggle, when dialog IS already placed
}First open: wasVisible is false → Qt places normally.
Runtime toggle: wasVisible is true → geometry preserved across the flag change.
Applied to 5 sites: AetherDspDialog, DxClusterDialog, MainWindow::setDialogFramelessMode, MemoryDialog, NetworkDiagnosticsDialog.
Part 2 — Frameless support added to 5 more popouts
PanLayoutDialog, MidiMappingDialog, MultiFlexDialog, ProfileManagerDialog, and the inline TX Band Settings dialog in MainWindow::buildMenuBar() now all support the global Frameless Window setting via the same setFramelessMode(bool) pattern.
The pattern is consistent across all four header-declared dialogs:
- New
outerVBoxLayout, embedFramelessWindowTitleBar+ bodyWidget FramelessResizer::install(this)in ctorsetFramelessMode()method that togglesQt::FramelessWindowHint, preserves geometry only when visible, hides/shows title bar, adjusts body margins- Wired into
MainWindow::setFramelessWindow()so runtime toggle propagates
For PanLayoutDialog specifically, the setFixedSize is bumped from 520→538 px when frameless to absorb the title bar height. Reasonable accommodation for a fixed-size dialog.
Side improvement worth calling out
TX Band Settings now reuses an existing dialog instance via QPointer<QDialog> m_txBandDialog and raises it on re-invocation, instead of creating a new instance each time. Previously each menu click leaked a fresh dialog. Clean side-effect-of-the-frameless-rewrite improvement.
Stale-code audit — three more dialogs need the same wasVisible guard
The PR's stated scope is the 5 fixed dialogs plus the 5 new popouts. Three other dialogs have the same setFramelessMode shape with unguarded setGeometry(geom) and would benefit from the same fix:
| File | Status |
|---|---|
src/gui/PropDashboardDialog.cpp |
Unguarded setGeometry(geom) — same first-open bug class |
src/gui/RadioSetupDialog.cpp |
Unguarded setGeometry(geom) — same first-open bug class |
src/gui/PanFloatingWindow.cpp |
Unguarded setGeometry(geom) after flag change — same bug class for popped-out panadapter windows |
The PR description's "Apply that placement fix to … the newly updated popouts" makes the scope clear; these three sites just weren't in scope. Worth filing as a follow-up issue so they don't drift further (currently their first-open macOS placement is broken, identical to what this PR fixes elsewhere).
Verified
- Built locally clean (only pre-existing unrelated warning)
- All 5 CI checks green:
build,analyze (cpp),check-paths, CodeQL,check-windowsskipped - New dialog wiring follows the established pattern from
AetherDspDialog,MemoryDialog, etc. — no architectural surprises m_txBandDialogmember added toMainWindow.hwithQPointerfor safety against accidental dangling
Minor notes
MidiMappingDialogand the others readAppSettings::instance().value("FramelessWindow", "True")directly in their ctors. That's consistent with how other dialogs do it, but creates 5 places to update if the default ever changes. A smallMainWindow::framelessWindowEnabled()accessor existed and could have been used — minor consistency win, not worth churning.PanLayoutDialog'ssetFixedSize(560, on ? 538 : 520)— the magic 538 vs 520 = 18 px title bar. Worth a one-line comment noting where that constant comes from in case the title bar height ever changes.
Recommendation
Merge after Jeremy spot-checks the new frameless dialogs on macOS (the platform the bug fix specifically targets). The five newly-frameless popouts should open with proper Qt placement on first invocation, and toggling View → Use frameless windows at runtime should preserve geometry on already-open dialogs.
Three follow-up cleanup sites worth filing: PropDashboardDialog, RadioSetupDialog, PanFloatingWindow.
Thanks @rfoust — the consistency across the 5 new dialogs (same pattern, same helper calls, same setFramelessMode shape) means anyone wiring a 6th later has a template to copy.
73, Jeremy KK7GWY & Claude (AI dev partner)
ten9876
left a comment
There was a problem hiding this comment.
Approved — clean two-part fix, consistent pattern across the 5 new popouts.
…log-patterns doc Cherry-picks the robustness improvements from AetherClaude's parallel PR aethersdr#2592 onto this branch: - m_restoringGeometry guard prevents the recursive save-during-restore loop when restoreGeometry() triggers move/resize events - Save geometry on moveEvent and resizeEvent in addition to closeEvent. setValue() is in-memory only — close-time save() flushes to disk. Move/resize saves keep the in-memory store current so crash or force-quit preserves the last-known position Also adds docs/dialog-patterns.md capturing the canonical lazy-construct + non-modal + geometry-persist + frameless-chrome pattern in one place. Multiple recent PRs (aethersdr#2591, aethersdr#2592, aethersdr#2580) have each rediscovered the same pitfalls — the doc lists them in a single table with the wrong/right pairing and the PR that surfaced each one. CLAUDE.md now points contributors at the doc before they write or modify any QDialog. The longer-term cleanup is the PersistentDialog base class proposed in aethersdr#2605; this doc is the bridge until that lands. Co-Authored-By: aethersdr-agent <aethersdr-agent@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add geometry restoration and saving in ProfileManagerDialog Signed-off-by: Miriam <m058325352@gmail.com> * Add ProfileManagerDialog pointer to MainWindow Signed-off-by: Miriam <m058325352@gmail.com> * Update profile manager dialog handling Refactor profile manager dialog connection to allow multiple instances. Signed-off-by: Miriam <m058325352@gmail.com> * Add closeEvent method to ProfileManagerDialog Signed-off-by: Miriam <m058325352@gmail.com> * feat(profile-manager): adopt #2592 geometry robustness + dialog-patterns doc Cherry-picks the robustness improvements from AetherClaude's parallel PR #2592 onto this branch: - m_restoringGeometry guard prevents the recursive save-during-restore loop when restoreGeometry() triggers move/resize events - Save geometry on moveEvent and resizeEvent in addition to closeEvent. setValue() is in-memory only — close-time save() flushes to disk. Move/resize saves keep the in-memory store current so crash or force-quit preserves the last-known position Also adds docs/dialog-patterns.md capturing the canonical lazy-construct + non-modal + geometry-persist + frameless-chrome pattern in one place. Multiple recent PRs (#2591, #2592, #2580) have each rediscovered the same pitfalls — the doc lists them in a single table with the wrong/right pairing and the PR that surfaced each one. CLAUDE.md now points contributors at the doc before they write or modify any QDialog. The longer-term cleanup is the PersistentDialog base class proposed in #2605; this doc is the bridge until that lands. Co-Authored-By: aethersdr-agent <aethersdr-agent@users.noreply.github.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Signed-off-by: Miriam <m058325352@gmail.com> Co-authored-by: Jeremy Fielder <kk7gwy@aethersdr.com> Co-authored-by: aethersdr-agent <aethersdr-agent@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
#2603) Follow-up to #2580. PropDashboardDialog, RadioSetupDialog, and PanFloatingWindow all reapplied geometry unconditionally after a flag change in setFramelessMode(), causing the same macOS cold-launch top-left placement bug fixed in the 5 dialogs touched by #2580. Wrap each setGeometry(geom) in `if (wasVisible)` so Qt's normal placement logic (centering on parent, restored last position, etc.) runs on first show, while runtime frameless toggles on already-visible dialogs still preserve geometry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
#2603) Follow-up to #2580. PropDashboardDialog, RadioSetupDialog, and PanFloatingWindow all reapplied geometry unconditionally after a flag change in setFramelessMode(), causing the same macOS cold-launch top-left placement bug fixed in the 5 dialogs touched by #2580. Wrap each setGeometry(geom) in `if (wasVisible)` so Qt's normal placement logic (centering on parent, restored last position, etc.) runs on first show, while runtime frameless toggles on already-visible dialogs still preserve geometry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
#2603) (#2635) * fix(gui): wasVisible-guard setGeometry in three more frameless toggles (#2603) Follow-up to #2580. PropDashboardDialog, RadioSetupDialog, and PanFloatingWindow all reapplied geometry unconditionally after a flag change in setFramelessMode(), causing the same macOS cold-launch top-left placement bug fixed in the 5 dialogs touched by #2580. Wrap each setGeometry(geom) in `if (wasVisible)` so Qt's normal placement logic (centering on parent, restored last position, etc.) runs on first show, while runtime frameless toggles on already-visible dialogs still preserve geometry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * fix(gui): wasVisible-guard setGeometry in three more frameless toggles (#2603) Follow-up to #2580. PropDashboardDialog, RadioSetupDialog, and PanFloatingWindow all reapplied geometry unconditionally after a flag change in setFramelessMode(), causing the same macOS cold-launch top-left placement bug fixed in the 5 dialogs touched by #2580. Wrap each setGeometry(geom) in `if (wasVisible)` so Qt's normal placement logic (centering on parent, restored last position, etc.) runs on first show, while runtime frameless toggles on already-visible dialogs still preserve geometry. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: aethersdr-agent[bot] <273844287+aethersdr-agent[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
…2646) (#2685) PRs #2580 and #2635 (closing #2603) applied the `if (wasVisible) setGeometry(geom)` guard across most `setFramelessMode(bool)` implementations. Four sites with the same code-shape were missed by both passes — fix listed in #2646: 1. ConnectionPanel::setFramelessMode 2. AetherialAudioStrip::setFramelessMode 3. FloatingContainerWindow::setFramelessMode 4. FloatingContainerWindow::setAlwaysOnTop All four captured `wasVisible` and `geom` early but called `setGeometry(geom)` unconditionally after `setWindowFlags(flags)`. On a cold-launched (never-shown) instance, `geom` is the default top-left rect and the unconditional `setGeometry` clobbers Qt's normal placement, leaving the window stuck at (0,0) on first show. Closes #2646. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Bug
Some dialogs that already supported frameless mode restored their geometry while still hidden during construction. On Qt/macOS this can preserve the pre-show default position, causing dialogs to open in the top-left corner instead of letting Qt place them normally.
Fix
Verification