eivindveg / HotSUploader

JavaFX-based Replay Uploader for Heroes of the Storm
Apache License 2.0
185 stars 36 forks source link

Minimize to tray #16

Closed allanjackson closed 8 years ago

allanjackson commented 9 years ago

It would be really nice if we could minimize the Mac application to the menu bar instead of having it remain in the dock.

eivindveg commented 9 years ago

Feature to minimize to tray is planned. :) Thanks for creating this issue for tracking.

eivindveg commented 9 years ago

Pushing up to 1.1 due to scope reduction of 1.2

eivindveg commented 9 years ago

Reopening as #19 only has sweet integration in Windows. Will be investigated for a possible 1.1.1 release or brought up at a later date.

zhedar commented 8 years ago

The system tray integration on OSX already works, I just tested this by removing the Windows condition at the tray addition. But I actually dont think the Dock icon should be hidden, when the app is minimized. This just isn't the way Macs work.

Also I needed some time to spot the little tray icon at the top of the screen and Mac users could be confused by the lack of the Dock icon, although they didn't close the program.

If you really want to hide the icon though, it might be achieved by putting

 <key>LSUIElement</key>
 <string>1</string>

in the plist. This method was bugged once, but should be fixed by now. Maybe there is a way to add this key by the packager? Note that this would disable the icon all the time, which may not be desirable. I don't think there's a way to temporarily disable it.

Also see the comments on https://bugs.openjdk.java.net/browse/JDK-8093206

eivindveg commented 8 years ago

One of the problems we've had is that the dock icon does not present the context menu we'd expect from the notification icon, and no direct ways to add this(to my knowledge). Insight here is welcome. In the meantime, I'm creating a branch for you to build from to test expected behaviour(my OSX availability is severely limited). I'll notify @FredrickB to test builds as I test.

eivindveg commented 8 years ago

Scratch that; this is currently an issue we cannot properly navigate, as per https://github.com/javafx-maven-plugin/javafx-maven-plugin/issues/152

eivindveg commented 8 years ago

@zhedar, could you please investigate the current functionality of the tray icon in the develop branch and let me know how that feels?

Current features: Hidden(I know, sorry) dock icon when Stage is closed. CMD+Q exits the application completely when Stage is showing and active Dock icon is not black/white - black icon will be added for OSX Tray specifically(White for Windows)

zhedar commented 8 years ago

First things first: the tray icon looks fine, it seems compliant to the design standards:

tray icon

Current hidden stage state:

current hidden stage state

Also why did you choose to show the window at the tray icon onclick, when there is a 'Show'-Button? If you misclick on the icon, the uploader will be shown. This seems rather unintentional.

Another idea on the tray: I'm not sure about this, but maybe the current state could be shown in the tray icon, like uploading or idle or even things like 500/501 uploaded files. That could function like a quick view function that seems neat.

eivindveg commented 8 years ago

Could you re-confirm that your OSX version is up-to-date and that tray icon logos in that version are supposed to be white, not black? @FredrickB's machine had black icons during testing, but his OSX is outdated.

I haven't tried anything to hide it; I was under the impression that it was hidden while the Stage was. Reason I want to hide it is that I can't trigger any interaction with that icon. Is there any way you can reshow the Stage whilst it's hiding without clicking the notification icon?

The show-on-click in notification icon should only count for double clicks. Can you confirm this?

Will see if I can add a String-Property for the status.

eivindveg commented 8 years ago

As for dock icon - I intended this to be black for OSX, but if the icon is supposed to be white, I'm a bit stumped on the colourations.

zhedar commented 8 years ago

I'm using the newest OSX 10.11 El Capitan. But I modified my system tray long time ago to use a dark color scheme. Apple also introduced dark mode as of OSX 10.10 Yosemite, which looks much nicer imo, thats why there are two different color schemes. With the old scheme enabled the logo isn't visible anymore:

Most applications seem to provide two different logos for this case (a dark and a white one), I'm just not sure how they change them, because they instantly change on color scheme changes.

No, it's not hidden, because the design default on OSX is to not hide the icon. There also is no way to reshow the application, when it was closed by pressing the 'X'. This is also not the way the application should react.

