Add window states API

[proneon267]

Fixes #1857

Design discussion: #1884

The following are the API changes:

On the interface:

toga.constants.WindowState
WindowState.NORMAL
WindowState.MAXIMIZED
WindowState.MINIMIZED
WindowState.FULLSCREEN
WindowState.PRESENTATION

toga.Window
Added	toga.Window.state(getter)
	toga.Window.state(setter)
Deprecated	toga.Window.full_screen(getter)
	toga.Window.full_screen(setter)

toga.App
Added	toga.App.is_in_presentation_mode(getter)
	toga.App.enter_presentation_mode()
	toga.App.exit_presentation_mode()
Deprecated	toga.App.is_full_screen(getter)
	toga.App.set_full_screen()
	toga.App.exit_full_screen()

On the backend:

toga.Window
Added	get_window_state()
	set_window_state()

toga.App
Added	enter_presentation_mode()
	exit_presentation_mode()

However, I did encounter some issues, which I have put as inline comments. I do have some other questions about rubicon for implementing the new API, but I will post them later on.

TODO: Write and modify documentation, fix issues with tests, implement for the other remaining platforms, etc.

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]

@freakboy3742 Can you also review this, when you are free?

[freakboy3742]

Unless there's a design or implementation issue that requires feedback, I generally don't review code until it's passing CI - this is currently failing coverage and Android tests, and I'm going to guess iOS tests as well (although #2476 is likely masking that problem).

Is there a specific design or implementation issue where you're seeking feedback prior to getting the tests to pass?

[proneon267]

I needed some guide regarding the Android testbed, specifically test_app.py::test_full_screen and test_presentation_mode. I explained the issue with inline comments on test_app.py.

[freakboy3742]

I needed some guide regarding the Android testbed, specifically test_app.py::test_full_screen and test_presentation_mode. I explained the issue with inline comments on test_app.py.

Well - I guess my first question is why is there anything to test at all? Mobile apps don't have any concept of "full screen" or "maximized"... even the representation of "window" is pretty loose. What behaviour are you implementing here?

Beyond that - if a test passes in isolation, but not as part of the suite, that usually indicates that one of the previous tests has left the test suite in an inconsistent state. A common example in the window tests is when a test leaves an open window (sometimes due to a test failing); subsequent checks of the window count then fail.

[proneon267]

Well - I guess my first question is why is there anything to test at all? Mobile apps don't have any concept of "full screen" or "maximized"... even the representation of "window" is pretty loose. What behaviour are you implementing here?

Mostly Fullscreen and presentation mode where navigation bar, title bar& menu bars remain hidden.

Beyond that - if a test passes in isolation, but not as part of the suite, that usually indicates that one of the previous tests has left the test suite in an inconsistent state. A common example in the window tests is when a test leaves an open window (sometimes due to a test failing); subsequent checks of the window count then fail.

Yes, I realize that, in this case, the test seems to fail as they don't exit the presentation mode after testing. I thought maybe adding a delay to the redraw method would work, but it doesn't. The tests only pass when I set the window state directly on the window object or run the test in isolation.

I have also tried to identify any problems with the app interface but didn't find any. The same test logic is run in test_window, but there it works properly. The app implementation of presentation mode calls the window implementation of presentation mode. So, the behaviour should be identical.

[proneon267]

@freakboy3742 Also could you also please take a quick peek at the mobile platforms tests on testbed of test_app.py::test_full_screen and test_presentation mode. Their implementation is identical to that of test_window.py::test_presentation_state, since the app APIs call into the window API endpoints.

[freakboy3742]

@freakboy3742 Also could you also please take a quick peek at the mobile platforms tests on testbed of test_app.py::test_full_screen and test_presentation mode. Their implementation is identical to that of test_window.py::test_presentation_state, since the app APIs call into the window API endpoints.

I have, and I've already told you the general class of problem you're hitting. You've even hinted at the problem yourself:

Yes, I realize that, in this case, the test seems to fail as they don't exit the presentation mode after testing.

If a single test isn't leaving the app in the same state that it found it... well, that's the source of your problem. That's what you need to fix.

As for "the implementation is identical"... well, the golden rule of computers applies: if in doubt, the computer is right. The computer doesn't have opinions. It runs the code it has. If you think something is identical, but testing is producing inconsistent results... something about your implementation isn't identical.

You're the one proposing this PR; the onus is on you to solve problems with the implementation. If you've got a specific question about the way Toga or the testbed operates, then I can do what I can to provide an explanation, but I can only dedicate time to resolving an open ended "why doesn't it work?" questions when the problem is on my personal todo list - which usually means it's something on Toga's roadmap, or it's the source of a bug that is preventing people from using the published version of Toga.

[proneon267]

The coverage report complains about:

Name              Stmts   Miss Branch BrPart  Cover   Missing
-------------------------------------------------------------
src/toga/app.py     [35](https://github.com/beeware/toga/actions/runs/9203102559/job/25314158711#step:6:36)7      2     82      2  99.1%   846, 852
-------------------------------------------------------------
TOTAL              5067      2   12[38](https://github.com/beeware/toga/actions/runs/9203102559/job/25314158711#step:6:39)      2  99.9%

https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/src/toga/app.py#L846-L849 So, Line 846 is just @property. I first thought maybe the is_in_presentation_mode property is not being invoked in the test, but it is invoked several times in the tests: https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/tests/app/test_app.py#L468 https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/tests/app/test_app.py#L480

Next, it reports missing coverage for Line 852: https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/src/toga/app.py#L851-L854

So, line 852 is just self,. Here also, `enter_presentation_mode() is invoked serveral times during the tests: https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/tests/app/test_app.py#L479 https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/tests/app/test_app.py#L494

So, both of these reported missing coverage don't make much sense to me.

Also, the tests expectedly fail on python 3.13 due to Pillow.

[freakboy3742]

I can't work out what's going on with the coverage report in CI (perhaps a state cache somewhere?); but if I run the tests locally (tox -m test310), the lines in app.py that report as uncovered are 836 and 842, which corresponds nicely to to return conditions in set_full_screen():

https://github.com/beeware/toga/blob/06b7f2ab0911303b6dc09603c6a8b9715f275ddf/core/src/toga/app.py#L830-L843

[proneon267]

I can't work out what's going on with the coverage report in CI (perhaps a state cache somewhere?); but if I run the tests locally (tox -m test310), the lines in app.py that report as uncovered are 836 and 842, which corresponds nicely to to return conditions in set_full_screen():

Thanks! I was running the test locally with tox -e py; maybe that's why it wasn't showing the missing coverage. I have added the missing coverage and this PR is ready for a review.

[proneon267]

I need some time to prepare the widget list for #2484. In the meantime, you can review this PR whenever you are free. 😊

[freakboy3742]

I'm happy to provide feedback on a specific issue if you're stuck on something; but otherwise, passing CI (except for known issues like 3.13 compatibility) is a pre-requisite for a review - especially when the test failures are tests that the PR is adding itself.

[proneon267]

Apologies. I hadn't noticed the testbed tests were failing after updating to the latest main branch. I'll resolve the issues and request for a review. Thanks :)

[proneon267]

Quick question, has there been a major change on adding commands to window toolbars?

Looking at the sources: On the core: https://github.com/beeware/toga/blob/bf99b38e895dc2f4e3f43c26251e12e013130c30/core/src/toga/window.py#L946-L975 toolbar property is defined only in MainWindow On the backends: https://github.com/beeware/toga/blob/bf99b38e895dc2f4e3f43c26251e12e013130c30/winforms/src/toga_winforms/window.py#L215-L218 https://github.com/beeware/toga/blob/bf99b38e895dc2f4e3f43c26251e12e013130c30/gtk/src/toga_gtk/window.py#L186-L191 toolbar_native is only defined in MainWindow.

Also, looking at the docs: https://toga.readthedocs.io/en/latest/reference/api/mainwindow.html#toga.MainWindow.toolbar, toolbar property is only present in the MainWindow and not in Window class.

Is only the MainWindow now allowed to have toolbars?

[freakboy3742]

Quick question, has there been a major change on adding commands to window toolbars? ... Is only the MainWindow now allowed to have toolbars?

Yes. This change note was added as part of #2646. However, you can now instantiate more than one instance of MainWindow.

[proneon267]

I have resolved all of the testbed failures. Furthermore, the testbed tests now make use of additional displays(if available) to test presentation mode.

Also, a side question, why was the decision to make toolbars limited to MainWindow taken? Since, now multiple instances of MainWindow can be executed, and for a window to have toolbars, it needs to be a MainWindow. This seems just like the old Window behavior with different steps. Was this limitation made to fix certain bugs, or was it a design decision?

[freakboy3742]

Also, a side question, why was the decision to make toolbars limited to MainWindow taken? Since, now multiple instances of MainWindow can be executed, and for a window to have toolbars, it needs to be a MainWindow. This seems just like the old Window behavior with different steps. Was this limitation made to fix certain bugs, or was it a design decision?

It's part of a larger redesign that allows for apps without menu bars on their main window (i.e. "Simple" apps), and apps without main windows (e.g., background app and document/session-based apps). See #2244 for the full discussion.

For most apps, it won't make any difference. You can still use a toga.MainWindow as your app's main window. The only difference is if you were previously adding a toolbar to a secondary window in your app - in which case, that window now needs to be a toga.MainWindow, not a toga.Window.

[proneon267]

I had a question, as mentioned in the cocoa source:

# This doesn't wait for completely exiting full screen mode. Hence,
# this will cause problems when rapid window state switching is done.
# We should wait until `windowDidExitFullScreen_` is notified and then
# return to user.
self.native.toggleFullScreen(self.native)

Since, toggleFullScreen is non blocking, how do I block until windowDidExitFullScreen_ is notified?

[freakboy3742]

I had a question, as mentioned in the cocoa source:

If you've got a question, the source code isn't the place to ask that question.

# This doesn't wait for completely exiting full screen mode. Hence,
# this will cause problems when rapid window state switching is done.
# We should wait until `windowDidExitFullScreen_` is notified and then
# return to user.
self.native.toggleFullScreen(self.native)

Since, toggleFullScreen is non blocking, how do I block until windowDidExitFullScreen_ is notified?

Why do we need to block at all?

Rapid window state switching isn't a real-world use case - or, to the extent it is, I'd be entirely happy documenting that rapid switching can be a problem.

If the issue is testbed testing, then we can insert whatever delays are needed - either a hard coded pause that is "long enough", or a poll-with-async-sleep until we can validate that the state change is complete.

[proneon267]

By rapid window state switching, I meant directly changing state from PRESENTATION state to FULLSCREEN state or vice-versa.

As an example, running the windows example app and first setting window state to PRESENTATION and then directly switching to FULLSCREEN should manifest the rendering glitch on macOS.

Hence, we need to block until windowDidExitFullScreen_ is notified.

[freakboy3742]

By rapid window state switching, I meant directly changing state from PRESENTATION state to FULLSCREEN state or vice-versa.

Is rapid an important part of that description? Or is it any transition from PRESENTATION to FULLSCREEN?

If it's a problem that only occurs when a user is mashing keys quickly, or programmatically changing from one to another with no delays, then I'd argue it's not a problem (or at least, a limitation we can document and live with).

If it's a fundamental issue with any transition between those modes, then sure - it needs a fix. It also needs tests - which is why I flagged that as an issue in my review.

As an example, running the windows example app and first setting window state to PRESENTATION and then directly switching to FULLSCREEN should manifest the rendering glitch on macOS.

Hence, we need to block until windowDidExitFullScreen_ is notified.

Or - we make that transition illegal. I don't think it's unreasonable to say that you can't transition between PRESENTATION and FULLSCREEN modes. They're both "whole of screen" modes; saying you need to move through an intermediate "not whole of screen" mode first doesn't seem like a huge restriction.

Alternatively, you don't block - you set up the logic so that windowDidExitFullScreen: fires the "rest of the transition" logic. This effectively sets up a "future" or "on_done" callback, but without being in async mode.

[proneon267]

Reposting here for better visibility:

As previously mentioned in https://github.com/beeware/toga/pull/2473#discussion_r1642620415, I couldn't directly use the following in parametrize:

@pytest.mark.parametrize(
    "windows",
    [
        [],
        [toga.Window()],
        [toga.Window(), toga.Window()],
   ]
)

So, I used: https://github.com/beeware/toga/blob/6b3e88391da04c2ca6d81e9a499fbe8681625c4e/core/tests/app/test_app.py#L469-L480

This works properly in python 3.10, but fails on python 3.12 with the following error:

=================================== ERRORS ====================================
___________________ ERROR collecting tests/app/test_app.py ____________________
tests\app\test_app.py:471: in <module>
    (toga.App(formal_name="Test App", app_id="org.example.test"), []),
..\.tox\py-cov\Lib\site-packages\toga\app.py:419: in __init__
    self._create_impl()
..\.tox\py-cov\Lib\site-packages\toga\app.py:422: in _create_impl
    self.factory.App(interface=self)
..\.tox\py-cov\Lib\site-packages\toga_dummy\app.py:24: in __init__
    self.loop = asyncio.get_event_loop()
C:\hostedtoolcache\windows\Python\3.12.4\x64\Lib\asyncio\events.py:697: in get_event_loop
    warnings.warn('There is no current event loop',
E   DeprecationWarning: There is no current event loop
=========================== short test summary info ===========================
ERROR tests/app/test_app.py - DeprecationWarning: There is no current event loop
!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
============================== 1 error in 4.86s ===============================
py-cov: exit 2 (6.45 seconds) D:\a\toga\toga\core> python -m coverage run -m pytest -vv --color yes pid=5480
  py-cov: FAIL code 2 (15.75=setup[1.41]+cmd[7.89,6.45] seconds)
  evaluation failed :( (15.92 seconds)
Error: Process completed with exit code 2.

Since, it gives a DeprecationWarning, is there a new way to parameterize this?

For the time being, I have restored the tests to their non-parametrized forms to continue the testbed tests on CI.

[proneon267]

The macOS testbed failure is not reproducible on my system, and is occasional on the arm64 testbed. I'll try to find the cause. Wayland also seems to be reporting wrong dimensions. I'll try to replicate the failure on a Wayland system and report back.

[proneon267]

All CI errors have been resolved. I've added some review notes in the cocoa source, which I'll remove after the review.

[freakboy3742]

An update from my end - I've currently got a bunch of deadlines related to the Python 3.13 release process and some other internal work; I'm hoping to be able to look at this early next week.

[proneon267]

Thanks for the update! No worries, take your time. Good luck with the Python 3.13 release :)

[proneon267]

I haven't gone through the whole review but regarding test_transitions, I had added a test in testbed/test_window.py: https://github.com/beeware/toga/blob/d0dcb359d4e134f6c8412384ee0101c44535221a/testbed/tests/window/test_window.py#L854-L886 Does it miss some test case?

[freakboy3742]

Does it miss some test case?

Yes - all the transitions to NORMAL.

[proneon267]

I have gone through some part of the review, and have replied to them, while others are pending. But, I think a major concern you have raised is the implementation of the state machine pattern. So, I want to address that first.

So, here is what I had intended to implement: The orange box operations are non-blocking and their exit status can only be confirmed at the window delegate events. Hence, I had to implement by using the "pending state". I have tried to follow the pattern of a state machine while implementing, but I have used recursion in the implementation to reduce code duplication. So, is recursion not allowed in state machine pattern?

Does it miss some test case?

Yes - all the transitions to NORMAL.

I had left them out since they were already being tested in the test_window_state_{x} tests, as in them, there is a switch from state {x} to NORMAL. But, I have added the cases of direct transition states to NORMAL in test_window_state_direct_change.

[proneon267]

Maybe I am creating more confusion by the diagram. But, the reason for why I had to implement the way it is currently, is because of 3 factors:

• The OS operations are non-blocking and return immediately. For example:

Line 1: set_window_state(FULLSCREEN)
Line 2: set_window_state(MAXIMIZED)

So, there would execution of Line 2: set_window_state(MAXIMIZED) while the window is still in full screen - Thereby, causing glitches. This problem is not exclusive to FULLSCREEN, but for any direct transition between 2 states. For example, between PRESENTATION->MINIMIZED->MAXIMIZED.

• Hence, the window needs to be in NORMAL state before switching to any other state to prevent any glitches.
• I also do not want to make any transition illegal as in such a case the user would have to manually switch to NORMAL state before changing to any other state.

If the OS operations would have been blocking, then the implementation would have been identical to the state machine structure that you have given:

if current state 1:
    if future state 1:
        ...
    elif future state 2:
       ...
elif current state 2:
    ...

I had earlier though about blocking until the delegate notification is received, but then realized that it would cause a deadlock. Hence, I had opted to use _pending_state_transition.

But, I am not sure, how can I conform to the structure of the state machine pattern that you have given, with these non-blocking OS operations.