chatterino2 icon indicating copy to clipboard operation
chatterino2 copied to clipboard

Add tray icon

Open univrsal opened this issue 3 years ago • 29 comments

Pull request checklist:

  • [x] CHANGELOG.md was updated, if applicable

Description

Added an optional tray icon. The tray icon can be used when the main window is minimized, closed or both. Clicking the tray icon will toggle the visibility of all windows. Right clicking it will open a context menu to show or close the application so users have an option to exit if they set Chatterino to hide when closing the main window. image

I only tested this on linux with KDE, so it would have to be tested on other platforms as well, but I assume Qt takes care of the tray icon fairly well. I hope I put everything in the right place, I didn't really know where to put the enum for the tray icon setting, so if there's some issues I'd be happy to correct them.

Closes #1358

univrsal avatar Mar 07 '21 16:03 univrsal

(Merging this closes #1358.)

leon-richardt avatar Mar 07 '21 16:03 leon-richardt

While minimized to tray, closing any other visible window such as usercard or settings dialog results in application quitting - this should not be a case.

This doesn't happen for me, so maybe it's only an issue on windows? I'll try and test it on Windows later on.

Right now, minimizing the application to system tray via tray icon, hides only main window and all other windows like settings dialog, usercards, emote picker, etc. stay open.

This one is a bit more complicated. I switched to a signal for hiding/showing windows which seems to have fixed the issue mostly from what I can tell. The only issue left is that the settings dialog is always set to visible when clicking the tray icon, but that should be fixable.

univrsal avatar Mar 08 '21 22:03 univrsal

This doesn't happen for me, so maybe it's only an issue on windows? I'll try and test it on Windows later on.

I am an arch user, btw. Maybe it's because of i3wm.

zneix avatar Mar 08 '21 23:03 zneix

This doesn't happen for me, so maybe it's only an issue on windows? I'll try and test it on Windows later on.

I am an arch user, btw. Maybe it's because of i3wm.

Could be, KDE integrates well with Qt applications. I'll try to test it on i3 and see if I can find a fix.

univrsal avatar Mar 09 '21 16:03 univrsal

How does this handle multiple Chatterino windows? This is mostly a thing for Windows users who use the extension, or StreamLink (or other external viewer programs) users who might start Chatterino with the -c flag?

Getting the behaviour right for most users will be a difficult task. I believe the setting(s) should be more akin to: Behaviour when pressing the Close button and Behaviour when pressing the Minimize button rather than a single setting trying to control both behaviours. In addition, the default value for Close should be "ask users for future preference" (not sure how that would be shown in the settings page), and the default value for Minimize should be the old behaviour (so just minimize to the task bar, not to the tray)

pajlada avatar Mar 14 '21 12:03 pajlada

How does this handle multiple Chatterino windows? This is mostly a thing for Windows users who use the extension, or StreamLink (or other external viewer programs) users who might start Chatterino with the -c flag?

It only reacts to the main window closing or being minimized. I wanted it so I could keep the program open and still get live notifications. I guess it should automatically start minimized if the -c flag is used? It should maybe also save the state and load it on launch.

I believe the setting(s) should be more akin to: Behaviour when pressing the Close button and Behaviour when pressing the Minimize button rather than a single setting trying to control both behaviours.

Seems fair.

In addition, the default value for Close should be "ask users for future preference"

I honestly dislike programs that do that. I'd leave it at the expected behavior and just let users set it in the settings.

I also gave this a quick test on Windows, I'll be pushing some changes which seem to work fine with multiple windows, the settings dialog, tooltips and user chat windows.

univrsal avatar Mar 14 '21 12:03 univrsal

In addition, the default value for Close should be "ask users for future preference"

I honestly dislike programs that do that. I'd leave it at the expected behavior and just let users set it in the settings.

Yeah if there is a valid default that works for most people I agree, but this is a pretty polarizing "feature", and the "ask users what their future preference is" has become pretty standard for some applications at least (I think Discord/Spotify at least)

pajlada avatar Mar 14 '21 13:03 pajlada

How does this handle multiple Chatterino windows? This is mostly a thing for Windows users who use the extension, or StreamLink (or other external viewer programs) users who might start Chatterino with the -c flag?

It only reacts to the main window closing or being minimized. I wanted it so I could keep the program open and still get live notifications. I guess it should automatically start minimized if the -c flag is used? It should maybe also save the state and load it on launch.

That's my bad - I thought the tray icon was always visible a la Discord, but if the tray icon only shows up when you minimize or close the application (depending on the user settings) that makes more sense to me.

One thing to take into consideration with multiple windows, still, is that if Chatterino is started with the -c flag, settings aren't saved so asking the question for future actions wouldn't make sense

pajlada avatar Mar 14 '21 13:03 pajlada

How does this handle multiple Chatterino windows? This is mostly a thing for Windows users who use the extension, or StreamLink (or other external viewer programs) users who might start Chatterino with the -c flag?

It only reacts to the main window closing or being minimized. I wanted it so I could keep the program open and still get live notifications. I guess it should automatically start minimized if the -c flag is used? It should maybe also save the state and load it on launch.

That's my bad - I thought the tray icon was always visible a la Discord, but if the tray icon only shows up when you minimize or close the application (depending on the user settings) that makes more sense to me.

One thing to take into consideration with multiple windows, still, is that if Chatterino is started with the -c flag, settings aren't saved so asking the question for future actions wouldn't make sense

Currently it is always visible, but I guess it'd make more sense to only show it when Chatterino is hidden.

univrsal avatar Mar 14 '21 13:03 univrsal

How does this handle multiple Chatterino windows? This is mostly a thing for Windows users who use the extension, or StreamLink (or other external viewer programs) users who might start Chatterino with the -c flag?

It only reacts to the main window closing or being minimized. I wanted it so I could keep the program open and still get live notifications. I guess it should automatically start minimized if the -c flag is used? It should maybe also save the state and load it on launch.

That's my bad - I thought the tray icon was always visible a la Discord, but if the tray icon only shows up when you minimize or close the application (depending on the user settings) that makes more sense to me. One thing to take into consideration with multiple windows, still, is that if Chatterino is started with the -c flag, settings aren't saved so asking the question for future actions wouldn't make sense

Currently it is always visible, but I guess it'd make more sense to only show it when Chatterino is hidden.

sorry for the double misunderstanding! :D

I think, for a single Chatterino window, it makes sense to keep the tray icon visible at all times, especially if we want to expand on the tray icon usage to show things like unread messages, have a place to spawn notifications from etc.

I think the question still is how tray icons behave with Chatterino windows other than the main window.

Maybe if Chatterino is started with -c no tray icon should be visible and close button/minimuze button has the same behaviour as now when no tray icon exists. Same thing if Chatterino is running in "extension mode", we should also make sure no tray icon is visible

pajlada avatar Mar 14 '21 13:03 pajlada

I think, for a single Chatterino window, it makes sense to keep the tray icon visible at all times, especially if we want to expand on the tray icon usage to show things like unread messages, have a place to spawn notifications from etc.

I think the question still is how tray icons behave with Chatterino windows other than the main window.

Maybe if Chatterino is started with -c no tray icon should be visible and close button/minimuze button has the same behaviour as now when no tray icon exists. Same thing if Chatterino is running in "extension mode", we should also make sure no tray icon is visible

Is the gui even started if it's running in extension mode? From what I can see here it doesn't look like it so the icon shouldn't show up. Where is the -c flag checked? And what does it do? Also what do you mean by a single Chatterino window? No popup and user chats?

univrsal avatar Mar 14 '21 15:03 univrsal

I know this isn't quite done, so I just wanted to quickly ask what I should do so this could be merged (if it's even good enough to be merged).

