Add macOS merge all windows command #2227
Conversation
HalfWhitt
changed the title
|
Hmm, it passes on my local machine. Perhaps I need a |
There was a problem hiding this comment.
A couple of comments inline; but I've got a couple of other questions.
To test this, I ran the windows example (since it's an obvious case that tests multiple windows). In that testing, I noticed 2 things:
- Despite the apparent implementation, the merge command seems operates on the current window, not the main window.
- Despite the "all" in the name, the merge command doesn't seem to operate on all windows - in the Window example, the non-resizable and non-closable windows aren't merged.
It's not entirely clear to me what the "right" behavior is in either of these cases. Since the implementation logic is fairly straightforward, I'm tempted to say that this is all intended behavior... but I thought I'd raise the issue just in case.
It's also unclear what the behavior should be with DocumentApp. That's a case where there isn't a main_window, so it's not clear which window the merge should operate on.
| @@ -297,6 +303,17 @@ def _create_app_commands(self): | |||
| group=toga.Group.HELP, | |||
| ), | |||
| ) | |||
| if macos_version_at_least(10, 12): | |||
There was a problem hiding this comment.
Good thinking, but probably not necessary. As a general rule, we don't support OS versions that the vendor themselves doesn't support; 10.12 (Sierra) has been EOL for 4 years.
Admittedly, there's a discrepancy with the main README, which says Yosemite is supported; however, the Briefcase support packages for packaged apps set a hard minimum of 11 (Big Sur), so this is as good an opportunity as any to update those docs and avoid the extra branch.
There was a problem hiding this comment.
Oh, yeah I was going off of the readme. Duly noted.
| app, main_window, second_window, second_window_probe | ||
| ): | ||
| if toga.platform.current_platform != "macOS": | ||
| pytest.xfail("Merging all windows is only implemented for macOS.") |
There was a problem hiding this comment.
We generally try to avoid having platform-specific tests in the testbed, instead writing a generic test,. and putting the xfail somewhere on the proble when the platform doesn't support that action (e.g., an "window_probe.perform_window_merge()" command).
This is somewhat similar to best practice in browsers - rather than looking for a specific version of a browser ID string, you test for the feature you need.
| toga.Command( | ||
| self._menu_merge_all_windows, | ||
| "Merge All Windows", | ||
| group=toga.Group.WINDOW, |
There was a problem hiding this comment.
This results in a "merge all windows" menu item that comes before "minimize". From what I can see, Minimise should be in a section, with "merge all" in a separate, higher-valued section.
At the risk of scope creep, I'd be interested in seeing how many of the other standard macOS window commands could be implemented.
There was a problem hiding this comment.
Ah, good point. I need to take more of a look at how Toga organizes commands.
|
I do think those seem like intended behaviors, but I should probably do some experimentation in some other apps and see if it's consistent. And I hadn't thought about DocumentApp, that's a good question. |
|
Okay, I've stared at things and I think I have a clearer sense of how/where to properly arrange tests and behavior. However, on the subject of conceptual / intended behavioral questions, I'm now pondering a rather sticky one. Native multi-window macOS apps don't typically have a main window; any Finder window, for instance, is just as "important" as any other. So it's fine that Merge All Windows apparently does consistently merge into whatever window is foremost, and tabs can be arbitrarily reordered. I'm not sure how best to integrate Toga's MainWindow concept into this; after a Merge All (or even after opening a new "window" or two in auto-tabbing mode), the main window will just be a tab. It will look just like the others aside from its title, and it could be at any position in a whole bar full of tabs, but it's still going to be the one and only one tied directly to the lifespan of the whole app. That could be a big gotcha, to developers and users both. Any thoughts? And at the risk of way more scope creep in this discussion, do you have any plans for adapting Toga to support persistent, main-window-less apps? I see that touched on some in #97 and #2210, but I'm not sure what your direction looks like there. Sometimes you tug one thread... @_@ |
That's definitely true. I guess the use case we're talking about here isn't especially common in common usage. I can't think of many examples of apps that do have multiple windows (i.e., don't have a "main" window), but don't adhere to the "all windows are equally important" model.
The only thought I've got is to lean into this exact distinction. If this is a menu option that only makes sense for document-based apps... let's only add it for document-based apps. The list of items in the Window menu is dramatically different on Safari compared to a "simple" app like Contacts or Maps. Perhaps the "fix" here is to lean into that, and only provide the option where it has the potential to make some sort of sense.
I do, and I have a branch on my repo that implements this, as part of a fix for #97. I've still got some tinkering to do with that branch (including tests, and working out what the everliving (#&!(*& the GTK team is smoking when it comes to notification icons). That branch isn't quite ready to be a PR yet, but it will add:
Well that'll learn ya to tug on things... :-) |
I guess that depends how abstractly one defines a "document" app. Is Finder (or any other file-system-viewing app) a document app, in the sense that it doesn't have a main window, each window is an equally important view of something, and also they can tab together arbitrarily? Is it possible that perhaps DocumentApp should be a particular type of "egalitarianly-multi-window-app"? ...Actually for that matter, is "egalitarianly-multi-window-app" just a WindowlessApp that can open multiple windows? @_@ At least for main window apps, and for the time being, what if secondary windows can tab with each other, but the main window stands alone? I'd have to play with it but from looking at NSWindow, I think that might be doable. It's still an idiom that feels strange on a platform where apps are (usually? always? almost always?) either persistent (with one or more windows) OR tied to the lifespan of one window (and that's their only window). It would definitely be closer though, and I think more intuitive, to have the main window remain clearly separate, and the others able to behave differently as subsidiaries. |
I'd argue that that's exactly what Finder/Safari are. To my read, a Document-based app is an app where there is 1-1 correspondence between a physical file on disk, and a DocumentMainWindow. On Cocoa, that means you end up with multiple DocumentMainWindows, and closing the last DocumentMainWindow doesn't close the app. I'm still trying to work out what DocumentMainWindow means in a non-macOS context - I'm not sure if a GTK/Windows DocumentApp should allow multiple DocumentMainWindows.
Is that sort of behavior even configurable? My understanding was that this was an OS-level behavior, not something an app develop could tweak into different app groups, etc... |
Okay yeah, that makes sense. Definitely a confusing thing to try and architecture across platforms that handle things differently.
I haven't tried it yet, but in addition to the class variable
So no, I'm not positive, but the way these are worded and the fact that they're instance variables makes me suspect. Unless you've already tried them and ruled them out, I'll fiddle around more when I can find the time. |
I haven't tried anything here - this is all new territory for me. However, this does make me wonder if the fix is to make the tabbing identifier the Window's class name (or something derived from it). That way, generic child windows will all tab together, but not with the Main Window; Document windows would all tab together; and a Windowless app that had a single Window base class would all tab together... |
|
That sounds entirely reasonable to me — I'll try that out when I next have time. |
|
Superceded by #2311 |
I've added a menu item, Window > Merge All Windows, that merges all windows into one tabs on the main window, like how it works in Finder and Safari.
If the user's OS-level "Prefer tabs when opening documents" setting is "Always", Toga gets tab creation for free, as well as the ability to drag tabs to reorder them and even move them to an existing tab bar on another window. However, this UI experience is oddly incomplete, and inconsistent with native apps, without the ability to merge existing windows into tabs.
Potential issues:
NSWindow.mergeAllWindowsis only available in macOS 10.12+. I've put in a check to only add the menu item when the method is available, but since the CI never runs on macOS older than 10.12, it never exercises the branch where it's not added. I marked it for Coverage to skip — hope that's okay.app.py. I'm amenable to putting it anywhere that would make more sense.PR Checklist: