Refresh visible page when wxPreferencesEditor window is shown or activated on macOS (PR #26912)
Daniel Kulp
The macOS implementation of wxPreferencesEditor only fills each page with data (via wxEVT_INIT_DIALOG / TransferDataToWindow) once, when the page window is first created. If settings are changed elsewhere in the application and the preferences window is then shown again, it keeps displaying the stale values.
Fixes #22165, reworked from #22169 along the lines discussed there: the refresh now happens when the window is activated (as suggested by @vslavik), which covers both re-showing it and bringing it back to the foreground after it was hidden behind other windows; Show() also refreshes directly in case the window is re-shown without becoming key. The refreshed page is re-fitted afterwards since the updated contents may change its best size.
Per @vadz's feedback on #22169, page switching is intentionally left unchanged: switching to another page and back does not re-run TransferDataToWindow.
🤖 Generated with Claude Code
You can view, comment on, or merge this pull request online at:
https://github.com/wxWidgets/wxWidgets/pull/26912
Commit Summary
- 77ac1aa Refresh visible page when wxPreferencesEditor window is shown or activated on macOS
File Changes
(1 file)
- M src/osx/cocoa/preferences.mm (31)
Patch Links:
- https://github.com/wxWidgets/wxWidgets/pull/26912.patch
- https://github.com/wxWidgets/wxWidgets/pull/26912.diff
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Copilot
@Copilot commented on this pull request.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Updates the macOS Cocoa wxPreferencesEditor window to refresh the currently visible preferences page when the window is re-shown or re-activated, ensuring displayed settings don’t become stale.
Changes:
- Refresh visible page data when the preferences window is shown again.
- Refresh visible page data when the preferences window becomes active (foreground).
- Re-fit and resize the window after refreshing to account for size changes.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
In src/osx/cocoa/preferences.mm:
> @@ -91,6 +91,7 @@
m_toolbar->Bind(wxEVT_TOOL,
&wxCocoaPrefsWindow::OnPageChanged, this);
Bind(wxEVT_CLOSE_WINDOW, &wxCocoaPrefsWindow::OnClose, this);
+ Bind(wxEVT_ACTIVATE, &wxCocoaPrefsWindow::OnActivate, this);
RefreshVisiblePage() can be invoked twice when re-showing the window: once from Show(true) and again from OnActivate (per the comment, showing activates it). This can cause duplicate InitDialog / TransferDataToWindow runs and redundant re-fitting/resizing. Consider adding a small guard to ensure a single refresh per re-show (e.g., set a “needs refresh on next activate” flag in Show(true) and only refresh in one place, or track/clear a “refreshed since last show” state).
In src/osx/cocoa/preferences.mm:
> + // The window is being shown again after having been hidden: the + // settings may have been changed elsewhere in the application in + // the meantime, so refresh the visible page (see #22165). + RefreshVisiblePage();
RefreshVisiblePage() can be invoked twice when re-showing the window: once from Show(true) and again from OnActivate (per the comment, showing activates it). This can cause duplicate InitDialog / TransferDataToWindow runs and redundant re-fitting/resizing. Consider adding a small guard to ensure a single refresh per re-show (e.g., set a “needs refresh on next activate” flag in Show(true) and only refresh in one place, or track/clear a “refreshed since last show” state).
In src/osx/cocoa/preferences.mm:
> + void OnActivate(wxActivateEvent& event)
+ {
+ event.Skip();
+
+ // Refresh the page when the already shown window comes back to the
+ // foreground: the settings may have been changed from the other
+ // application windows while it was in the background (see #22165).
+ // Note that this also covers the window being re-shown, as showing
+ // it activates it.
+ if ( event.GetActive() && m_visiblePage && IsShownOnScreen() )
+ RefreshVisiblePage();
+ }
RefreshVisiblePage() can be invoked twice when re-showing the window: once from Show(true) and again from OnActivate (per the comment, showing activates it). This can cause duplicate InitDialog / TransferDataToWindow runs and redundant re-fitting/resizing. Consider adding a small guard to ensure a single refresh per re-show (e.g., set a “needs refresh on next activate” flag in Show(true) and only refresh in one place, or track/clear a “refreshed since last show” state).
In src/osx/cocoa/preferences.mm:
> + void RefreshVisiblePage()
+ {
+ m_visiblePage->InitDialog();
+ FitPageWindow(m_visiblePage);
+ SetClientSize(m_visiblePage->GetSize());
+ m_visiblePage->Refresh();
+ }
RefreshVisiblePage() assumes m_visiblePage is always non-null. While current call sites check it, adding an internal guard (e.g., wxCHECK_RET(m_visiblePage, ...) or an assert) makes the helper safer against future changes and prevents hard crashes if it’s ever called without the precondition.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
@vadz approved this pull request.
Thanks, I didn't follow Copilot comments in details and don't know if you want to do it, but if the PR works in practice as written, it's good enough to be merged for me.
I'll wait for any comments from Vaclav for a few days before merging it.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Václav Slavík
vslavik left a comment (wxWidgets/wxWidgets#26912)I'll wait for any comments from Vaclav for a few days before merging it.
FWIW, I don't see anything wrong with it; looks like a clear improvement to me.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
Stefan Csomor
csomor left a comment (wxWidgets/wxWidgets#26912)@dkulp Thanks a lot!
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.
VZ
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.