univrsal avatar Mar 29 '21 20:03 univrsal

These are the changes that are required for further review: In the settings dialog, instead of having them be checkboxes, it should instead be a dropdown menu of actions.

The actions possible for the "What should we do when you press the Close button" are:

  • "Ask me" <-- DEFAULT OPTION
  • "Minimize to tray icon"
  • "Close Chatterino (Standard behaviour)"

The actions possible for the "What should we do when you press the Minimize button" are:

  • "Ask me" <-- DEFAULT OPTION
  • "Minimize to tray icon"
  • "Minimize to task bar (Standard behaviour)"

The "Ask me" option should open a dialog asking the user what they would like to do, and saving that option to the setting.

After that there's some thinking/testing to be done with regards to multiple Chatterino windows and how that behaviour works, in particular we will need to handle "Ask me" differently if Chatterino is running in a mode where it does not save settings.

Alrighty, I'll try to find some time next week. Does "Ask me" need to be an option in the settings dialog? Wouldn't it be enough to show this dialog the first time when the user minimizes/closes, apply the users choice to the settings dialog and leave it at that?

univrsal avatar Apr 04 '21 19:04 univrsal

It doesn't necessarily need to be an option in the list, but I thought it might be the simple way to do it code-wise. Maybe it just shouldn't render in the list. So for action, since you'd never want to set it to "ask me", just default to minimize to task bar and let the use change it in the settings if they'd like. For close we ask still

pajlada avatar Apr 05 '21 00:04 pajlada

hey was there any update on this? curious since i was searching around for exactly this.

pritamh avatar Jul 17 '21 05:07 pritamh

No, I sadly haven't had time or motivation to work on this as of late.

univrsal avatar Jul 17 '21 10:07 univrsal

How about now, any updates? Using the setting that shows full window name in the taskbar for applications makes Chatterino take up a lot of space (especially when I'm just using it in the Twitch browser using the Chrome extension thing).

srnyx avatar Jan 31 '22 01:01 srnyx

How about now, any updates?

You can track the progress of this PR by following the commit history on this branch.

leon-richardt avatar Jan 31 '22 16:01 leon-richardt

Well, better late than never: image image image

Works on my machine™

univrsal avatar May 09 '22 21:05 univrsal

@univrsal it should just say Minimize to tray instead of Minimize to tray icon

srnyx avatar May 09 '22 21:05 srnyx

@univrsal it should just say Minimize to tray instead of Minimize to tray icon

I just copy pasted that from pajlada's message.

univrsal avatar May 09 '22 21:05 univrsal

@univrsal it should just say Minimize to tray instead of Minimize to tray icon

I just copy pasted that from pajlada's message.

He must have hit his head that day, including icon in the setting name sounds fucking stupid.

edit: I thought about this a bit and I realize from a conceptual standpoint, thinking of it with that wording does make sense, but the setting name should not reflect this.

Felanbird avatar May 10 '22 00:05 Felanbird

Initial thoughts (tested with both QtCreator build & nightly artifact):

  1. It seems the minimize setting does not properly hold the window to request what behavior you want, it always minimizes itself and then the prompt will appear next time you go to maximize it again, and because of this will re-minimize itself on confirmation. c2_2501_1

(It should be noted that, while clicking the C on your taskbar to minimize, the same issue arises, but the prompt does not follow and is still accessible.) image

  1. Clicking X on the popup forces the window into tray for both settings, and sets their respective BoolSetting to match. It should either abort and keep the settings set to Ask me, or use original defaults (minimize & close) and keep the setting on Ask me. c2_2501_2

  2. Closing Chatterino from the taskbar right-click -> Exit will force it back to the taskbar on next startup. c2_2501_3

  3. Unrelated to code but reminder, Changelog will have to be dragged out of the depths of hell, and stripped of it's issue #. (we only do PR #'s now).

Felanbird avatar May 10 '22 05:05 Felanbird

Initial thoughts (tested with both QtCreator build & nightly artifact):

1. It seems the minimize setting does not properly hold the window to request what behavior you want, it always minimizes itself and then the prompt will appear next time you go to maximize it again, and because of this will re-minimize itself on confirmation.
   ![c2_2501_1](https://user-images.githubusercontent.com/41973452/167548498-4b32ed67-da13-438a-bd29-8eac9399f01d.gif)

(It should be noted that, while clicking the C on your taskbar to minimize, the same issue arises, but the prompt does not follow and is still accessible.) image

2. Clicking `X` on the popup forces the window into tray for both settings, and sets their respective `BoolSetting` to match.
   It should either abort and keep the settings set to `Ask me`, or use original defaults (minimize & close) and keep the setting on `Ask me`.
   ![c2_2501_2](https://user-images.githubusercontent.com/41973452/167549281-5f428f49-f693-4a5c-b301-8521aedc2a0d.gif)

3. Closing Chatterino from the taskbar `right-click -> Exit` will force it back to the taskbar on next startup.
   ![c2_2501_3](https://user-images.githubusercontent.com/41973452/167547549-54d9844c-a6c4-4c91-8839-292f1805b268.gif)

4. Unrelated to code but reminder, Changelog will have to be dragged out of the depths of hell, and stripped of it's issue #. (we only do PR #'s now).

Looks like 1. and 2. are issues on Windows. I'll try and debug it. 3. is probably a leftover from when the first attempt, the idea was that it should remember whether the program was hidden or not, but I guess that's not really necessary/useful.

univrsal avatar May 10 '22 13:05 univrsal

So windows needed some extra code, because of the custom toolbar, but it seems to work now, I'll have to test it on linux again though

https://user-images.githubusercontent.com/8353672/167714926-a31782f3-4bcd-494b-9d41-b13a04933876.mp4

So things seem mostly fine on linux (KDE), two Issues I found were

  • Minimizing the message box opens another message box
  • Popup chats seem to show up on the main screen if they were on a secondary monitory when Chatterino was hidden

univrsal avatar May 10 '22 20:05 univrsal

when user minimize chatterino window by clicking on task bar, OS will minimize it by itself while chatterino (with AskMe mode) will still show its MsgBox. Suggested behavior in this case would be not showing msgbox and not changing AskMe settings.

That's how I've now implemented it on windows. I don't think there's an easy way to distinguish these events on linux though. So on linux clicking the taskbar icon just shows the message box.

univrsal avatar May 11 '22 20:05 univrsal

Alright, here comes the macOS review to make everything more complicated image

This PR in it's current form is problematic on macOS, as the concept of the menu bar (which is where the QSystemTrayIcon is put) differs quite a bit from the system tray on i.e. windows.

How this would be expected to work for a macOS application:

  • A single setting "keep in menu bar", that adds a persistent chatterino icon (preferably monochrome) to the menu bar while chatterino is running
  • If that setting is enabled, don't close the main chatterino window on clicking the close button and just hide it (like this PR is doing rn)
  • On pressing the close button for the first time, display a dialog asking if you want to keep chatterino in the menu bar
  • On clicking the menu bar icon, always focus the chatterino window
  • Don't change anything about the minimizing behavior, macOS has a separate space for minimized applications in the dock and there is nothing that chatterino should change about that (ofc still un-minimize when clicking the menu bar icon)

Since those changes are fundamentally different from the implementation on windows/linux, I would guess that they'll probably require a lot of separate code for each platform (I haven't looked at the code in-depth so maybe it's less than I expect?) but I am unsure whether it makes sense for you to include that in this PR, without having a macOS system to test on. I could theoretically PR into your fork's branch or just open a separate PR after these changes get merged in, for a macOS implementation, but the current code will definitely have to be hidden, at least in part, behind ifdef's for macOS.

LosFarmosCTL avatar May 11 '22 22:05 LosFarmosCTL

Since those changes are fundamentally different from the implementation on windows/linux, I would guess that they'll probably require a lot of separate code for each platform (I haven't looked at the code in-depth so maybe it's less than I expect?) but I am unsure whether it makes sense for you to include that in this PR, without having a macOS system to test on. I could theoretically PR into your fork's branch or just open a separate PR after these changes get merged in, for a macOS implementation, but the current code will definitely have to be hidden, at least in part, behind ifdef's for macOS.

Yeah I'd prefer it if I didn't have to deal with macOS, but I'd assume that the changes shouldn't be that complex.

univrsal avatar May 11 '22 23:05 univrsal

Yeah I'd prefer it if I didn't have to deal with macOS

understandable 😄

Then I'd say it's probably easiest for you to just make this PR non-macOS only and for me to open another one afterwards adapting it.

LosFarmosCTL avatar May 13 '22 09:05 LosFarmosCTL

Is this supposed to be live on non-macOS platforms?

ghost avatar Oct 22 '22 20:10 ghost