beeware / toga

A Python native, OS native GUI toolkit.
https://toga.readthedocs.io/en/latest/
BSD 3-Clause "New" or "Revised" License
4.19k stars 655 forks source link

Add app-level dialogs. #2669

Closed freakboy3742 closed 4 days ago

freakboy3742 commented 1 week ago

At present, all dialogs are defined at the Window level. This means you need to have a window instance to display any dialog.

However, not all dialog requests are necessarily window specific; and with the introduction of session-based and background apps (#2651), there will be situations where there isn't a window to which a dialog can be attached.

This PR modifies the API for dialogs to allow for app-based dialogs. It does this by deprecating the older "functional" dialog methods on Window, in favor of surfacing the dialogs as public classes; these classes can be displayed by calling await window.dialog(some dialog) and app.dialog(some dialog).

On macOS and GTK, window-based dialogs are presented as "sheets" attached to the window; app-based dialogs are presented as free floating. Windows presents all dialogs at the app level. iOS and Android always have a window, so app-level dialogs are presented in that context.

This change necessitated a fairly major rework of dialog testing. The old API displayed the dialog on creation, but returned an object that could be awaited; the new API doesn't display the dialog until an async show method is invoked.

PR Checklist:

freakboy3742 commented 1 week ago

@mhsmith There's something odd going on with the macOS M1 testbed... it's passing 100%, then reporting a failure. I'm guessing this is a segfault on cleanup or coverage; I need to investigate further.

However, I'd be interested in your high level thoughts on this as an approach.

What I've done here is the obvious approach - replicate the Window dialog APIs onto App. A different approach would be to expose dialogs as a first class module, then expose either:

I'm not especially enthused about the spelling of either of these APIs... the former feels like creating a temporary object for no reason; the latter doesn't feel consistent with the rest of the Toga API. I'm open to other suggestions if you have them.

That said - the one notable upside to dialogs being a first-class modules is that we get a standalone documentation page for dialogs, rather than needing to find a way to direct people to the app and window pages.

It also gives us an opportunity to fully deprecate dialog on_result handling, which massively simplifies the typing interface. Of course, we'll need to carry the old window API entry points for a while, so it's not a complete win.

mhsmith commented 1 week ago

the one notable upside to dialogs being a first-class modules is that we get a standalone documentation page for dialogs, rather than needing to find a way to direct people to the app and window pages.

I think that would be very useful, because the App and Window pages are already getting quite large.

InfoDialog(...).show seems reasonably consistent, as Window has a show method as well. But if calling it with zero arguments would display a non-modal dialog, then the most obvious way of using the method would be the wrong way for most use cases. Instead, how about making show take a required argument called context, which can be either a Window or an App?