Use a single base class for all app types #2244
Conversation
|
Running Ubuntu 22.04 with Wayland, the PR works as expected for a document app. Closing all windows also seems to follow system behaviors of other applications. Only issue that I can see is spacing below menu bar and text field as discussed in the discord. |
|
My two cents: I think this arrangement feels more intuitive than the multiple-classes route, and quite possibly avoids snarly inheritance problems down the road. There's a use case this abstraction doesn't quite cover, though. I don't think document-based, create/open/edit-style apps are the only ones that are suited to the behavior you describe of 1) having no main window and 2) remaining persistent (Mac) or closing (Windows/GTK) when the last window is closed. Consider practically every web browser, as well as any filesystem browser (at least third party ones that aren't special-cased by the OS). I feel like it would be good to have a way to flag an app to say more generally "All my windows are equally important" without binding it specifically to document files. Your proposed system can get the appropriate native behavior for that on macOS by setting I also think naming the attribute and the not-necessarily-related class "main window" is definitely misleading, but at present I'm not sure what I'd recommend instead. |
That's a good point. I agree this is a use case we should support. The first thought that comes to mind is to allow In that way, "browser"-type apps and document apps would both declare themselves as
Introducing the |
|
Hmm. If the concept is centrality, i.e. "What is the central window this app presents as", then it does start to make sense for Admittedly even that last one starts to break down slightly when you think about Mac apps like Activity Monitor that have one primary window but also persist, so it's still not a perfectly mapped abstraction. |
I agree LAST_WINDOW isn't a great description of what happens on macOS, but naming things is hard. Unless we can come up with a good name for the "lifecycle is tied to window count" concept, I'm comfortable with using a slightly awkward name, and documenting around the problem.
That's usually spelled "subclassing" :-) And, FWIW, it doesn't really solve the naming problem, because now we need to find a name for the app subclass that encompasses the same concept.
Except that you don't "close" a status icon - there's a menu item that exposes "exit".
FWIW - I'm definitely comfortable leaving some esoteric use cases on the table. Toga isn't the toolkit you're going to use to write every app - a cross platform framework can't be that. There are always platform-specific details that can't be cleanly abstracted. |
Okay, point taken. 😆 But it could also be spelled "having one or more flags/options on a class" like you've proposed here, it just depends what those flags are...
That's true, but is it a problem? If it's the concept of I dunno. It makes sense to me, at least, but no two brains are alike, after all. |
Oh, absolutely. I was just tickled that the way you were describing it was so close to subclassing without saying the magic word. More importantly, using flags allows us to have multiple orthogonal behavioural concepts - we can have flags for "this is how the app closes" and "this is how the app displays the app icon", rather than locking in "One type of app is a DocumentApp, which has these specific behaviours" etc.
Hrm... So - I guess what you're advocating for here is that rather than making That definitely allows us to avoid the problem of trying to find a name for the "exit/persist on last window close" use case. |
|
Yep, that's exactly what I meant!
And document apps would just be the last type, with one or more document types associated. |
Ok - The one gap this leaves is the "Default window" case. If We currently use the |
|
Ah, I had not thought of that. I could see an argument for either option (accepting the behavior or raising an error), or for a middle ground of printing a warning. Personally I'd lean toward raising an error, so no one inadvertently makes an uncloseable app that their users have to force kill. |
|
Sorry for taking so long to respond. I like the way this is going, but I think the UI element visibility:
App and window lifecycle:
Documents:
Not all of this necessarily has to done in a single PR – in fact, it would probably be easier if it wasn't. |
How do you envision this handling the platform-distinct handling of "browser"-style apps? If all its windows are MainWindows and you close the last one, the app (correctly) exits on Windows. We would want it to instead keep running on Mac, but we can't just exempt Mac from exiting on the MainWindow being closed because smaller, single-window apps usually should still exit when that window is closed. (On the other hand, implementing it as all normal windows would get the correct behavior on Mac, but it would keep running on Windows too.) |
I think I see how this would/could work; some questions about specifics:
FWIW, the design in #2238 does allow for status icons to open windows (and the demo app does). When you do, the app-level menus aren't visible (that is tied to the 'hide app icon' behavior), but I believe that could be controlled on a "gain focus" basis. The ongoing discussion on #2238 has flagged some problems and a potential redesign that might happen in this area, but the constraint of possibly using app commands to register status icons is worth keeping in mind.
I'm not sure how practical this is. If nothing else, it gives the impression that a window can change from "has titlebar" to "doesn't have titlebar" arbitrarily... which might be possible to implement, but it strikes me as an axis of flexibility that isn't a great idea. It's also functionality that I'm not sure we'll be able to implement at all on non-mobile platforms. That's not fundamentally an issue - we can definitely have features that are desktop- or mobile-specific - but if we can avoid them, I'd argue that's preferable.
There's a discrepancy, which @HalfWhitt has picked up on; but it's not just "browser" style apps, it's also document based apps. In these, closing the last window exits the app on Windows/GTK, but on macOS, we need the app to persist.
Agreed that having one less thing to configure (and potentially cause confusion) is desirable. However, I think differentiating "this is what the main window looks like" from "this is how the app closes" does make some kind of sense - and provides a way to differentiate the Document/Browser app use case. I will freely admit I don't love There's also something to be said for the fact that in the simplest use case, the MainWindow and
Agreed that the "buggy app" use case is unfortunate, but not something we need to take extraordinary measures to avoid.
As noted above, the lifecycle of the app itself is also a factor. Transforming open into an event is an interesting idea; a lot of the mechanics in the current implementation are very close to being an event, but "hard coded". Making open (et al) explicit events also means we've got a clear way to determine why the new/open/save menu items should be displayed - document apps would install default handlers that tie to the document APIs, but non-document apps could define on_open handlers to provide a way to provide file-based behavior without actually becoming full document apps (say, a single-window graphing app that is able to load a data set).
Agreed. I included document apps here to flesh out whether the core ideas held together, but once we've got a high level design that we know can be implemented, it will likely be easier to revisit this PR in parts. |
I think that's a reasonable default, and it's no big inconvenience to create a CommandSet, as long as we make it part of the public API. (EDIT: although, if menus were the only difference between MainWindow and Window, then this would be unnecessary – see below.)
On GTK and Windows, the only default commands are Exit, Preferences (which should probably be removed, because it doesn't have any implementation), and About. I don't think there's any problem with having those in every window that has a menu bar.
Yes, adding or removing a title bar in an existing window might be difficult to implement, so maybe we say that you have to decide at construction time, and you can't switch between Title-bar-less windows are certainly a thing on desktop platforms, but I agree it's not worth implementing them just now. Even so, I think
Good point. In that case, I guess we do need a separate flag, and maybe we can use that to avoid the "undead app" case as well. Imagine an Auto-exit would default to true, but DocumentApp would set it to false on macOS. This would automatically cover the case where an app has a single window, or multiple equivalent windows, which should cover a large majority of apps. If the developer wants to do something more complex, like exit the app when a specific window closes, or when all of a particular class of windows has closed, it would be easy enough to implement that themselves. This suggests that we would remove the special
That would still work with this proposal. |
Those are the only menu items right now; but that's as much because I'm a lot more familiar with platform expectations on macOS, so I've been able to fill out more menu items. It seems reasonable to expect that this default list will be longer with time. However, that doesn't really change anything - it's really a question of whether not having the core menu set would be more platform appropriate.
Except these aren't independent features. In the Android context, you can't hide the titlebar and have menu items. macOS and GTK both put the toolbar in the window title; GTK's preferred menu handling puts the menu in the toolbar as well (see #874). Looking to the future, Android and iOS apps aren't going to be able to use stack-based navigation views (i.e., select item from list, page slides in from the right, slides off to the left when dismissed) if you haven't got a title bar in which to put the navigation elements. I'm sure we could put in a bunch of gating logic that raises an error if you try to hide the titlebar if the app has commands, if you add a toolbar to an app without a titlebar... or we can make a clear distinction that there's a "simple" window that only has the bare bones necessary to display content, and there's a "full" window that can accomodate all the extra options.
Except that this would (a) require the reintroduction of DocumentApp, and (b) results in "boolean but it might be ignored" business logic for For my money, nominating a "main" window is more explicit, and assigning
But what would it mean? As I understand what you're proposing, |
freakboy3742
force-pushed
the
app-base-class
branch
from
7c186a0 to
0fb585f
Compare
Including them allows an easy to understand implementation as laid out in #2210. I doubt many developers would have a problem with this, but if they do we can always come back to it later.
On further thought I think you're right. The problem with So just to clarify, is this the proposal?
|
I never proposed removing it, I only pointed out that if there was an
I'm not sure whether this refers to DocumentApp setting it to false on macOS, or the possibility of App.on_exit cancelling the exit, but those both seem easier for the developer to understand than a complex interpretation of the If you're still leaning in that direction, can you specify exactly what that interpretation would be? It's already changed a couple of times during this discussion.
Yes, that's correct. The |
Broadly yes. I think there's still some API work to be done on #2210 (specifically, how to differentiate between app-level commands and window-level commands, especially at time of creation); but whatever form that API takes, I think we're agreed on the interpretation - that macOS gets all commands in the single app-level menubar; other platforms get the union of "app level" commands, plus the commands registered against the current window. I have one other possible clarification/inconsistency:
I'm unsure whether these two should be "does not have", or "doesn't not have by default". Saying that a menu is only available on a MainWindow makes some sense - as you've described, it provides a good entry point for the interpretation of #2210, and allows a way for us to dodge questions of whether we should allow menus without app commands. However, toolbar is a simpler case, because it doesn't exist at all unless you add something to it. My question is whether there might be a use case for a window that has a toolbar, but no menu. From an implementation point of view, it's not that hard to allow for toolbars on a simple Window (after all, we do right now); the argument against it would be more on the grounds of "conceptual consistency" - that a Window is truly "simple", and a MainWindow is a complex UI element. I could go either way on this one. Probably the biggest argument in favor of allowing toolbar on Window is that removing it would be backwards incompatible.
Ok... but the point of this PR was to converge a single App class, rather than needing different specialised app types... so removing it would seem to be the point of the exercise. :-) That said, I agree we should be trying to converge on having common behavior in the base class; and abstracting
I was referring to macOS having a different functional interpretation of "True" of "auto exit". An auto-exit macOS app won't exit. Ok, this could be explained in docs as a platform inconsistency; but I think that representing this in a form that is tied to the high-level role (main_window) rather than the specific interpretation (app exit) makes more sense.
Yeah - apologies for the confusion. My current understanding is that
The advantage I see with this is that it requires the user to be intentional about how the app exits - either naming a window, or explicitly saying that there is no main window. We can use that to catch the case of neglecting to set a main window during startup. As an independent concern; if an app declares document types, and a command line argument matching that document type is provided, a Document of the registered type will be opened. If an app declares document types, and no command line arguments are provided, the app-specific "default document" behaviour will be triggered - an empty document on Windows and GTK; the "open file" dialog on macOS.
I am sympathetic to the idea of removing one usage of the term "MainWindow"; however, in this case, it means There's also the case of an app that opens a main window, opens a secondary window (say, a non-modal dialog), and then the user closes the main window. Unless I'm missing something, the The alternative is an app-level main window attribute, and a MainWindow class. An introductory app (and, I'd warrant, most apps) will include a |
As you said above, "these aren't independent features. In the Android context, you can't hide the titlebar and have menu items", and you can't have a toolbar either. So I don't see how we can avoid the backward incompatibility here.
Yes, I think there would be. If we implemented #2210, this could be expressed by setting
I never said a True value would be ignored on macOS, only that DocumentApp would set it to False on macOS. Having said that, I think you've convinced me that a simple boolean isn't good enough here. Just a couple of questions about the proposed meaning of
I agree with not having SimpleApp / WindowlessApp classes as in #2238. But I think it still makes sense for DocumentApp to be a subclass of App, because it groups together a lot of closely-related elements:
Any app that wants one of these will probably want the others, but an app that doesn't want one of them probably doesn't want any of the others. So rather than putting them all into the base App class, which is quite complex already, I think it would be easier to understand and maintain if we keep it as a separate layer on top, providing a ready-made structure for a specific kind of app. That way, the document-related code and documentation is all in one place, making it easier to get an overview if you want it, and keeping it out of the way if you don't want it. The other rationale for having a single App class was to make it more testable, but if DocumentApp didn't require any backend-specific code other than the |
That's a good point - I had forgotten the Android (and potentially iOS) implication here. I guess that's decided that one, then - Window has to be a true "simple" window, without menus or toolbars.
Makes sense. In the macOS case, that means the app will only have app commands; the Windows/GTK case could get a little interesting because the default app commands will be hidden from the user - so features like opening help or preferences won't be exposed anywhere. It then falls on the developer to make sure they're exposed as a toolbar item, or via some other interface. However, I think that's firmly in the camp of "well you asked for this, it's up to you to deal with the consequences" :-)
That was my thought - but I'm open to other suggestions for ways to designate the dock-hiding behavior.
We're now in a position to allow On any window, regardless of app mode, If
Side note: we also require platform logic for iOS and Android to raise an error if
I'm not sure that is true. For example, consider a simple log viewer app - you might want to accept command line arguments to open a specific data source, and have a File/Open menu item to select a new data source, but not have a full multi-window document interface (because you only want the app to manage a single log source). That's a slightly contrived example, but I don't think it's entirely unreasonable.
Hrm... I think I see what you're saying - I'll have a bit more of a tinker and see what I can make work. I think I can see the vague outline of how document handling can be managed without making the App class excessively complicated (or at least, not having features that overlap with other functionality), but I need to get my thoughts straight. |
freakboy3742
force-pushed
the
app-base-class
branch
from
30bb3f6 to
96b1684
Compare
freakboy3742
force-pushed
the
app-base-class
branch
from
db9789f to
3ce4c5f
Compare
freakboy3742
force-pushed
the
app-base-class
branch
from
3ce4c5f to
e75069e
Compare
|
Replaced by a number of smaller PRs listed in the top comment. |
[Most of the comments below refer to an older version of this design. To see the differences between the two versions, view the edit of this comment on Mar 18, 2024 – @mhsmith]
Following the discussion on #2238, a revised approach to the window type question.
In this PR:
toga.App.toga.MainWindowis a "window with full decoration". This means it includes a menu bar (on Windows and GTK), and a titlebar on iOS and Android. On macOS, the app-level menubar still exists, but the list of commands is vastly reduced.toga.App.main_windowdetermines the behavior of the app:toga.MainWindowinstance generates the historical app behavior.toga.Windowinstance generates a "simple" app, with minimalist menus/titlebarsNoneresults in a Session-based app. This is what a "Document-based app" was previously called, but it now covers the generic case of web/file browsers that don't have a "document" per se, but have the same "close on last window"/"persistent app with no windows" behavior.toga.App.BACKGROUNDmakes the app a background app - an app with no requirement for windows, and a hidden app icon (where possible). This is the placeholder for a status icon app.main_window) is also an error.main_window, rather than being specifically bound totoga.MainWindow.main_windowis a Window (or subclass), closing that window closes the app.main_windowis None, the exit behavior is platform dependent; see discussion below.main_windowistoga.App.BACKGROUND, it's up to the user to define a way to exit the app. This isn't very helpful at present, but will make more sense once status icons are added.Session-based (nee Document-based) apps are now fully implemented on all desktop platforms. On Cocoa, the behavior hasn't changed; the app instance is persistent, and there's an app-level "exit" menu option. On GTK and Winforms, the behavior has been modified so that there's a check when closing windows; when the last window in the app is closed, the app exits.
This PR doesn't implement status icons; status icons become a relatively straightforward piece of app-level menu item handling once this PR is implemented.
The current state of this PR implements tests for the core API, and is fully documented; but the testbed is currently showing coverage gaps. That said, there are very few testbed tests required - most of the new behaviour has been kept in the core.
In addition to an app like
tutorial2that exercises the "MainWindow" case, there are three new examples:However, my intention is that this PR won't even land. It is unmanageably large to review in it's current state - but it is useful as context for some of the design decisions that have been made, and it's a useful proof-of-concept that the final API design works in practice across all platforms. The following is a list of discrete changes that can be factored out of this work; I'll update this list with PR references as they are submitted:
finalize()step to app creation to allow for deferring the commandon_changehandler until commands are fully prepared (Ensure menu creation sequence is consistent. #2619)Window.on_closehandling so that app exit conditions are triggered byWindow.close()(Ensure that programmatically closing the main window triggers on_exit handling #2643)Windowso that toolbars are only valid onMainWindow, and menus are constructed on a per-window basis if the platform requires (Limit toolbars to MainWindow instances, and allow for MainWindow menus. #2646)document_typesa property on toga.App. (Finalize API for Document-based apps #2666)