Skip to content
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

Open
wants to merge 64 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
64 commits
Select commit Hold shift + click to select a range
6e2eda0
Added support for `WinForms` and `gtk`.
proneon267 Aug 23, 2023
4bb2a42
Added changelog.
proneon267 Aug 23, 2023
3a202e4
Added support for `cocoa`.
proneon267 Aug 24, 2023
9cecb4a
Added support for `web`
proneon267 Aug 24, 2023
60a3cac
Merge branch 'beeware:main' into patch-12
proneon267 Aug 24, 2023
87ea0a4
Merge branch 'beeware:main' into patch-12
proneon267 Aug 25, 2023
6f38ae2
Added support for `android`.
proneon267 Aug 28, 2023
6dce43e
Merge branch 'patch-12' of https://github.com/proneon267/toga into pa…
proneon267 Aug 28, 2023
3133dfb
Merge branch 'beeware:main' into patch-12
proneon267 Aug 28, 2023
3b03ec4
Coreected `cocoa` implementation.
proneon267 Aug 28, 2023
d746ba8
Merge branch 'patch-12' of https://github.com/proneon267/toga into pa…
proneon267 Aug 28, 2023
096e6bf
Re: `cocoa` implementation correction.
proneon267 Aug 28, 2023
943612d
Added support for `iOS`.
proneon267 Aug 28, 2023
7b864fe
Fixed `winforms` implementation.
proneon267 Aug 28, 2023
88c56b9
Added a test in window example app.
proneon267 Aug 28, 2023
156bc9e
Corrected `Android` implementation.
proneon267 Aug 30, 2023
aa4714e
Merge branch 'beeware:main' into patch-12
proneon267 Aug 30, 2023
6895d8d
Merge branch 'beeware:main' into patch-12
proneon267 Sep 4, 2023
27bf2a7
Merge branch 'beeware:main' into patch-12
proneon267 Sep 11, 2023
9fabc3e
Merge branch 'beeware:main' into patch-12
proneon267 Sep 12, 2023
7b23209
Added `on_show` & `on_hide` handlers on winforms.
proneon267 Sep 12, 2023
0427a8f
Fixed overlapping event triggers on winforms.
proneon267 Sep 12, 2023
b9e0412
Added `on_show` & `on_hide` handlers on gtk.
proneon267 Sep 13, 2023
7f7468d
Added `on_show()` & `on_hide()` handlers on cocoa.
proneon267 Sep 14, 2023
429568b
Added `on_show()` & `on_hide()` on iOS.
proneon267 Sep 14, 2023
3440221
Corrected iOS implementation.
proneon267 Sep 14, 2023
fc8a29c
Added `on_show` & `on_hide` handlers on Android.
proneon267 Sep 14, 2023
6fd5e3c
Added `on_show` & `on_hide` on web.
proneon267 Sep 14, 2023
44bade9
Merge branch 'beeware:main' into patch-12
proneon267 Sep 14, 2023
8604faa
Modified as per suggestions.
proneon267 Sep 15, 2023
84234e8
Merge branch 'patch-12' of https://github.com/proneon267/toga into pa…
proneon267 Sep 15, 2023
eaeaf5d
Added comment for android version exclusion.
proneon267 Sep 15, 2023
ea28017
Merge branch 'main' into patch-12
proneon267 Nov 18, 2023
894a89c
Rebasing on the latest main branch
proneon267 Nov 18, 2023
cb2e541
Miscellaneous Fixes
proneon267 Nov 18, 2023
f7a014b
Added Core Tests
proneon267 Nov 18, 2023
0a1d093
Miscellaneous Fixes
proneon267 Nov 18, 2023
c441e77
Added tests for windows testbed
proneon267 Nov 19, 2023
059ea14
Fixed event triggers on gtk, cocoa
proneon267 Nov 19, 2023
81dd316
Fixed event triggers on Android
proneon267 Nov 19, 2023
d958394
Merge branch 'beeware:main' into patch-12
proneon267 Nov 19, 2023
d94a69a
Fixed cocoa implementation
Nov 19, 2023
8c364a1
Empty commit for CI
proneon267 Nov 19, 2023
c0405c3
Miscellaneous Fixes
proneon267 Nov 19, 2023
927b3d4
Fixed iOS implementation
proneon267 Nov 19, 2023
869da6a
Miscellaneous Fixes
proneon267 Nov 20, 2023
2423548
Miscellaneous Fixes
proneon267 Nov 20, 2023
9fd0e0e
Update changes/2009.feature.rst
proneon267 Nov 20, 2023
77b8822
Miscellaneous Fixes
proneon267 Nov 20, 2023
89f095d
Merge branch 'patch-12' of https://github.com/proneon267/toga into pa…
proneon267 Nov 20, 2023
69f90d5
Miscellaneous Fixes
proneon267 Nov 20, 2023
d5e8e3f
Merge branch 'beeware:main' into patch-12
proneon267 Dec 2, 2023
52e75b8
Merge branch 'beeware:main' into patch-12
proneon267 Dec 10, 2023
dc8b52d
Merge branch 'beeware:main' into patch-12
proneon267 Dec 17, 2023
004a98a
Merge branch 'beeware:main' into patch-12
proneon267 Dec 19, 2023
2fa2bfa
Merge branch 'beeware:main' into patch-12
proneon267 Dec 22, 2023
246eeb0
Merge branch 'beeware:main' into patch-12
proneon267 Jan 13, 2024
673e2cd
Merge branch 'beeware:main' into patch-12
proneon267 Jan 14, 2024
5cf9bbe
Merge branch 'beeware:main' into patch-12
proneon267 Jan 17, 2024
bfe62fa
Merge branch 'beeware:main' into patch-12
proneon267 Jan 17, 2024
e65c528
Merge branch 'beeware:main' into patch-12
proneon267 Jan 20, 2024
5273e54
Merge branch 'beeware:main' into patch-12
proneon267 Jan 25, 2024
69f3f25
Merge branch 'main' into patch-12
proneon267 Feb 3, 2024
30cb114
Merge branch 'beeware:main' into patch-12
proneon267 Feb 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 32 additions & 7 deletions android/src/toga_android/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

from android.graphics.drawable import BitmapDrawable
from android.media import RingtoneManager
from android.os import Build
from android.view import Menu, MenuItem
from java import dynamic_proxy
from org.beeware.android import IPythonApp, MainActivity
Expand Down Expand Up @@ -36,22 +37,46 @@ def onCreate(self):

def onStart(self):
print("Toga app: onStart")
for window in self._impl.interface.windows:
window.on_show()

def onResume(self):
def onResume(self): # pragma: no cover
print("Toga app: onResume")

def onPause(self):
print("Toga app: onPause") # pragma: no cover

def onStop(self):
print("Toga app: onStop") # pragma: no cover
# onTopResumedActivityChanged is not available on android versions less
# than Q. onResume is the best indicator for the gain input focus event.
# https://developer.android.com/reference/android/app/Activity#onWindowFocusChanged(boolean):~:text=If%20the%20intent,the%20best%20indicator.
if Build.VERSION.SDK_INT < Build.VERSION_CODES.Q:
proneon267 marked this conversation as resolved.
Show resolved Hide resolved
for window in self._impl.interface.windows:
window.on_gain_focus()

def onPause(self): # pragma: no cover
print("Toga app: onPause")
# onTopResumedActivityChanged is not available on android versions less
# than Q. onPause is the best indicator for the lost input focus event.
if Build.VERSION.SDK_INT < Build.VERSION_CODES.Q:
for window in self._impl.interface.windows:
window.on_lose_focus()

def onStop(self): # pragma: no cover
print("Toga app: onStop")
for window in self._impl.interface.windows:
window.on_hide()

def onDestroy(self):
print("Toga app: onDestroy") # pragma: no cover

def onRestart(self):
print("Toga app: onRestart") # pragma: no cover

def onTopResumedActivityChanged(self, isTopResumedActivity): # pragma: no cover
print("Toga app: onTopResumedActivityChanged")
if isTopResumedActivity:
for window in self._impl.interface.windows:
window.on_gain_focus()
else:
for window in self._impl.interface.windows:
window.on_lose_focus()

# TODO #1798: document and test this somehow
def onActivityResult(self, requestCode, resultCode, resultData): # pragma: no cover
"""Callback method, called from MainActivity when an Intent ends.
Expand Down
1 change: 1 addition & 0 deletions changes/2009.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Added `on_gain_focus` and `on_lose_focus` handlers both on the `toga.App` and `toga.Window`.
proneon267 marked this conversation as resolved.
Show resolved Hide resolved
58 changes: 58 additions & 0 deletions cocoa/src/toga_cocoa/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,60 @@ def windowDidResize_(self, notification) -> None:
# Set the window to the new size
self.interface.content.refresh()

@objc_method
def windowDidBecomeMain_(self, notification):
self.impl.interface.on_gain_focus()

@objc_method
def windowDidResignMain_(self, notification):
self.impl.interface.on_lose_focus()

@objc_method
def windowDidBecomeKey_(self, notification):
if bool(self.impl.native.isVisible) and not self.impl._is_previously_shown:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line of repeated logic doesn't seem entirely right. You're adding a stateful variable is_previously_shown; but it's not clear to me that the value of this variable can't be derived from other properties. You're also checking the value of is_visible in a number of cases where it's not clear that you'd be able to be invisible (how do you deminaturize a window that isn't visible?)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do you deminaturize a window that isn't visible?

Not visible to the user refers to window states like minimized, hidden, etc. So, this checks for cases where the window was in a minimized state(Not visible to the user) to a maximized state(Visible to the user).
Same logic as: #2096 (comment)

self.impl._is_previously_shown = True
self.impl.interface.on_show()

@objc_method
def windowDidMiniaturize_(self, notification): # pragma: no cover
if not bool(self.impl.native.isVisible) and self.impl._is_previously_shown:
self.impl._is_previously_shown = False
self.impl.interface.on_hide()

@objc_method
def windowDidDeminiaturize_(self, notification): # pragma: no cover
if bool(self.impl.native.isVisible) and not self.impl._is_previously_shown:
self.impl._is_previously_shown = True
self.impl.interface.on_show()

@objc_method
def windowDidEnterFullScreen_(self, notification): # pragma: no cover
if bool(self.impl.native.isVisible) and not self.impl._is_previously_shown:
self.impl._is_previously_shown = True
self.impl.interface.on_show()

@objc_method
def windowDidExitFullScreen_(self, notification): # pragma: no cover
if bool(self.impl.native.isVisible) and not self.impl._is_previously_shown:
self.impl._is_previously_shown = True
self.impl.interface.on_show()
Comment on lines +66 to +76
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two strike me as either unnecessary, or incomplete. I didn't think you could make a hidden window full screen (in which case, they're unnecessary); if you can, then what happens when you exit full screen? Does it return to the hidden state? If it returns to a visible state, then how is the window not is_visible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same logic as #2096 (comment)
Not visible to the user refers to window states like minimized, hidden, etc. So, these check for cases where the window goes to fullscreen(Visible to the user) from a minimized state(Not Visible to the user).


# when the user clicks the zoom button to unzoom a window
@objc_method
def windowWillUseStandardFrame_defaultFrame_(
self, window, defaultFrame
): # pragma: no cover
if bool(self.impl.native.isVisible) and not self.impl._is_previously_shown:
self.impl._is_previously_shown = True
self.impl.interface.on_show()

# when the user clicks the zoom button to zoom a window
@objc_method
def windowShouldZoom_toFrame_(self, window, toFrame): # pragma: no cover
if bool(self.impl.native.isVisible) and not self.impl._is_previously_shown:
self.impl._is_previously_shown = True
self.impl.interface.on_show()
Comment on lines +78 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I following why these 2 are necessary. A window has to be visible in order for a user to click on the zoom/maximize button - so it must already be shown.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had added self.impl._is_previously_shown because minimizing a window is same as losing focus to the native OS.

But from the API viewpoint, minimizing a window should mean that it is not visible to the user and also it has lost focus.

isVisible would be True even when the window is minimized. But as per the use case of the API, the window contents are practically not visible to the user and the app can pause updating its contents.

These two cases are necessary as the window can be maximized(making the window visible to the user) from a minimized state(not visible to the user).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - but that just reinforces that the issue is the absence of a window state API. You've added an internal state variable to work around the fact that you can't ask a window if it's minimized. The right approach here isn't the add a workaround - you fix the underlying problem.

However, more generally, my comment still stands. This particular endpoint is invoked when a macOS window goes "full screen" as a result of hitting the "maximise" button. That requires that the window is on the screen, at some size, with its title bar visible. You can't click a maximise button that isn't there. So the window must already be in a shown state. Similar, if it's in a maximized state, it must be on the screen, with a title bar visible.

So why are these two handlers required? What's the edge case where a window goes from a "not shown" state to a "shown" state as a result of being maximized.

Copy link
Contributor Author

@proneon267 proneon267 Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the edge case where a window goes from a "not shown" state to a "shown" state as a result of being maximized.

The one edge case I had in mind was if the window was made full screen(Visible to user) from a minimized state(Not Visible to user) through API. In this case, the window doesn't needs to be in a visible to user state for it become full screen.

I agree a window state API would be better. I will start working on it as soon as the other dependent PRs are completed.


######################################################################
# Toolbar delegate methods
######################################################################
Expand Down Expand Up @@ -122,6 +176,8 @@ def __init__(self, interface, title, position, size):
self.interface = interface
self.interface._impl = self

self._is_previously_shown = False

mask = NSWindowStyleMask.Titled
if self.interface.closable:
mask |= NSWindowStyleMask.Closable
Expand Down Expand Up @@ -278,9 +334,11 @@ def set_app(self, app):

def show(self):
self.native.makeKeyAndOrderFront(None)
self.interface.on_show()

def hide(self):
self.native.orderOut(self.native)
self.interface.on_hide()

def get_visible(self):
return bool(self.native.isVisible)
Expand Down
8 changes: 8 additions & 0 deletions core/src/toga/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,10 @@ def __init__(
size: tuple[int, int] = (640, 480),
resizable: bool = True,
minimizable: bool = True,
on_gain_focus: callable | None = None,
on_lose_focus: callable | None = None,
on_show: callable | None = None,
on_hide: callable | None = None,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've been moving away from generic callable type annotations toward specific Protocol-based definitions - see OnExitHandler for an example.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. Thanks!

resizeable=None, # DEPRECATED
closeable=None, # DEPRECATED
):
Expand All @@ -164,6 +168,10 @@ def __init__(
resizable=resizable,
closable=True,
minimizable=minimizable,
on_gain_focus=on_gain_focus,
on_lose_focus=on_lose_focus,
on_show=on_show,
on_hide=on_hide,
# Deprecated arguments
resizeable=resizeable,
closeable=closeable,
Expand Down
41 changes: 41 additions & 0 deletions core/src/toga/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,10 @@ def __init__(
closable: bool = True,
minimizable: bool = True,
on_close: OnCloseHandler | None = None,
on_gain_focus: callable | None = None,
on_lose_focus: callable | None = None,
on_show: callable | None = None,
on_hide: callable | None = None,
resizeable=None, # DEPRECATED
closeable=None, # DEPRECATED
) -> None:
Expand Down Expand Up @@ -143,6 +147,11 @@ def __init__(

self.on_close = on_close

self.on_gain_focus = on_gain_focus
self.on_lose_focus = on_lose_focus
self.on_show = on_show
self.on_hide = on_hide

@property
def id(self) -> str:
"""A unique identifier for the window."""
Expand Down Expand Up @@ -335,6 +344,38 @@ def as_image(self) -> Image:
:returns: A :class:`toga.Image` containing the window content."""
return Image(data=self._impl.get_image_data())

@property
def on_gain_focus(self) -> callable:
return self._on_gain_focus

@on_gain_focus.setter
def on_gain_focus(self, handler):
self._on_gain_focus = wrapped_handler(self, handler)

@property
def on_lose_focus(self) -> callable:
return self._on_lose_focus

@on_lose_focus.setter
def on_lose_focus(self, handler):
self._on_lose_focus = wrapped_handler(self, handler)

@property
def on_show(self) -> callable:
return self._on_show

@on_show.setter
def on_show(self, handler):
self._on_show = wrapped_handler(self, handler)

@property
def on_hide(self) -> callable:
return self._on_hide

@on_hide.setter
def on_hide(self, handler):
self._on_hide = wrapped_handler(self, handler)

############################################################
# Dialogs
############################################################
Expand Down
52 changes: 52 additions & 0 deletions core/tests/test_window.py
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,58 @@ def test_as_image(window):
assert image.data == b"pretend this is PNG image data"


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()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't strike me as a very useful test. Firstly, the initial on_show event occurs when you call window.show(); subsequent events aren't the result of a notional "show" event, but the result of being minimized, made full screen etc. That is the behaviour that needs to be tested.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this would be better tested with a window state API in place.


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)


def test_info_dialog(window, app):
"""An info dialog can be shown"""
on_result_handler = Mock()
Expand Down
12 changes: 12 additions & 0 deletions dummy/src/toga_dummy/window.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,3 +99,15 @@ def set_full_screen(self, is_full_screen):

def simulate_close(self):
self.interface.on_close()

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()
29 changes: 28 additions & 1 deletion examples/window/window/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -122,17 +122,42 @@ def close_handler(self, window, **kwargs):
return False
return True

def on_window_gain_focus(self, widget, **kwargs):
self.window_focus_label.text = "MainWindow is in focus"
print("MainWindow is in focus")

def on_window_lose_focus(self, widget, **kwargs):
self.window_focus_label.text = "MainWindow is not in focus"
print("MainWindow is not in focus")

def on_window_show(self, widget, **kwargs):
self.window_visible_label.text = "MainWindow is visible"
print("MainWindow is visible")

def on_window_hide(self, widget, **kwargs):
self.window_visible_label.text = "MainWindow is not visible"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Take a second to think this one through. Under what conditions will you be able to see this label? :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-) That's why the visibility state is also printed to stdout. The only place where it's visible is when you preview the minimized window without unminimizing it(by placing the mouse on the minimized app icon in taskbar on windows).

print("MainWindow is not visible")

def startup(self):
# Track in-app closes
self.close_count = 0

# Set up main window
self.main_window = toga.MainWindow()
self.main_window = toga.MainWindow(
on_gain_focus=self.on_window_gain_focus,
on_lose_focus=self.on_window_lose_focus,
on_show=self.on_window_show,
on_hide=self.on_window_hide,
)

self.on_exit = self.exit_handler

# Label to show responses.
self.label = toga.Label("Ready.")

self.window_focus_label = toga.Label("Window focus status")
self.window_visible_label = toga.Label("Window visible status")

# Buttons
btn_style = Pack(flex=1, padding=5)
btn_do_origin = toga.Button(
Expand Down Expand Up @@ -180,6 +205,8 @@ def startup(self):
self.inner_box = toga.Box(
children=[
self.label,
self.window_focus_label,
self.window_visible_label,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why multiple labels? We just need to display some text to say an event occurred; we don't need multiple lines of text for each independent signal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But some window focus & window visibility events would overlap at the same time, and we won't be able to tell if the event occurred, as the singular label updated by 1st event, would be instantly updated by the overlapping 2nd event.

btn_do_origin,
btn_do_left,
btn_do_right,
Expand Down
Loading