Yeah, it gets shown on double clicks. But all the other tray icons close on a second click and won't show an application. That's why it attracted my attention.

eivindveg commented 8 years ago

Alright. So since I can't change logo colours(afaik), I should use a black icon for dock and notification area on OSX?

I'll once again look into hiding the dock icon, but I doubt I'll have much luck.

Confirm that clicking the tray icon is supposed to toggle, not open, the showing state?

zhedar commented 8 years ago

Maybe we could use a white or black logo with the inverse color as a border? So it would be visible atleast. I may test the black icon, but I would prefer a white one with that color scheme. I will have a look, if there's a way to distinguish the themes once and set it then.

All of my trax icons behave this way, but maybe I find a design guideline for this.

eivindveg commented 8 years ago

Merged the branch. Can update the logo to a white one with black borders or a multiple iconset(if that's possible) if needed closer to release. Could you test the current develop branch? Logos should be fully updated at least. I couldn't get the toggle working without great hazzle, so I set it to only accept double clicks.

zhedar commented 8 years ago

With the block logo and dark scheme it's the same hassle is with the white on the light one:

I will have a look at how to detect these schemes, otherwise a multicolored logo may be the only way to make at visible on all systems.

The dock icon also looks a bit pixelated, because I'm on a retina mac (seems like I have some hardcore testing device here :D), but that's something that shouldn't be prioritized too high.

The double click listener feels better though.

eivindveg commented 8 years ago

If we're able to detect the settings from the Java app, there's nothing stopping us from rebuffering the PNG image and invertering colours as needed post-load, by the way. That might be a viable focus.

Sent by Outlook for Android

On Fri, Nov 6, 2015 at 3:04 PM -0800, "Philipp G." notifications@github.com wrote: With the block logo and dark scheme it's the same hassle is with the white on the light one:

I will have a look at how to detect these schemes, otherwise a multicolored logo may be the only way to make at visible on all systems.

The dock icon also looks a bit pixelated, because I'm on a retina mac (seems like I have some hardcore testing device here :D), but that's something that shouldn't be high prioritized.

The double click listener feels better though.


Reply to this email directly or view it on GitHub: https://github.com/eivindveg/HotSUploader/issues/16#issuecomment-154570745

zhedar commented 8 years ago

Theming seems to be done by templating in objective-c and is still an open problem for java apps. But this SO answer uses the a CLI to access user defaults. defaults read -g AppleInterfaceStyle indeed returns Dark on my machine and throws an error if the light theme is used. The suggested java code works as well. It feels a bit hacky though, because you need to invoke an external program, but since you created the specific PlatformServices, that should be in the acceptable scope.
Edit: Submitted a PR with this solution for you to review, already tested on OS X with both theme variants.

eivindveg commented 8 years ago

Anything else we need to review on this topic before we consider it fixed? Is this useable as is on OSX?

zhedar commented 8 years ago

While the tray icon integration works, the Dock icon is still there. While I appreciate this a bit, wasn't this the thing the issue was about?
Also, when closing the application by pressing the x button, the app will now remain in the dock (and will continue to run), but can't be brought up again by clicking on the dock icon. Only by using the tray menu, which is quite annoying.

eivindveg commented 8 years ago

Yeah, that sounds like a showstopper.

zhedar commented 8 years ago

It doesn't look like it would be triggered by the tray integration itself. If I comment out the calling of Client.addTray(), things get worse, since you can't quit the application anymore, if it's already hidden. Did you change application close handling elsewhere?

eivindveg commented 8 years ago

Closing the last stage calls Platform.exit(), which I haven't bothered investigating why is broken. However, if you look at https://github.com/eivindveg/HotSUploader/blob/develop/src/main/java/ninja/eivind/hotsreplayuploader/Client.java#L94 you'll see that the application expects a PlatformNotSupportedException in order to fix this.

zhedar commented 8 years ago

I must have overseen that. I will investigate our options on this issue then.

The broken Platform.exit() could be cause by running several background threads (some services + AWT thread now with the TrayIcon). A quick Thread.activeCount() showed 13, so maybe a proper exit routine should close them first?

eivindveg commented 8 years ago

Well, let's try hacking the plist. Could you extract the info.plist file from the .dmg file, upload it somewhere and paste me the link?

Or, if you want, submit a pull request adding it to src/main/deploy/package/macosx

zhedar commented 8 years ago

That's the generated plist's content.

I already tried adding

<key>LSUIElement</key>
<string>1</string>

to the generated plist with this suggested workaround, but the Dock icon won't disappear.

Also com.sun.javafx.application.PlatformImpl.setTaskbarApplication(boolean) is no option, since it's not API and Maven buils would fail.

Seems like we're currently quite stuck on this one.

eivindveg commented 8 years ago

Okay, so, for OSX, if we want to keep the tray icon at all, we need to either make the close icon minimize the app, disable the close icon, or see if we can find a way to make the dock icon pop the stage back up. Does this reasoning seem agreeable?

eivindveg commented 8 years ago

Could you see if you can do anything with this?

https://blog.codecentric.de/en/2015/04/tweaking-the-menu-bar-of-javafx-8-applications-on-os-x/

zhedar commented 8 years ago
  1. Keeping the tray icon should be right, so we can add additional functionality to it later on. Minimizing the app on close may be ok, because most applications normally won't really close without pressing CMD + Q anyways.
  2. Interesting approach, although that would mean introducing another depedency for swt. But it seems, that you cannot do much about the Dock this way, atleast there aren't any necessary methods in NSDockTile.
eivindveg commented 8 years ago

https://www.cct.lsu.edu/~rguidry/eclipse-doc36/org/eclipse/swt/internal/cocoa/NSWindow.html However, it is possible we can switch around the window behaviour here.

Also, the EventListener for dock icon may include a click. Something along the lines of "if dock is clicked and stage is hiding, show the stage"

eivindveg commented 8 years ago

Furthermore, it might be time we start proguarding the jar before we package it. The important part would be never touching SWT for platforms it isn't used. Using SWT might allow me to fix the tray on some Linux distributions(it really was horrible)

eivindveg commented 8 years ago

To clarify regarding ProGuard: the intention is minification only, not obfuscation.

zhedar commented 8 years ago

I've no experience with proguard yet, so I'm not sure, you can separate the depedencies by os usage, especially if we check the used OS' at runtime. I wouldn't like this to overcomplicate things (application architecture and build process), but it may be worth a try. Same for SWT, this could introduce more complexity, but if you can get of AWT with that, we may be fine.
Nethertheless hiding the Dock icon seems to be a problem every java UI framework has had problems with, there was also an issue for SWT.

eivindveg commented 8 years ago

We don't need to filter dependencies as long as we can prevent the cruel bloat we get by adding them freely.

Getting rid of the dock icon won't be possible until JDK9 at the earliest. This means we should focus our efforts on the dock icon not adding piles of confusion("I closed the window and now I can't open it anymore") by making sure it can at the very least reopen the window. Otherwise we may have to disable the tray icon for OSX or add a warning prompt when closing the window.

eivindveg commented 8 years ago

Starting to get ready for a release now. Any news on this? How would simply minimizing the application on close work?

zhedar commented 8 years ago

Miniziming on close would work for me, (but not on CMD + Q of course). However calling stage.hide() or setIconified(true) freezes the app for me. But while testing this I can no longer hide the application by pressing CMD + H, did commit 52ce01b5f4cf51d12e9d3b047bd58030377d3f12 do this? That only spaws this dialog: (in native package and jar)

bildschirmfoto 2015-11-26 um 00 30 50

Edit: 52ce01b5f4cf51d12e9d3b047bd58030377d3f12 doesn't seem to be the issue. Problem still persists.
Edit 2: I have no idea, what is causing this. I get it even with an on old commit like 08e5020bf6aedc972bb11c83a4d8c4361633a0f7, but im quite sure it didn't happen back then. Can someone cross check this, maybe @FredrickB ?

eivindveg commented 8 years ago

I'm pretty sure that's AWT hitting us square in the face. Noticed(but couldn't see) that popup being listed in available Windows. Does CMD+H work for you if you disable the tray icon?

zhedar commented 8 years ago

No, it doesn't. I traced this back to cb5b244d22adcfcb725e43673aba1e01f6b35c2e, it was introduced by the Preloader. Now that I have found the source, I can look into this again tomorrow. No check needed by @FredrickB anymore I guess, I'm just buffled I didn't discover this before, because I'm used to minimize applications...

Edit: To specify it, it seems to be caused by the Stage primaryStage of the preloader. If that isn't shown, everything will work correctly. I already tried preloaderStage.close(); instead of preloaderStage.hide();, but that had no effect. As I have seen now, you already had that changed in the newest version.

eivindveg commented 8 years ago

Did some digging. The preloader seems aimed at embeddable applications(is this really still a thing?)

I've seen a few samples that implement a preloader/splash with a single stage preparing the stage components in init(), then adding them to the stage in start(Stage) before commencing heavy work. It should be a quick port, since the DI context blocks on its own init.

eivindveg commented 8 years ago

Also, does the notification in 52ce01b5f4cf51d12e9d3b047bd58030377d3f12 display when you close the window? If it does, we should alter the OSX service to add a small informational text about the stage closed bug.

zhedar commented 8 years ago

Currently the application only hangs, when trying to close the app (CMD +Q works as intended, but you can't quit by clicking x). That's something I already noticed, but forget to tell. Any hint, where I could start to track this down?

eivindveg commented 8 years ago

Close the app or close the window? Could you check if this is a problem when uploading less than ten replays? The tray icon should push a notification then as well.

zhedar commented 8 years ago

Closing the window. Did see such a notification yet, trying to reset my db.

eivindveg commented 8 years ago

If the AWT functions are bad, we should simply try porting them all to SWT using this POM layout: https://github.com/jendap/multiplatform-swt

eivindveg commented 8 years ago

Check if minimizing using CMD+H works in 5484c871c074cc3df9801adfeb840a2d34073c33?

zhedar commented 8 years ago

This change also seems to hang the application, but I also found that Stage.hide() doesn't work properly, when testing the minimize on close. But I also had it hang while reuploding some files, which didn't happen before. Sadly no exceptions, only a non responsive application with an busy cursor (sth. like the hourglass cursor). This is getting a bit messy :/

zhedar commented 8 years ago

primaryStage.setIconified(true); instead of primaryStage.hide(); works though!
Now that I have gotten the hiding to work again, I can confirm that the about dialog pops up on every hiding.

eivindveg commented 8 years ago

Oh shoot me. Hide is synonymous with close. Iconified should be used across the board. Does an iconified Stage reappear when clicked in dock?

eivindveg commented 8 years ago

For OSX that is. For Windows we want to use hide/close.

EDIT: Actually, iconified across the board.

zhedar commented 8 years ago

Yeah, you can bring an iconified application back by clicking on it's dock icon.
I rechecked the default on close action on most programs and it feels rather blurry. Eclipse quits completely on pressing x, Safari closes all it's tabs, but is still active. Mail also stays active, but rebuilds it's ui on showing again. iMessages keeps it full state, so does Calendar. The App Store exits completely as well. I thought there was a guideline for those actions... ... I guess we may keep the exit application on close then. Users will get used to it. It may be nice to have some thoughts of other Mac users though. How do you handle close on windows? Send to tray?

eivindveg commented 8 years ago

All actions should now be iconified in da2918e08f97c7d6ea8a0eb47b909b278b8ebabe

Please verify the following actions: Closing the window allows it to be reopened by clicking the dock icon The System Tray sends out a notification when the window is closed(no longer necessary if the first works) CMD+Q and CMD+H work as intended.

If the above actions work, I'm going to apologize for implementing this wrongly all along. I have spent countless hours dredging this issue, and I imagine you have as well. I hope this issue is now satisfactory to the degree that we can sign off on it. This application takes a long time to reload its state(the DIContext), so keeping full state should be no problem as long as we have a very low memory footprint(we finally do). As for Windows, it goes completely to tray.