-
Notifications
You must be signed in to change notification settings - Fork 12
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
Switch from systray to tray-item package #25
Conversation
This does a minimal change from the systray package to the tray-item package. This implementation can still be cleaned up a bit, but lets check first how well this works. This implementation drops libappindicator support and switches to KSNI on linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also drops support for the right-click menu with the "Quit" option, but I think that's probably okay.
Otherwise, this looks nice and straightforward! I'm trying to decide whether this warrants a major version bump or not... It feels like maybe it does?
I'd also love to hear from someone who can test this on X with an X tray (like polybar). I don't know if there's support for things like KSNI there? |
I did not realise that was in there, I think I can add it back with this package as well.
I'd say it's a grey area, not much a user can do from a maintenance perspective. If you were to bump the version, I'd suggest we first check for some other things that might warrant a major version bump as well. For example, my other PR is implemented in a backward compatible way right now, but I'd say it's nicer to change the config a bit, the same goes for a change like: #6, which I still think is an improvement structurewise. |
Yep, agreed, let's go ahead and make some of those config changes in a new release too! |
I am completely swamped right now, but probably have some time over the holidays to get the changes in. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to go ahead and merge this so we make forward progress at least! I'd still like to see a "Quit" menu item come back, but it's relatively low-priority.
Released as 1.7.0-rc.3 🎉 |
This change includes few but notable changes: - A significant upgrade to the `imap` crate. - Significantly updated parsing for buzz config files (#28), which includes support for user-configured icons. - Support for multiple folders (#24). - A complete swap of the library used for tray icons (#25). From a user's perspective, the main change is that config files need to be updated to match the new format. In particular, this means changing `accounts` from a set of named tables to an array of tables, and moving from `folder = "something"` to `folders = ["something"]`. See the README for an updated example config. Big thanks to @DanielVoogsgerd for the changes in this release!
This does a minimal change from the systray package to the tray-item package. The implementation can still be cleaned up a bit, but let's check first how well this works.
The implementation drops libappindicator support and switches to KSNI on Linux.
I'm not against the inclusion of the libappindicator support, however I think this would need a GTK event loop.
I tested this on waybar, and it seems to work for me.
Furthermore, I think the old implementation did not work on Windows and macOS, this one might.
fixsee also: #6EDIT: Changed the fix reference to see also, as it does not entirely fix this issue. It just makes it easier to implement it.