-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Feature/systray #2812
Feature/systray #2812
Conversation
Remove the text, as it's icon only for now...
Just wondering if the icon approach is the right one here. I haven't used systray on multiple platform, but it doesn't seems to be an in sync behavior on Linux. You can have "blinky" icon in the systray, but I haven't noticed them much in the application window icon. |
Can you clarify what you mean by whether “the icon approach” being right @Bluebugs ? I’m not sure what aspect is being questioned. |
My understanding is that the current PR does use the application icon as the one for the systray icon. This might be limiting, especially because icon are usually "animated" or show some kind of state information by changing there content, which I don't think is necessarily expected for the application icon. Basically I am not sure that they have the same purpose. |
Thanks. The plan is that this will be the fallback but developers will be able to specify an override, I hope this makes sense (I do still need to add that API). |
That does make sense indeed. |
Icon replace is in and this seems feature complete. |
Seems tricky as it is such an OS-dependent API. I'm not even sure you can programmatically trigger system tray elements on most OSes. It might be good to add a couple of usage tests tho - to ensure it doesn't break if someone calls the APIs, or calls them in the wrong order, or updates the menu & icon every frame, and that the icon adjusts to theme-changes etc. |
But if we call any of these APIs then icons will appear and disappear from system tray, something we generally try to avoid in tests. Such testing should be in the |
Running fyne test already flashes up windows and changes the menus in the top bar, icons appearing and disappearing from the system tray isn't the end of the world |
Only on macOS IIRC. And it's a bit of a regression, we used to manage without visually interfering with the screen. |
Is not having the test in a blocker @stuartmscott? I would happily add some Os specific testing to the "systray" project, the only thing to test here really is that a fyne.Menu translates to a systray menu, I guess that would be possible? |
Sorry @Bluebugs I did an update and it invalidated your review :( |
Sounds good! The only other thing to check is that the Icon updates on theme change |
Wow now that is an interesting thought. I don't think any of our app/window icon code refreshes on theme change. |
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.
Remaining items could land in a follow up PR. This feature is in high demand so let's land it and get it in the hands of devs for testing.
Added #2842 to follow up on the icon theme change |
Jacob said continue without him, so I did |
Description:
Add support for system tray on macOS, Windows and Linux/BSD.
This adds a 6% increase in binary size, but I cannot find a way to do this with less.
Fixes #283
Checklist:
Where applicable:
go mod vendor
).