System tray

[dantelecan]

Hey Jack. I worked with @letalvoj on system tray feature. We think it's in a pretty usable state now. Please review the changes and merge them back if we're pleased with them.

[oconnor663]

This is so awesome! Works great under Gnome 3.10 (a lot better with the TopIcons plugin, which is true of a lot of apps).

I need to spend some more time looking at the code, but here are at least a few changes I'll want to make before I merge into master. Let me know what you guys think. I'll do them myself, or you can add them to the pull request if you like.

• Some trailing whitespace in a couple of files.
• I think we should set MinimizedOnStart to remember the last thing the user did with the window. So if the user hides it, it'll stay hidden the next time they starts the app, without requiring them to add a line to config.py. But they can still add the line, if they want to force a particular behavior no matter what they did last time. (The somewhat new, two-layered behavior of settings.py is a little confusing, sorry about that.)
• We should avoid having the name and default value for the TrayIcon setting duplicated in different places. Maybe add a small wrapper function to read it.
• Let's go ahead and default TrayIcon to True, since most users would never discover it otherwise. I don't think many people will want to disable it. That said, I should test it to see what happens in a tray-less environment like plain XMonad.
• closable is a boolean parameter, and I like to avoid those, since they usually make it harder to look at a callsite and guess what's going on. In other languages I'd prefer to use an enum, but I think in Python 3 a keyword-only argument would be best.
• I've tried to keep Qt-specific details confined to a few files, like event.py and browser.py. I've also tried to stick to the simple event model in event.py, rather than having lots of Qt signals around the code. Qt signals can have more than one kind of threading semantics, and I've run into problems before with the JS thread and the MQTT thread getting angry at each other. The other idea was also to make it easier to switch to a different toolkit, if that ever seemed desirable, but I'm starting to think this separation might have been foolish. Still, I wonder if there's a clean way to hide the Qt-specific and signal-specific details of create_sys_tray inside the MessengerWebView base class. I'm not sure yet.

Thanks again for implementing this!

[letalvoj]

Hi :smiley:

c) is my fault, I'm new to Python and I was not sure where to put a wrapper of the getter in your code.

And as I said before I do not think that there should be more than one way of minimizing. It is quiet confusing. I use Fluxbox and when I minimize it (normally) it hides to bottom bar but when I minimize it my closing it hides to system tray. It is not ergonomic because one should expect something be always on a very same place. :wink:

[dantelecan]

I agree with you Jack on all of the points. I'll change the things you mentioned and I'll let you know. Maybe the last bullet point will need a bit of research but we'll talk and find the best approach.

[oconnor663]

I'm sorry, one of my suggestions was ambiguous. I think we can default SystemTray to True in the code, by calling get_setting("SystemTray", True) instead of get_setting("SystemTray", False). Then we don't need to include anything but comments in the starter config.py file, which is probably a good thing.

Another random thought: Does having a tray icon mean that users will expect a "singleton" application? For example, if I hide the main window and then run fbmessenger again from the command line, should it talk to the existing process and tell it to show, instead of starting a second process with a second icon in the tray? I prefer the current, not-a-singleton behavior for simplicity, but there might be some common cases where allowing more than one process will confuse users, so I just want to make sure we think about it.

Apparently there is a method we can call to see if the tray is supported at all: http://pyqt.sourceforge.net/Docs/PyQt4/qsystemtrayicon.html#isSystemTrayAvailable We probably want to force the SystemTray = False behavior on these systems, or else there will be no way for the user to quit the app. The wrapper function for SystemTray that we talked about above will be a good place to put this check. I'm going to have to remind myself how to use startx for testing this...

[dantelecan]

I setted SystemTray to True directly in code.

Yes, it makes more sense to make it a "singleton". I think is is the way most chat apps act on Linux. This is a thing to keep in mind.

We should check for system tray availability, and I think this is a high priority thing to implement.

[oconnor663]

I just merged this, along with a few cleanup commits of my own. Thanks so much for putting this together!

Unfortunately, some bad news: It looks like the Facebook endpoints the app relies on might not be alive for very much longer. I really enjoyed working with you guys, and I'm sorry we're going to get cut off. I'll let you know more when I do.

[dantelecan]

It was a pleasure to contribute to this project. I'm sorry to hear that the support will be discontinued, it's a great application. :worried:

[oconnor663]

Actually, when I try this on Ubuntu, I can't see the tray icon anywhere. Qt still reports that the tray is enabled, though. What are you guys seeing?

[dantelecan]

I tested on Ubuntu 13.10 and I can see the tray but not the picture. It's acting like it doesn't find the picture.