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

feat: add snap config item for tray-icon; disable by default #200

Merged
merged 2 commits into from
Nov 25, 2023
Merged

feat: add snap config item for tray-icon; disable by default #200

merged 2 commits into from
Nov 25, 2023

Conversation

jnsgruk
Copy link
Member

@jnsgruk jnsgruk commented Nov 17, 2023

This PR introduces a new snap configuration option tray-icon. When tray-icon is set to true, the signal-desktop binary is launched with the --use-tray-icon flag.

The option is set to false by default.

Fixes #143
Fixes #153
Related: #195

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 21, 2023

@merlijn-sebrechts can you take a look at this? :)

Copy link
Member

@merlijn-sebrechts merlijn-sebrechts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced what the default should be. I'm expecting 99% of our userbase wants this turned on, so it might be better to default to "enabled".

I'm pinging some people out of band to get more points of view here.

snap/local/usr/bin/signal-desktop-wrapper Outdated Show resolved Hide resolved
@merlijn-sebrechts
Copy link
Member

On second thought, I think it should be disabled by default since upstream is getting a bunch of weird bug reports from our users complaining about broken systray support. Let's follow upstream on this to keep the peace.

So just address my one request and we can merge this, thanks @jnsgruk !

@jnsgruk
Copy link
Member Author

jnsgruk commented Nov 25, 2023

@merlijn-sebrechts good to go!

Copy link
Member

@merlijn-sebrechts merlijn-sebrechts left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@merlijn-sebrechts merlijn-sebrechts merged commit 6cdc284 into snapcrafters:candidate Nov 25, 2023
1 check passed
@jnsgruk jnsgruk deleted the tray-icon-config branch July 3, 2024 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants