feat: improve focus restoration when restoring window from tray#1060
Open
feat: improve focus restoration when restoring window from tray#1060
Conversation
- Save focused control (weakref) on minimize; restore it on show - Defer focus restore by 80ms so window is visible first - Fall back to prompt, panel, or frame if saved control is invalid - Add _restore_focus() used by on_restore and post_screen_capture - IPC handler calls _restore_focus when window is already visible
Contributor
WalkthroughIntroduces a new focus restoration mechanism in the main frame that remembers the last focused control before window minimization. On restore, it attempts to refocus that control with fallbacks to current tab panels or the main frame. Updates the app to use this new restoration path instead of direct focus forcing. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@basilisk/gui/main_frame.py`:
- Around line 429-431: The saved weak reference _last_focused_before_minimize is
not cleared after a successful restore, allowing stale controls to be refocused
later; update the minimize/restore logic (where _last_focused_before_minimize is
set and where _restore_focus() runs) so that after you successfully set focus to
the referenced window you immediately clear/consume the weakref (set
_last_focused_before_minimize to None or similar) to prevent subsequent calls
from using a stale reference; ensure both the code that assigns the weakref (the
block using wx.Window.FindFocus()) and the block in _restore_focus() that
actually refocuses the control are updated to clear the ref after success.
Comment on lines
+429
to
+431
| focused = wx.Window.FindFocus() | ||
| if focused and focused.GetTopLevelParent() == self: | ||
| self._last_focused_before_minimize = weakref.ref(focused) |
Contributor
There was a problem hiding this comment.
Consume _last_focused_before_minimize after restore to avoid stale refocus.
The saved weak reference is kept when restore succeeds, so later _restore_focus() calls can incorrectly force focus back to an old control from a previous minimize cycle.
🔧 Proposed fix
def on_minimize(self, event: wx.Event | None):
"""Minimize the main application frame to the system tray.
@@
- focused = wx.Window.FindFocus()
- if focused and focused.GetTopLevelParent() == self:
- self._last_focused_before_minimize = weakref.ref(focused)
+ focused = wx.Window.FindFocus()
+ self._last_focused_before_minimize = None
+ if focused and focused.GetTopLevelParent() == self:
+ self._last_focused_before_minimize = weakref.ref(focused)
@@
def _restore_focus(self):
"""Restore focus to the last focused control or a meaningful fallback."""
- if self._last_focused_before_minimize:
+ focus_ref = self._last_focused_before_minimize
+ self._last_focused_before_minimize = None
+ if focus_ref:
try:
- ctrl = self._last_focused_before_minimize()
+ ctrl = focus_ref()
if (
ctrl is not None
and ctrl.IsShown()
and ctrl.GetTopLevelParent() == self
):
ctrl.SetFocus()
return
except (AttributeError, RuntimeError) as e:
log.error("Failed to restore focus to last control: %s", e)
- self._last_focused_before_minimize = NoneAlso applies to: 451-463
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@basilisk/gui/main_frame.py` around lines 429 - 431, The saved weak reference
_last_focused_before_minimize is not cleared after a successful restore,
allowing stale controls to be refocused later; update the minimize/restore logic
(where _last_focused_before_minimize is set and where _restore_focus() runs) so
that after you successfully set focus to the referenced window you immediately
clear/consume the weakref (set _last_focused_before_minimize to None or similar)
to prevent subsequent calls from using a stale reference; ensure both the code
that assigns the weakref (the block using wx.Window.FindFocus()) and the block
in _restore_focus() that actually refocuses the control are updated to clear the
ref after success.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit