toga icon indicating copy to clipboard operation
toga copied to clipboard

Added `on_gain_focus`, `on_lose_focus`, `on_show` & `on_hide` handlers on `toga.Window`

Open proneon267 opened this issue 2 years ago • 38 comments

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 in toga.Window.

Only tested on WinForms and gtk. This will take sometime to complete for all backends.

Fixes #2009

PR Checklist:

  • [x] All new features have been tested
  • [x] All new features have been documented
  • [x] I have read the CONTRIBUTING.md file
  • [x] I will abide by the code of conduct

proneon267 avatar Aug 23 '23 17:08 proneon267

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.

proneon267 avatar Aug 24 '23 00:08 proneon267

@mhsmith I need your guidance on android side. According to: https://developer.android.com/guide/components/activities/intro-activities#onpause

The system calls onPause() when the activity loses focus and enters a Paused state. This state occurs when, for example, the user taps the Back or Recents button.

On the Emulator(Android 12):

When starting the app, the following events are triggered: onCreate()->onStart()->onResume() . But, when I press either the Back or Recents button, onPause() event is not triggered. No other events are triggered.

When I select the app by pressing the Recents button, only onStart()->onResume() are triggered. onRestart() is not triggered.

D/MainActivity: onStart() start
I/python.stdout: Toga app: onStart
D/MainActivity: onStart() complete
D/MainActivity: onResume() start
I/python.stdout: Toga app: onResume
D/MainActivity: onResume() complete
I/OpenGLRenderer: Davey! duration=18074ms; Flags=1, FrameTimelineVsyncId=10773, IntendedVsync=2419137327998, Vsync=2419137327998, InputEventId=0, HandleInputStart=2419139817470, AnimationStart=2419139847710, PerformTraversalsStart=2419139888290, DrawStart=2419223580770, FrameDeadline=2419170661330, FrameInterval=2419139788630, FrameStartTime=16666666, SyncQueued=2419225272400, SyncStart=2419226783620, IssueDrawCommandsStart=2419227470230, SwapBuffers=2419249275450, FrameCompleted=2437213784760, DequeueBufferDuration=21300, QueueBufferDuration=360800, GpuCompleted=2437213784760, SwapBuffersCompleted=2419251729810, DisplayPresentTime=0,

When I press the home button, neither onPause() or onStop() events are triggered. No other events are triggered.

On a Physical Device(Android 13):

When starting the app, the following events are triggered: onCreate()->onStart()->onResume() . But, when I press either the Back or Recents button, onPause() event is not triggered. Instead the following events are triggered:

D/VRI[MainActivity]: onFocusEvent false
D/VRI[MainActivity]: dispatchAppVisibility visible:false
D/BufferQueueProducer: [VRI[MainActivity]#0(BLAST Consumer)0](id:55c400000000,api:1,p:21956,c:21956) disconnect: api 1
D/BufferQueueConsumer: [VRI[MainActivity]#0(BLAST Consumer)0](id:55c400000000,api:0,p:-1,c:21956) disconnect
D/VRI[MainActivity]: setWindowStopped stopped:true
D/ActivityThread: do gfx trim 40 success

When I select the app by pressing the Recents button, only onStart()->onResume() are triggered. onRestart() is not triggered. Instead the following events are triggered:

D/VRI[MainActivity]: dispatchAppVisibility visible:true
D/VRI[MainActivity]: setWindowStopped stopped:false
D/MainActivity: onStart() start
I/python.stdout: Toga app: onStart
D/MainActivity: onStart() complete
D/MainActivity: onResume() start
I/python.stdout: Toga app: onResume
D/MainActivity: onResume() complete
I/Quality : Skipped: false 0 cost 5.253646 refreshRate 8332212 bit true processName com.example.helloworld
D/BufferQueueConsumer: [](id:55c400000001,api:0,p:-1,c:21956) connect: controlledByApp=false
E/IPCThreadState: attemptIncStrongHandle(57): Not supported
D/BufferQueueProducer: [VRI[MainActivity]#1(BLAST Consumer)1](id:55c400000001,api:1,p:21956,c:21956) connect: api=1 producerControlledByApp=true
D/VRI[MainActivity]: registerCallbacksForSync syncBuffer=false
D/VRI[MainActivity]: Received frameCommittedCallback lastAttemptedDrawFrameNum=1 didProduceBuffer=true syncBuffer=false
D/VRI[MainActivity]: draw finished.
D/VRI[MainActivity]: onFocusEvent true

When I press the home button, neither onPause() or onStop() events are triggered. But, the following events are triggered:

D/VRI[MainActivity]: onFocusEvent false
D/VRI[MainActivity]: dispatchAppVisibility visible:false
D/BufferQueueProducer: [VRI[MainActivity]#0(BLAST Consumer)0](id:55c400000000,api:1,p:21956,c:21956) disconnect: api 1
D/BufferQueueConsumer: [VRI[MainActivity]#0(BLAST Consumer)0](id:55c400000000,api:0,p:-1,c:21956) disconnect
D/VRI[MainActivity]: setWindowStopped stopped:true
D/ActivityThread: do gfx trim 40 success

As, you can see, onFocusEvent, dispatchAppVisibility visible and setWindowStopped stopped show consistent behaviors.

Why are the documented Activity lifecycle events not being triggered as per the documentation?

proneon267 avatar Aug 25 '23 15:08 proneon267

Regarding the gain/lose focus on mobile platforms like android:

From my testing, the app will lose focus when either the Home or Recent App List buttons are pressed. On pressing the back button, the app also loses focus and sends the user to the home screen.

In split screen mode (like dual app mode), suppose there are two apps A and B. App A will gain focus when the user touches the A app's screen. The focus is lost when the user touches B app's screen or interacts with the system launcher or presses the Home or Recent App List Button.

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 UIApplicationDelegate, which has methods like: applicationDidBecomeActive and applicationWillResignActive

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, on_background/foreground to call the handler when the app is in background/foreground and not visible/ visible to the user.

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.

proneon267 avatar Aug 27 '23 06:08 proneon267

When I press the home button, neither onPause() or onStop() events are triggered.

That's because those methods aren't included in the Android template, either in IPythonApp or in MainActivity. They should be added to both places, but ideally with an emptydefault implementation in order not to break any third-party implementations of IPythonApp. And all the existing methods should become default as well.

In split screen mode (like dual app mode), suppose there are two apps A and B.

See this page for how this is notified on Android.

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).

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.

mhsmith avatar Aug 27 '23 12:08 mhsmith

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 on_background and on_foreground events are mostly needed for mobile platforms. But there is a need for distinction between [not visible state] and [visible but not receiving inputs state] of an app.

For example, the app should be put to a sleep mode(not updating the layout or text) when it is in background or [not visible state]. But when it is in a [visible but not receiving inputs state](like in a multi app screen mode), the app should update the layout or text, so that the user can get the latest updated information.

What do you think?

proneon267 avatar Aug 27 '23 14:08 proneon267

I have tested android implementation both on a physical device and on the emulator.

I have submitted a PR at https://github.com/beeware/briefcase-android-gradle-template/pull/69 so that the app focus event can be detected.

proneon267 avatar Aug 28 '23 02:08 proneon267

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.

proneon267 avatar Aug 28 '23 15:08 proneon267

Researching some more on the topic, it seems like we need 3 categories of events:

  • Input Focus -> [Receiving Inputs] or [Not Receiving Inputs]
  • Visibility -----> [Visible to User] or [Not Visible to User]
  • Hover

The following are the states associated with the event categories mentioned above and their implications for other event categories:

Input Focus ---> [Receiving Inputs]   |                               | ->[Visible to User]   |   |---------------> [Not Receiving Inputs]                                  | ->[Visible to User] or                                  | ->[Not Visible to User]

Visiblity -> [Visible to User]   |                      | ->[Receiving Inputs] or   |                      | ->[Not Receiving Inputs]   |   |---------> [Not Visible to User]                          | ->[Not Receiving Inputs]

Hover ->[Interacting with a Pointing Device or Mouse] & [Visible to User] & [Not Receiving Inputs]

Use Cases:

  • The use case for Input Focus might be to show a highlight effect indicating to the user that it is receiving input.

  • The use case for Visibility might be to:

    • Put the app to a sleep mode(not updating the layout or text) when it is [Not Visible to User].
    • Update the layout or text when it is [Visible to User], so that the user can get the latest information.
  • So far, I am not sure what the use case for Hover might be. Maybe showing effects that would imply to the user, clicking on it, will make it to receive input focus.

Who should have which event categories:

  • Toga.Window should have:
    • Input Focus: Since, multiple windows may be present, hence each window should trigger Input Focus event when they receive/lose input focus.
    • Visibility: Same reason as above
    • Hover: Same reason as above
  • Toga.App doesn't really need any of the event categories since it can just iterate through the Toga.App.windows list and add handler methods to the event handlers of the windows. But, we can add them if it would make intuitive sense for the user.

APIs are not much of a problem as the available platform APIs can be properly mapped onto the above described event categories.

proneon267 avatar Sep 01 '23 09:09 proneon267

Researching some more on the topic, it seems like we need 3 categories of events:

  • Input Focus -> [Receiving Inputs] or [Not Receiving Inputs]
  • Visibility -----> [Visible to User] or [Not Visible to User]
  • Hover

I'm not sure I understand what the practical manifestation of "hover" would be, beyond "cursor is inside an area of interest". In particular, I'm not sure how this definition:

Hover ->[Interacting with a Pointing Device or Mouse] & [Visible to User] & [Not Receiving Inputs]

could exist on a desktop platform - if you're not receiving inputs, how can you be interacting? And on a mobile platform - what does "interacting with a pointing device" even mean?

  • Toga.App doesn't really need any of the event categories since it can just iterate through the Toga.App.windows list and add handler methods to the event handlers of the windows. But, we can add them if it would make intuitive sense for the user.

That might be true of Windows and Linux, but it's not (necessarily) true of macOS. A macOS app can be running without having any windows, and still have focus. This is the default mode for a document-based app with no open documents.

But desktop platforms are the least of my concern - a focus event makes sense on those, if only because desktop platforms have clear focus events at the window level. The area where I'm not clear is how this maps to mobile app lifecycle. How do these events map to a device that can only have a single window visible at a time, with no pointer? What about platforms (like tablets) that allow 2 apps to be visible at the same time?

freakboy3742 avatar Sep 03 '23 22:09 freakboy3742

I agree with you, hover event is mouse based and there likely won't be a mouse on mobile platforms. The definition of hover doesn't make sense in this context as it shouldn't be a window event. It is more suitable for widgets.

It should be for a widget which is visible and not yet in input focus, but the mouse cursor is hovering over the widget. Hence, hover won't be a part of this API.

The following are the states associated with the event categories and their implications for other event categories:

Input Focus ---> [Receiving Inputs]   |                               | ->[Visible to User]   |   |---------------> [Not Receiving Inputs]                                  | ->[Visible to User] or                                  | ->[Not Visible to User]

Visiblity -> [Visible to User]   |                      | ->[Receiving Inputs] or   |                      | ->[Not Receiving Inputs]   |   |---------> [Not Visible to User]                          | ->[Not Receiving Inputs]

The following are the states associated with the app and their implications for the event categories for mobile platforms:

When the App is in:        |->Foreground => [Visible to User] & [Receiving Inputs]        |        |->Background => [Not Visible to User] & [Not Receiving Inputs]        |        |->Floating Window Mode        |              |->User is interacting with App => [Visible to User] & [Receiving Inputs]        |              |        |              |->User is Not interacting with App => [Visible to User] & [Not Receiving Inputs]        |        |->Split Screen Mode with apps A and B                      |->User is interacting with App A                      |                     |->App A => [Visible to User] & [Receiving Inputs]                      |                     |->App B => [Visible to User] & [Not Receiving Inputs]                      |                      |->User is interacting with App B                                           |->App A => [Visible to User] & [Not Receiving Inputs]                                           |->App B => [Visible to User] & [Receiving Inputs]

I agree with you that these events should also be present on the app class, as they would make much sense at the app level rather than at window level.

Hence, the event categories: Visibility and Input Focus would be present both at the app and window level.

proneon267 avatar Sep 04 '23 14:09 proneon267

A macOS app can be running without having any windows, and still have focus. This is the default mode for a document-based app with no open documents.

That's true, but I think the use case for detecting that would be even smaller than detecting focus changes in a window. So to simplify the situation, I suggest we limit this PR to just events on the Window.

The area where I'm not clear is how this maps to mobile app lifecycle. How do these events map to a device that can only have a single window visible at a time, with no pointer? What about platforms (like tablets) that allow 2 apps to be visible at the same time?

In split screen mode on Android, the "focused" state, which can be held by only one app at a time, is notified with the onTopResumedActivityChanged event, as discussed in https://github.com/beeware/briefcase-android-gradle-template/pull/69. I haven't tested it, but presumably this is the last app to be touched. Meanwhile, "visibility", which can be held by multiple apps, is notified with the onStart and onStop events.

So my proposal is that Window would have 4 handlers:

  • on_gain_focus
  • on_lose_focus
  • on_hide
  • on_show

On mobile platforms, on_hide / on_show would fire depending on whether the app is on screen. On desktop platforms, they would fire when the window enters or leaves the hidden or minimized states.

mhsmith avatar Sep 04 '23 18:09 mhsmith

Although on_hide / on_show could be confusing names if they don't correspond to the existing hide / show methods, so I'm open to other suggestions.

mhsmith avatar Sep 04 '23 19:09 mhsmith

A macOS app can be running without having any windows, and still have focus. This is the default mode for a document-based app with no open documents.

That's true, but I think the use case for detecting that would be even smaller than detecting focus changes in a window. So to simplify the situation, I suggest we limit this PR to just events on the Window.

That sounds reasonable to me. We can always revisit this in the future if it turns out there is a use case for app-level events (and/or a meaningful mapping becomes apparent).

The area where I'm not clear is how this maps to mobile app lifecycle. How do these events map to a device that can only have a single window visible at a time, with no pointer? What about platforms (like tablets) that allow 2 apps to be visible at the same time?

In split screen mode on Android, the "focused" state, which can be held by only one app at a time, is notified with the onTopResumedActivityChanged event, as discussed in beeware/briefcase-android-gradle-template#69. I haven't tested it, but presumably this is the last app to be touched. Meanwhile, "visibility", which can be held by multiple apps, is notified with the onStart and onStop events.

I've done some preliminary investigation into the situation on iOS; and the story is... complicated.

iOS allows for 4 modes of app execution:

  1. Full screen
  2. Split screen - a 50/50 split between two apps
  3. Slide over - on iPad, an app that supports this mode can be collapsed to an iPhone-like vertical window that overlays another app.
  4. Picture in Picture - video applications can overlay a small playback window.

Right now, we're using UIApplicationDelegate, which only allows the app to run in full screen mode. However, other apps can overlay in slide over or PiP mode. To support split screen or running as a slide-over, we'd need to modify toga.App to be a "Scene", and use UISceneDelegate rather than UIApplicationDelegate. I think Scenes would also allow the app to have multiple "windows", so that might be another reason to make this change.

However, both Scene and Application delegates have the concept of:

  • will enter foreground - app/scene will becoming visible
  • will/did become active - app/scene will start processing events (both as an "about to" and "is now" event)
  • will/did resign active - app/scene will not be processing events
  • did enter background - app/scene is no longer visible.

In the simple case of a UIApplicationDelegate app, you get a foreground event then will/did become active events in rapid succession when the app starts. When the app is sharing the screen with a slide-over, you don't lose active status when you type into the slide-over app window. There appears to be very limited situations when you lose Active status - receiving a phone call is the only common example I can think of.

So my proposal is that Window would have 4 handlers:

  • on_gain_focus
  • on_lose_focus
  • on_hide
  • on_show

On mobile platforms, on_hide / on_show would fire depending on whether the app is on screen. On desktop platforms, they would fire when the window enters or leaves the hidden or minimized states.

That definitely sounds like it could map to the iOS events - although the differentiation between gaining focus/show, and lose focus/hide will be pretty minimal.

Although on_hide / on_show could be confusing names if they don't correspond to the existing hide / show methods, so I'm open to other suggestions.

Would it make sense to borrow the mobile terminology of "active/inactive" or "foreground/background" here?

freakboy3742 avatar Sep 05 '23 02:09 freakboy3742

When the app is sharing the screen with a slide-over, you don't lose active status when you type into the slide-over app window.

It seems that using the provided UIAppDelegate events and cross checking our window.native instance with UIApplication.sharedApplication().keyWindow (https://developer.apple.com/documentation/uikit/uiapplication/1622924-keywindow?language=objc) would have helped here, but it has been deprecated.

So, we can use the provided UIAppDelegate events and cross check with: UIApplication.sharedApplication().applicationState(https://developer.apple.com/documentation/uikit/uiapplication/1623003-applicationstate?language=objc) which will give us an enum value of type: UIApplicationState(https://developer.apple.com/documentation/uikit/uiapplicationstate?language=objc)

When we will use Scenes and still then UISceneDelegate can't give accurate input focus event, then we can cross check with: UIWindowScene.keyWindow(https://developer.apple.com/documentation/uikit/uiwindowscene/3750932-keywindow?language=objc)

To support split screen or running as a slide-over, we'd need to modify toga.App to be a "Scene", and use UISceneDelegate rather than UIApplicationDelegate. I think Scenes would also allow the app to have multiple "windows", so that might be another reason to make this change.

I agree with you that it would be better to use Scenes.

Should this modification be included in this PR? or Should I implement this PR's APIs on the existing codebase and we can deal with the modifying Toga.App to use Scenes in another PR later on?

proneon267 avatar Sep 05 '23 10:09 proneon267

Although on_hide / on_show could be confusing names if they don't correspond to the existing hide / show methods, so I'm open to other suggestions.

Would it make sense to borrow the mobile terminology of "active/inactive" or "foreground/background" here?

For "is receiving input", on_gain_focus and on_lose_focus are the names already used by TextInput, and I think they work for this purpose as well.

For "visible", I think "active/inactive" and "foreground/background" don't really capture it. For example, on a desktop a partially-covered window could be said to be inactive or background, but it's still visible. And "hide/show" would be confusing because it doesn't mean the same as the existing methods of the same name, which I assume are intended to completely remove the window from existence as far as the user is concerned.

So how about on_visible and on_invisible? And then we clarify the definition of the existing visible property so that it's consistent with that, i.e. a minimized Windows window, a background Android app, or a hidden macOS app, would all set visible=False and generate an on_invisible event.

mhsmith avatar Sep 05 '23 19:09 mhsmith

I agree with you that it would be better to use Scenes.

Should this modification be included in this PR? or Should I implement this PR's APIs on the existing codebase and we can deal with the modifying Toga.App to use Scenes in another PR later on?

I think it would be better to do them in separate PRs.

mhsmith avatar Sep 05 '23 19:09 mhsmith

I think it would be better to do them in separate PRs.

Ok then, after we arrive at the final event handler names, I will implement the APIs on the existing codebase, without modifying to use Scenes. And in another PR, we can modify Toga.App to use Scenes.

proneon267 avatar Sep 06 '23 04:09 proneon267

Although on_hide / on_show could be confusing names if they don't correspond to the existing hide / show methods, so I'm open to other suggestions.

Would it make sense to borrow the mobile terminology of "active/inactive" or "foreground/background" here?

For "is receiving input", on_gain_focus and on_lose_focus are the names already used by TextInput, and I think they work for this purpose as well.

Agreed.

For "visible", I think "active/inactive" and "foreground/background" don't really capture it. For example, on a desktop a partially-covered window could be said to be inactive or background, but it's still visible. And "hide/show" would be confusing because it doesn't mean the same as the existing methods of the same name, which I assume are intended to completely remove the window from existence as far as the user is concerned.

So how about on_visible and on_invisible? And then we clarify the definition of the existing visible property so that it's consistent with that, i.e. a minimized Windows window, a background Android app, or a hidden macOS app, would all set visible=False and generate an on_invisible event.

I'm not sure that's much better. A fully obscured app window isn't "visible", but it would be visible. A minimized app is also "visible", in the sense that you can see a manifestation of the window, just not the full window contents.

I wonder if on_show and on_hide might be our best option. We have existing APIs with those names on Window; and invoking those APIs will trigger "show and "hide" events. We'd just be augmenting the events to also include show/hide actions invoked by UX interactions, such as minimisation or putting an app in the background.

The only other complication is that show() and hide() are effectively no-ops on mobile; we could argue whether calling hide() on mobile actually triggers an on_hide event, or if the platform interpretation is tied to the event lifecycle, independent of the actual API calls made. You're still going to get an on_show event when the app starts (because the window has a lifecycle event at startup); but hide() is a no-op, so it arguably won't trigger on_hide. However, I think that's in the territory of "explainable discrepancies between platforms" - fundamentally, on_show and on_hide are describing whether the window is present for the user to see.

freakboy3742 avatar Sep 07 '23 02:09 freakboy3742

The existing show()/hide() totally hides the window, like there will be no indication of the window, even on the taskbar.

In contrast, the proposed visibility event handlers (on_visible/on_invisible) would be triggered when the window will be minimised, hidden, or when the app will in "systray-only" mode.

As there is a clear difference between the two cases, hence on_visible/on_invisible event handler names would be much more preferable.

proneon267 avatar Sep 07 '23 03:09 proneon267

The existing show()/hide() totally hides the window, like there will be no indication of the window, even on the taskbar.

That may be true on Windows and Linux, but it isn't on macOS. The Dock indicator is an app property, not a window property.

In contrast, the proposed visibility event handlers (on_visible/on_invisible) would be triggered when the window will be minimised, hidden, or when the app will in "systray-only" mode.

As there is a clear difference between the two cases, hence on_visible/on_invisible event handler names would be much more preferable.

There's no difference at all, other than the name. The implementation of the visible property literally calls show() and hide()`.

In both cases, we're discussing raising an event whose name has additional significance on top of the literal API call that might be able to make that that event occur. I contend that on_show/on_hide is a better option because:

  • There are corresponding show()/hide() methods; there is no invisible().
  • Linguistically, "on show" and "on hide" make more sense because they're verbs; "visible" is an adjective, so it should really be "on become visible", not "on visible".
  • show and hide are the same length, so they'll line up when vertically aligned :-)

freakboy3742 avatar Sep 07 '23 04:09 freakboy3742

Apologies, perhaps I made it more confusing.

I meant that the events [Visible to User] and [Not Visible to User] will be triggered both when the window is minimized or hidden.

But being minimized and hidden are 2 different window states.

If we were to name the events [Visible to User] as on_show and [Not Visible to User] as on_hide, then the user will perceive that these events will be triggered only when the Toga.Window.hide()/Toga.Window.show() methods will be called.

But this will miss out the state when the window will be minimized.

Hence, I am suggesting a different name for the API.

proneon267 avatar Sep 07 '23 05:09 proneon267

If we were to name the events [Visible to User] as on_show and [Not Visible to User] as on_hide, then the user will perceive that these events will be triggered only when the Toga.Window.hide()/Toga.Window.show() methods will be called.

But this will miss out the state when the window will be minimized.

And I'm saying there is literally no difference between "visible/invisible" and "show/hide" from the perspective of Toga's existing API; so "visible" isn't an inherently better name than "show" for the functionality you're describing.

I'm also saying that while show() will programmatically show the window, and that it makes sense that call will generate an on_show event, there is nothing stopping us from raising an on_show event in response to other changes in the GUI - like restoring from minimisation.

freakboy3742 avatar Sep 07 '23 07:09 freakboy3742

from the perspective of Toga's existing API; so "visible" isn't an inherently better name than "show" for the functionality you're describing.

I agree with you.

We can trigger an on_hide event both when hide() method is invoked and when other GUI changes like window minimization occur.

Malcolm would you also confirm this?

proneon267 avatar Sep 07 '23 09:09 proneon267

@freakboy3742 Can I please implement this PR first and then write tests for both #1930 and #2096?

proneon267 avatar Sep 09 '23 06:09 proneon267

As I indicated on #2108, preference would be to have one large piece of work in flight at a time.

freakboy3742 avatar Sep 10 '23 00:09 freakboy3742

And I'm saying there is literally no difference between "visible/invisible" and "show/hide" from the perspective of Toga's existing API; so "visible" isn't an inherently better name than "show" for the functionality you're describing.

OK, I wasn't aware that visible was a writable property which would simply call show and hide. This should be mentioned in the documentation in #2058.

In that case, I agree that on_show and on_hide are the better event names. And logically, if un-minimizing a window causes an on_show event, then calling show on an minimized window should un-minimize it.

The only other complication is that show() and hide() are effectively no-ops on mobile; we could argue whether calling hide() on mobile actually triggers an on_hide event, or if the platform interpretation is tied to the event lifecycle, independent of the actual API calls made. You're still going to get an on_show event when the app starts (because the window has a lifecycle event at startup); but hide() is a no-op, so it arguably won't trigger on_hide. However, I think that's in the territory of "explainable discrepancies between platforms" - fundamentally, on_show and on_hide are describing whether the window is present for the user to see.

Yes, so if the platform can't actually perform the action, I think it's better not to call the event, but to log a warning instead.

Although FYI, on Android an app actually can hide itself (one of my highest-scoring StackOverflow answers), but once hidden, only the user can show it again.

mhsmith avatar Sep 10 '23 18:09 mhsmith

In that case, I agree that on_show and on_hide are the better event names. And logically, if un-minimizing a window causes an on_show event, then calling show on an minimized window should un-minimize it.

👍

Although FYI, on Android an app actually can hide itself (one of my highest-scoring StackOverflow answers), but once hidden, only the user can show it again.

Good to know; I'm not sure if that's a better mapping for "minimize" or "hide" (plus, it's being called on the Window, not the App), but it's good to know we have options.

freakboy3742 avatar Sep 11 '23 03:09 freakboy3742

I have added the on_show and on_hide event handlers on the WinForms backend.

Only tested on WinForms. This will take some time to complete for all backends.

proneon267 avatar Sep 12 '23 15:09 proneon267

Completed implementation on all platforms. Tested on WinForms, GTK, Android. A test is present in the window example app.

On MacOS, can you confirm which event is triggered when NSWindow.orderOut() is invoked? I think, windowDidResignKey_(https://developer.apple.com/documentation/appkit/nswindowdelegate/1419711-windowdidresignkey?language=objc) would be triggered.

Also, on the web backend, the events are not being triggered. I am not sure what might be the reason.

proneon267 avatar Sep 14 '23 13:09 proneon267

I honestly agree with you about not providing non tested implementations. Developing for cocoa and iOS feels like guesswork. All I have are apple API documentations and the existing codebase to best guess if an implementation is going to work on cocoa and iOS. As with any of my previous PRs like #1930, the implementations for apple platforms are successful, only because you helped to test them and correct their implementations.

I have access to 3 platforms: Windows, GTK & Android. Developing for them is easier as I can test and reiterate on the implementations much faster. For any future PRs, I will not be providing feature complete implementations for cocoa and iOS, instead will only provide no-op implementations on them.

I will maintain my currently opened PRs with the latest main branch, until they are reviewed and merged.

As you had warned earlier, this PR would be needed to rebase with #2075 and #2058.

However, due to my current semester exams, I won't be able to give time here for a couple of weeks. But, as I have mentioned earlier, I'll complete all of my opened PRs, any related issue and other PRs or designs, albeit only after my exams.

During my exams, I hope both #2075 and #2058 would be merged. My plan after the exams is to write tests for both #1930 (Screen API) and #2096 (Window event handler API). After #1930 is merged, then I will continue with our window state discussion #1884 and complete window state API implementation. After, all these PRs are over, I will continue with StatusIcon design & implementation #2108, and then with Notification design & implementation #1890.

Thank you @freakboy3742. I couldn't have done this without you. proneon267

proneon267 avatar Sep 15 '23 04:09 proneon267