Rework app handling, and add status icons #2238
Conversation
|
Wouldn't the analogous thing to Windows system tray icons on macOS be the status menus accessible on the right of the menu bar?
There are built-in ones like Bluetooth and WiFi, apps can have their own too. Adobe Creative Cloud, Karabiner Elements, and Lunar, for instance, are accessible from their icon in the menu bar. They're apparently capable of bringing seemingly arbitrarily complex UI, as evidenced by Lunar:
But they're also certainly capable of presenting straightforward menus, like Karabiner:
Clicking on the Creative Cloud icon even opens an entire actual window. I'm not actually sure what Apple's HIG recommendations for them are... Edit: I think Lunar is probably just sneakily opening a heavily modified window / pane, especially since the icon doesn't remain "clicked" like for Karibiner. |
"Icon in the status bar" is how the UI manifests on macOS.
The only question is what (if anything) pops up when the icon is clicked:
I imagine we could add support for (3) by allowing commands to be status items as well. I'm not sure that's worth the effort, though - I'm not sure what a status icon that can open a window does that a normal macOS app icon can't achieve. I'm not sure how we'd expose an API for an arbitrary GUI in a popover. There isn't an obvious mapping to Group/Command that I can think of. the only idea I've got is an object that can be added as a "command" that can contain a "content" widget... but that feels messy, as it would only really be usable in a StatusItem context.
They explicitly encourage using menus :-) While I can definitely see the use case for having a complex GUI, I'm also (personally) willing to put that into the combination "future enhancements" bucket. You can do a lot with just a menu, and having the feature available at all would be an improvement. My only hesitation would be if the design we have now prevents future alternatives. |
|
...You know what, maybe I should get some sleep. I totally read iOS in your post as macOS, which is why I was confused! |
|
Ping @samschott for your thoughts, since I know you've done some work in this area with Maestral. |
|
It's finally arriving in toga! I'll have a look this weekend :) |
|
Filling in some discussion I've had in-person with @mhsmith: There may not be a need for separate SimpleApp, WindowlessApp and DocumentApp base classes. There are 2 key features competing here:
Thinking about this, the core of the issue may be that we have 2 subtly incompatible uses of Main Window. There's the One option would be to modify MainWindow slightly so that it doesn't have any specific controls related to closing the app. A MainWindow then becomes "an window with a menu bar"; a simple "Window" doesn't (or, at least, it doesn't at present - see #2210 for a related topic). Your app can has as many MainWindows as it wants - which is effectively what DocumentApp does - there's a "main" window for each document. In the context of macOS, this doesn't really change anything, because menus aren't bound to windows. On iOS and Android, it controls whether the titlebar is added (and also controls whether a toolbar is available. On that basis, it might be worth migrating toolbar functionality to be specific to MainWindow, leaving Window as a simple container. What then matters for app lifecycle is which window is assigned as All the handling in DocumentApp for defining document types and command line handling can then be moved into the base app as well, allowing any app to respond to document types, respond to argv, and so on. The This also fits in with the use of default app control commands on Status item groups. If you assign a main window, the app control commands go on the main window. If you don't, they get assigned to the first status group. If you don't define a status group, you get an error. In this context, there might be some benefit to renaming Some notable downsides to this approach:
Maybe we can accomodate this with another |
…lity to define status icons.
freakboy3742
force-pushed
the
app-types
branch
from
f53cc04
to
49c54e5
Compare
freakboy3742
changed the title
|
Hello
This would be much more analogous to the desktop behavior. |
Really? I'm not an Android user, but a notification - which is transient, and will disappear - doesn't seem like it's a match for status icons, which are permanent features that run in the "background" of the runtime environment. Can you provide an example of an Android application that uses notifications in this way? |
|
There can be permanent notifications, in which the app runs in the background. These notifications cannot be dismissed while the app is running in the background.
|
|
It's true, music players in particular, on both Android and iOS, often have a permanent "notification" that appears in notifications and the lock screen, shows what's playing, and offers basic playback controls. I know both Pandora and Apple Music do this. I don't know what the official name or recommended behavior for this type of thing is. I haven't seen one that expands to a dropdown menu, though. |
On iOS, that functionality is called a "Widget" - and it's definitely not a good match for what has been implemented in this PR. You wouldn't ever see a list of menu options on a Widget - they're designed to be primarily informative, contextual (e.g., the current weather, or the status of an in-progress delivery), with occasionally actions (such as play/pause) exposed. That might be an argument using Widgets as the manifestation of "arbitrary GUI" status icons; but I'm not sure they're a good match for "menu style" status icons. My inclination is to ignore mobile platforms for now, at least until we've got a better idea for how "arbitrary GUI" status icons can be supported. |
|
Example of a drop down notification:
👍 |
Separate app classesI've had a read through your discussion notes and PR description and my initial thoughts are:
WindowsRegarding the discussion around windows: As far as the implementation of apps is concerned, the way we think about windows makes a difference. But I would still argue that one should consider the main window question independently and find the best model / API regardless of how you want to represent apps.
Edit: I do see the irony in my last point. If we don't require a main window, what distinguishes an App from a WindowlessApp? Maybe not that much after all. |
| section: int = 0, | ||
| order: int = 0, | ||
| status_item: bool = False, |
There was a problem hiding this comment.
IIUC, the group itself will represent the status item. How do you think about this API vs having a light-weight StatusItem class with icon and commands / command_group properties? IMO, this would be more explicit than having a group marked status_item = true, assigning this group to a command, and adding that command to the app to finally get a status icon.
There was a problem hiding this comment.
I did consider this; the problem is that I couldn't find an abstraction around the existing usage of Commands that made sense to me.
The current API doesn't create "menu items" and "menus"; we create and register "Commands" (as blocks of functionality that are surfaced by the app somehow). The group structure then sorts those into menus. A toolbar then becomes an alternate way of accessing that command; a command added to a toolbar is automatically registered with the app (if it isn't already registered).
So - the entry point to commands is registering them with the app. You don't register groups explicitly - you need to have them for organization purposes, but you don't explicitly tell your app "these are my groups"; the app discovers them by iterating over commands.
If we introduce a StatusItem class, then the'd need to have an analog of toolbar registration; That's not big problem; but what is registered against this new collection? Either:
- StatusItem is a subclass of group. That essentially gives us what we have in this PR, except there's an extra subclass that adds nothing except a mechanism to differentiate it from a normal group... at which point, why do we need a separate registration process?
- StatusItem contains a group. This is a departure from the historical norm as the group becomes the point of registration, but you still need to register the commands, as there's no way to get from the group the commands - so you have to register all the commands, then there's a separate process to declare your status item and the group it contains. It also means you register commands with a toolbar, but you register a wrapper around a group with the status item... so we've lost API consistency for command-related tooling.
I came to the conclusion that (2) was a bunch of complexity and conceptual confusion for not a whole lot of gain.
That said, one advantage to (2) is that it opens an obvious door for a separate "view-based" status item base class, so popdown menus that have complex GUIs become a possibility. However, I'm not sure whether the complexity of the registration approach it requires is worth it; and I'm sure we can find some other way to accommodate them that doesn't require the conceptual gymnastics.
There was a problem hiding this comment.
Hm, I seem to recall a discussion that we had around the need of a separate MenuItem class some ago, for cases where Command does not fit well.
That being said, maybe Command is indeed the right abstraction for menu items in the status bar.
Another option might be to have a StatusItem class and register Commands with it instead of registering commands with the App and assigning them to a status item group. This would however mean a full separation between status item and app commands. According to the documentation, this already seems to be the case at least in effect, if not in code:
Status item groups are not added to the main menu; instead, an icon will be added to the system tray or notification bar, and the Commands associated with the group will be displayed in a menu when the status icon is clicked.
There was a problem hiding this comment.
I think I can see how that could work; and in many regards, the implementation wouldn't be that different from what is here now. There's a slightly more complex path to look up and create them menu representing the group, but that's resolvable.
However, there are some interesting edge cases in the mapping between status item and menu bar. If I have 2 commands in different groups, but both registered with the same StatusItem - does that produce 2 status items or 1? Are groups on the top-level status item menus ignored? Do they convert into sections in the status menu? A toolbar puts a separator between 2 menu items in different groups, so I guess there's precedent for interpreting groups as "sections"; but the difference with toolbar is that the underlying commands still have their group structure in the main menu. This wouldn't be true for Status Items.
| self.on_change = orig_on_change | ||
| if self.on_change: | ||
| self.on_change() | ||
|
|
There was a problem hiding this comment.
Out of curiosity, what is the intended use-case or concern that this is addressing? Do we have something similar for other classes with on_change handlers?
There was a problem hiding this comment.
It's an optimisation for app startup. On Cocoa, the app instance is created and configured as part of the App constructor. The last thing the app constructor does is install an on_change handler on the app's command set, so on_change is only triggered when there is an actual change after initial setup.
However, on GTK, the app is created during the constructor, but isn't configured until the GTK startup signal is received. This means the on_change handler is installed before the app commands have been fully constructed, so you end up invoking create_menus multiple times as the user's startup menu is invoked.
An analogous problem exists on Winforms; the user's startup() method isn't invoked until the main loop is started.
Thinking about it with a clear head, I guess an alternate approach would be to introduce a "finalise app construction" method on the core app, so that anything that needs to happen once the user's configuration code has all executed, and split core's constructor into two, using the call to the factory that constructs the App impl as the split point. At present, the only thing that needs to be done "late" is installing the on_change handler, but I guess there could be others.
There was a problem hiding this comment.
Thanks for the explanation!
I think my preference would be to indeed keep this only as a public API if you think it is useful to toga users, for example when modifying commands after startup, and not because of implementation details for some backends.
There was a problem hiding this comment.
That's reasonable. I think there's definitely a use case for a public "suspend" API for events as an optimisation, but that needs a more general approach. Plus, reworking startup() so there's a "finalise" stage makes more architectural to me sense anyway.
So - it turns out (see #2244) that there is no difference in implementation. The only differentiation is:
All of these concepts can be independently accommodated without having different App types (e.g., you can make the window not close the app, but still have a main menu). It's also worth noting that none of the APIs we're wrapping have different App types. That's not a strong argument in itself, but it's an interesting data point.
A reasonable argument; however, it's a moot point if there aren't app subclasses :-)
It's entirely likely that we will need to introduce a menu_bar attribute on Window (see #2210). The distinction I'd make is whether the window's menu includes the "app level" commands. On a Main Window, they are included; on a "normal" window, they're not.
This is effectively resolved in #2244 - you need to assign a value to |
|
You are indeed convincing me that separate app classes are not needed after all :)
Yes, that does indeed make sense! |
|
I'm going to close this PR; the status icon code is the only part of this that is likely to survive, and it will be dependent on #2244 (or it's derivatives) landing first. |
Adds a
SimpleAppbase class. This is an MainWindow-based app that has the bare minimum of decoration. For platforms that put a menu in the main window, no menu is created. For platforms that have a global menubar, the menu only contains the bare minimum of required items.Adds the ability for an app to declare status items (i.e., system tray/notification icons). A command Group can be declared as a status item group. The Status item group is a normal menu, but it is not added to the main menu; it's added to a status icon. An app can declare multiple status items, resulting in multiple status icons. The first status item group (by group sorting order) gets default app control items (about, settings, exit) automatically added to it.
Adds a
WindowlessAppbase class. This is an app that doesn't have any window by default. There isn't much use for this by itself, except when used in combination with status icons - a "status tray only" app is a WindowlessApp that puts all its commands in a status item Group.Fixes:
This isn't ready to be merged yet - I've pushed it (a) so that nobody duplicates any effort, and (b) to get feedback on the general design. There are 2 new sample apps - SimpleApp and StatusIconApp - that demonstrate the new features.
Some other details on the current state of the PR:
gir1.2-ayatanaappindicator3-0.1.PR Checklist: