Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to hide tray icon on non-Mac (which has no tray icon) #11258

Merged
merged 4 commits into from Oct 30, 2019

Conversation

@t3chguy
Copy link
Collaborator

t3chguy commented Oct 29, 2019

@t3chguy t3chguy requested a review from vector-im/riot-web Oct 29, 2019
Copy link
Member

dbkr left a comment

I'm very confused between this option and the existing 'minimise to tray' option. Shouldn't 'minimise to tray' be all that's needed here? Or at least would be if it worked correctly, but it looks like when 'minimise to tray' is disabled, the tray icon is still shown but closing the window closes the whole app. Also the 'minimise to tray' is shown in macOS which it shouldn't be because there isn't one.

What I'd expect would be one option which is shown on Linux & Windows. By default it's enabled in which case the tray icon is shown, when the window is closed riot continues to run in the background and clicking the tray icon shows the window again. When it's disabled, no tray icon is shown at all and closing the window causes the app to exit.

@t3chguy

This comment has been minimized.

Copy link
Collaborator Author

t3chguy commented Oct 30, 2019

on Windows for example what if you want a tray icon but want the Close button to actually close rather than collapse to tray, clicking on the tray icon would minimize to tray.

On Mac the option should say Minimize to Dock, unchecking it makes X actually close the app fully.

As shown in Spotify, disabling minimize to tray does not get rid of the tray icon:
image
and identically in Discord:
image

@dbkr

This comment has been minimized.

Copy link
Member

dbkr commented Oct 30, 2019

On Mac the option should say Minimize to Dock, unchecking it makes X actually close the app fully.

ARGH - you're right! It definitely shouldn't be doing this - this just isn't a thing that apps do on macOS.

As shown in Spotify, disabling minimize to tray does not get rid of the tray icon:

Ah, surprising, but OK: should we not follow suit from Spotify & Discord then and just have one option that controls the behaviour when closing the window, and the option doesn't appear on mac at all?

@t3chguy

This comment has been minimized.

Copy link
Collaborator Author

t3chguy commented Oct 30, 2019

ARGH - you're right! It definitely shouldn't be doing this - this just isn't a thing that apps do on macOS.

Fair enough, am not a Mac user so do not know what behaviours are sub-par even as an option

Ah, surprising, but OK: should we not follow suit from Spotify & Discord then and just have one option that controls the behaviour when closing the window, and the option doesn't appear on mac at all?

I'm not against merging the two options and disallowing X = Close but show tray icon, @lampholder thoughts?

@dbkr
dbkr approved these changes Oct 30, 2019
@t3chguy t3chguy merged commit eab6ffe into develop Oct 30, 2019
5 checks passed
5 checks passed
buildkite/riot-web/pr Build #1171 passed (3 minutes, 24 seconds)
Details
buildkite/riot-web/pr/eslint-lint Passed (44 seconds)
Details
buildkite/riot-web/pr/i18n Passed (2 minutes, 7 seconds)
Details
buildkite/riot-web/pr/karma-tests Passed (3 minutes, 14 seconds)
Details
buildkite/riot-web/pr/pipeline Passed (3 seconds)
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.