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

Linux: Use tray_manager instead of system_tray; remove system_tray #1255

Merged
merged 4 commits into from
Apr 3, 2024

Conversation

ix5
Copy link
Contributor

@ix5 ix5 commented Apr 2, 2024

Using tray_manager for Linux as well allows ditching of the unreliable and seemingly less-than-well maintained flutter library system_tray and unifies desktop tray handling across platforms.

At the same time, use upstream tray_manager instead of a forked repo (which was stuck on 0.2 and did not contain e.g. leanflutter/tray_manager#38 ). This additionally enables building on Linux again since Tienisto's forked repo used to disable the Linux plugin: Tienisto/tray_manager@b37f5e0. This also works around issues with newer libayatana-appindicator versions: leanflutter/tray_manager#38

Caveats discussed previously still apply: #404

Mainly:

As for @TheGB0077 saying:

(I tried to make the tray manager work for Linux but it didn't work still)

I'm not quite sure what the issue was, but it seems to work okay now. The only thing to keep note of for me was to guard the call to setToolTip() since it would throw a MissingPluginException on Linux, but this was noted in the API docs of tray_manager.

For reference, potentially relevant prior discussions/commits:

Issues?

I have only tested this on an up-to-date Arch Linux system and cannot speak to the viability of tray_manager for other (older) distros.

It seems that the AppImage and Flatpak distributors as well as Debian-based systems might not be fully compatible with anything recent, but I cannot see how re-enabling tray_manager on Linux would surface any additional errors that weren't previously being thrown by system_tray.

My setup:

libappindicator-gtk3 12.10.0.r298-3
libayatana-appindicator 0.5.93-1
libayatana-indicator 0.9.4-1
libcanberra 1:0.30+r2+gc0620e4-3

With flutter installed through fvm:

$ fvm flutter --version
Flutter 3.13.9 • channel stable • https://github.com/flutter/flutter.git
Framework • revision d211f42860 (5 months ago) • 2023-10-25 13:42:25 -0700
Engine • revision 0545f8705d
Tools • Dart 3.1.5 • DevTools 2.25.0

ix5 added 3 commits April 2, 2024 19:15
And use upstream instead of a forked repo (which was stuck
on 0.2 and did not contain e.g.
leanflutter/tray_manager#38 )

This additionally enables building on Linux again since
Tienisto's forked repo used to disable the Linux plugin:
Tienisto/tray_manager@b37f5e0
This is a fixup of c734471 which did not re-generate the
necessary files
This allows ditching of the unreliable and seemingly
less-than-well maintained flutter library' system_tray' and
unifies desktop tray handling across platforms.

Note: This commit requires bumping of tray_manager from
0.2.0 to at least 0.2.2 in a previous commit and works
around issues with newer libayatana-appindicator versions:
leanflutter/tray_manager#38
@ix5
Copy link
Contributor Author

ix5 commented Apr 2, 2024

@Nixuge I see your efforts at packaging for the AUR. With this PR (for me locally) I can successfully use LocalSend with a tray icon again.

Note however that this is using the generated artifacts (flutter build linux --release) for packaging using a PKGBUILD and not extracting anything from the debian tarballs.

See #40

I'm not quite sure what to do about segfaulting AppImages (see #409) but interested in what others might report.

@Tienisto
Copy link
Member

Tienisto commented Apr 2, 2024

Thanks for your effort! I will test this ASAP

Superceded by usage of tray_manager on all platforms in a
previous commit

Also remove superfluous dart:io import (unused since no
exit() is performed any more)
@ix5
Copy link
Contributor Author

ix5 commented Apr 2, 2024

Splendid, thank you @Tienisto! You've created and maintained a great app, I'm happy to contribute.

Pushed with a fix for the CI complaining about unused imports (it was dart:io that I missed because I didn't realize exit(0) belonged to that lib... sorry, first time using flutter/dart)

@Tienisto
Copy link
Member

Tienisto commented Apr 3, 2024

LGTM!

@Tienisto Tienisto merged commit f45c5c9 into localsend:main Apr 3, 2024
3 checks passed
@ix5 ix5 deleted the linux/use_tray_manager branch April 3, 2024 14:51
@Nixuge
Copy link
Contributor

Nixuge commented Apr 26, 2024

Finally here, will try and switch the -bin aur package to use the deb/targz see if it works

@Nixuge
Copy link
Contributor

Nixuge commented Apr 26, 2024

Note however that this is using the generated artifacts

Those seem to work just fine anyways (see the -git AUR package) unlike the .deb & .tar.gz from the releases tab for some reason (no idea how you're making those @Tienisto?)

Anyways if you got rid of that dependency it's probably fixed anyways, thanks for the epic PR.

Got a PKGBUILD for deb support ready for when this gets into a release build

@Tienisto
Copy link
Member

The release binaries are all built on the Github Pipeline:

https://github.com/localsend/localsend/blob/main/.github%2Fworkflows%2Frelease.yml

@Nixuge

@Nixuge
Copy link
Contributor

Nixuge commented Apr 26, 2024

Aight thanks, did a test build and it seems to work perfectly now with the deb

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants