-
-
Notifications
You must be signed in to change notification settings - Fork 687
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added on_gain_focus
, on_lose_focus
, on_show
& on_hide
handlers on toga.Window
#2096
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution - this is another feature that is going to hit up against #2058 (and #2075); as such, I'm hesitant to merge it without tests.
I'm also hesitant because it doesn't currently have a Cocoa implementation (that should be easy enough, but it's worth flagging); and it's not 100% obvious how this would behave on mobile. My immediate reaction (and the one raised in #2006) is that gain/lose focus might link into application lifecycle hooks (so when the app comes into the foreground, that's the "gain focus" event for both the window and the app), but there's an open question of how those signals interact with tablet platforms that allow split-screen and other multi-app modes. This is an area where some additional design is required.
Yes, I agree with you. This should not be merged currently. I will need some time to write the implementation for other backends. Like #1930, I will wait until the audits are merged, and will write the tests thereafter. As for additional design for tablet modes, I agree with you. I will research more about it and will discuss with you while implementing for mobile platforms. |
@mhsmith I need your guidance on android side. According to: https://developer.android.com/guide/components/activities/intro-activities#onpause
On the Emulator(Android 12):When starting the app, the following events are triggered: When I select the app by pressing the Recents button, only
When I press the home button, neither On a Physical Device(Android 13):When starting the app, the following events are triggered:
When I select the app by pressing the Recents button, only
When I press the home button, neither
As, you can see, Why are the documented Activity lifecycle events not being triggered as per the documentation? |
Regarding the gain/lose focus on mobile platforms like android: From my testing, the app will lose focus when either the In split screen mode (like dual app mode), suppose there are two apps A and B. App In floating window mode, the app will gain focus when the user touches the app's screen. The focus is lost when the user touches anything outside the app, like interacting with the system launcher or another app. In iOS like the cocoa, there exists But, there needs to be another handler to differentiate between the states when (the app is not visible to the user & is not receiving inputs) and (when the app is visible to the user & is receiving inputs). Hence, I would like to propose other additional handlers, What do you guys think? Also, without confirmation from @mhsmith regarding the Activity life events triggering behavior, I cannot proceed with the android implementation. Hence, I was thinking about working on the iOS implementation first. |
That's because those methods aren't included in the Android template, either in
See this page for how this is notified on Android.
Every API has a maintenance and testing cost, so I'd prefer not to add additional events unless there's a clear need for them, especially if they're only applicable to certain platforms. |
Thank you for helping. I will add default implementations for the remaining methods in the Android template and will submit a PR there after getting a stable behavior. I agree with you that additional events will incur more maintenance. I feel that the For example, the app should be put to a sleep mode(not updating the layout or text) when it is in background or What do you think? |
I have tested android implementation both on a physical device and on the emulator. I have submitted a PR at beeware/briefcase-android-gradle-template#69 so that the app focus event can be detected. |
Completed implementations of all the platforms and also added a test in the window example app. I will write the tests after the audits are merged. But I think this PR is ready for a review. Also, the CI android testbed is failing on its own for some reason. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you've done here looks like a good pass at implementing the API as proposed in #2009; however, I think we're hitting an area where we need more design before we proceed.
The detail you've dug up as part of the Android implementation has opened a bunch of design questions about what "focus" even means at the App/Window level. What are we actually trying to achieve with these signal handlers? Is "visibility" a better metaphor than "focus" in this case? Is there any use case for a literal "focus" event on a window? Do we need to differentiate between an app that is "visible", but isn't currently accepting input events, and an app that isn't accepting input events? How does the rest of the app lifecycle map into these events (on all platforms)?
Rather than pressing forward with an implementation, I think we need to step back and come up with a consistent design for these app/window lifecycle events, and work out how they map onto all the platforms we're targeting.
Researching some more on the topic, it seems like we need 3 categories of events:
The following are the states associated with the event categories mentioned above and their implications for other event categories: Input Focus ---> Visiblity -> Hover -> Use Cases:
Who should have which event categories:
APIs are not much of a problem as the available platform APIs can be properly mapped onto the above described event categories. |
on_gain_focus
, on_lose_focus
, on_show
& on_hide
handlers on toga.Window
on_gain_focus
, on_lose_focus
, on_show
, on_hide
& on_resize
handlers on toga.Window
|
Sure. I'll do it in a separate PR. |
on_gain_focus
, on_lose_focus
, on_show
, on_hide
& on_resize
handlers on toga.Window
on_gain_focus
, on_lose_focus
, on_show
& on_hide
handlers on toga.Window
@@ -32,6 +32,9 @@ class Window(Container, Scalable): | |||
def __init__(self, interface, title, position, size): | |||
self.interface = interface | |||
|
|||
# Required for triggering event handlers at appropriate times. | |||
self._is_previously_shown = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This flag is required to keep track of when to trigger on_show()
and on_hide()
events at the correct time. This flag can be eliminated by the use of https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.control.wndproc?view=windowsdesktop-9.0. However, for whatever reason directly overriding the method doesn't work: pythonnet/pythonnet#2536.
So, I had used win32 APIs directly to override WndProc()
, which worked correctly:
diff --git a/winforms/src/toga_winforms/libs/user32.py b/winforms/src/toga_winforms/libs/user32.py
index 32d2698df..cd18f5020 100644
--- a/winforms/src/toga_winforms/libs/user32.py
+++ b/winforms/src/toga_winforms/libs/user32.py
@@ -1,4 +1,4 @@
-from ctypes import c_void_p, windll, wintypes
+from ctypes import c_long, c_void_p, windll, wintypes, WINFUNCTYPE, WinDLL
from System import Environment
@@ -34,3 +34,32 @@ MONITOR_DEFAULTTONEAREST = 2
MonitorFromRect = user32.MonitorFromRect
MonitorFromRect.restype = wintypes.HMONITOR
MonitorFromRect.argtypes = [wintypes.LPRECT, wintypes.DWORD]
+
+user32 = WinDLL("user32", use_last_error=True)
+# https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-getwindowlongptrw
+GWL_WNDPROC = -4
+
+GetWindowLongPtrW = user32.GetWindowLongPtrW
+GetWindowLongPtrW.argtypes = [wintypes.HWND, wintypes.INT]
+GetWindowLongPtrW.restype = c_void_p
+
+# https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-setwindowlongptrw
+SetWindowLongPtrW = user32.SetWindowLongPtrW
+SetWindowLongPtrW.argtypes = [wintypes.HWND, wintypes.INT, c_void_p]
+SetWindowLongPtrW.restype = c_void_p
+
+# https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-callwindowprocw
+CallWindowProcW = user32.CallWindowProcW
+CallWindowProcW.argtypes = [
+ c_void_p,
+ wintypes.HWND,
+ wintypes.UINT,
+ wintypes.WPARAM,
+ wintypes.LPARAM,
+]
+CallWindowProcW.restype = c_long
+
+# https://learn.microsoft.com/en-us/windows/win32/api/winuser/nc-winuser-wndproc
+WNDPROC = WINFUNCTYPE(
+ c_long, wintypes.HWND, wintypes.UINT, wintypes.WPARAM, wintypes.LPARAM
+)
diff --git a/winforms/src/toga_winforms/window.py b/winforms/src/toga_winforms/window.py
index de43e55df..823eebee6 100644
--- a/winforms/src/toga_winforms/window.py
+++ b/winforms/src/toga_winforms/window.py
@@ -1,6 +1,7 @@
from __future__ import annotations
from typing import TYPE_CHECKING
+import ctypes
import System.Windows.Forms as WinForms
from System.Drawing import Bitmap, Font as WinFont, Graphics, Point, Size as WinSize
@@ -15,6 +16,13 @@ from toga.types import Position, Size
from .container import Container
from .fonts import DEFAULT_FONT
from .libs.wrapper import WeakrefCallable
+from .libs.user32 import (
+ GWL_WNDPROC,
+ WNDPROC,
+ GetWindowLongPtrW,
+ SetWindowLongPtrW,
+ CallWindowProcW,
+)
from .screens import Screen as ScreenImpl
from .widgets.base import Scalable
@@ -36,6 +44,18 @@ class Window(Container, Scalable):
self.create()
+ # Get the window procedure (WndProc) for the window
+ self.original_wndproc = GetWindowLongPtrW(
+ self.native.Handle.ToInt64(), GWL_WNDPROC
+ )
+ # Register our WndProc to handle window messages
+ self.wndproc_callback = WNDPROC(self.win32_WndProc)
+ SetWindowLongPtrW(
+ self.native.Handle.ToInt64(),
+ GWL_WNDPROC,
+ ctypes.cast(self.wndproc_callback, ctypes.c_void_p),
+ )
+
self._FormClosing_handler = WeakrefCallable(self.winforms_FormClosing)
self.native.FormClosing += self._FormClosing_handler
super().__init__(self.native)
@@ -89,6 +109,30 @@ class Window(Container, Scalable):
######################################################################
# Native event handlers
######################################################################
+ def win32_WndProc(self, hwnd, msg, w_param, l_param):
+ previous_state = self.get_window_state()
+ result = CallWindowProcW(self.original_wndproc, hwnd, msg, w_param, l_param)
+ current_state = self.get_window_state()
+ if previous_state != current_state:
+ if previous_state == WindowState.MINIMIZED and current_state in {
+ WindowState.NORMAL,
+ WindowState.MAXIMIZED,
+ WindowState.FULLSCREEN,
+ WindowState.PRESENTATION,
+ }:
+ self.interface.on_show()
+ elif (
+ previous_state
+ in {
+ WindowState.NORMAL,
+ WindowState.MAXIMIZED,
+ WindowState.FULLSCREEN,
+ WindowState.PRESENTATION,
+ }
+ and current_state == WindowState.MINIMIZED
+ ):
+ self.interface.on_hide()
+ return result
def winforms_Resize(self, sender, event):
if self.native.WindowState != WinForms.FormWindowState.Minimized:
But, I found through testing that doing custom work in WndProc()
slows down rendering of the window due to excessive redraws. The only way to prevent unnecessary redraws of the window is by putting it to sleep for sometime: https://stackoverflow.com/a/4429310. However, since you have earlier warned me about putting delay in production code, therefore I have used _is_previously_shown
flag to keep track and trigger the events at appropriate times.
def test_on_gain_focus(window): | ||
assert window._on_gain_focus._raw is None | ||
|
||
on_gain_focus_handler = Mock() | ||
window.on_gain_focus = on_gain_focus_handler | ||
|
||
assert window.on_gain_focus._raw == on_gain_focus_handler | ||
|
||
window._impl.simulate_on_gain_focus() | ||
|
||
on_gain_focus_handler.assert_called_once_with(window) | ||
|
||
|
||
def test_on_lose_focus(window): | ||
assert window.on_lose_focus._raw is None | ||
|
||
on_lose_focus_handler = Mock() | ||
window.on_lose_focus = on_lose_focus_handler | ||
|
||
assert window.on_lose_focus._raw == on_lose_focus_handler | ||
|
||
window._impl.simulate_on_lose_focus() | ||
|
||
on_lose_focus_handler.assert_called_once_with(window) | ||
|
||
|
||
def test_on_show(window): | ||
assert window.on_show._raw is None | ||
|
||
on_show_handler = Mock() | ||
window.on_show = on_show_handler | ||
|
||
assert window.on_show._raw == on_show_handler | ||
|
||
window._impl.simulate_on_show() | ||
|
||
on_show_handler.assert_called_once_with(window) | ||
|
||
|
||
def test_on_hide(window): | ||
assert window.on_hide._raw is None | ||
|
||
on_hide_handler = Mock() | ||
window.on_hide = on_hide_handler | ||
|
||
assert window.on_hide._raw == on_hide_handler | ||
|
||
window._impl.simulate_on_hide() | ||
|
||
on_hide_handler.assert_called_once_with(window) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had written these core tests when there was no API for window states. Currently, I have added new tests that test the behavior of triggering the events with changes in window state. So, should I keep these tests?
def simulate_on_gain_focus(self): | ||
self.interface.on_gain_focus() | ||
|
||
def simulate_on_lose_focus(self): | ||
self.interface.on_lose_focus() | ||
|
||
def simulate_on_show(self): | ||
self.interface.on_show() | ||
|
||
def simulate_on_hide(self): | ||
self.interface.on_hide() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are only required for the previously written core tests: https://github.com/beeware/toga/pull/2096/files#r1900451951
Implements the APIs described in #2009.
on_gain_focus
,on_lose_focus
,on_show
&on_hide
handles are available both as properties and also as initialization parameters intoga.Window
.Only tested on
WinForms
andgtk
. This will take sometime to complete for all backends.Fixes #2009
PR Checklist: