albertlauncher / albert

A fast and flexible keyboard launcher
https://albertlauncher.github.io
Other
7.26k stars 305 forks source link

Don't hardcode tray icon #324

Closed bilelmoussaoui closed 7 years ago

bilelmoussaoui commented 7 years ago
Environent
Steps to reproduce

Just open the application and activate the tray icon

Expected behaviour

The tray icon should use the icon from the theme instead of hardcoding it https://github.com/albertlauncher/albert/blob/dev/src/application/resources.qrc

Actual behaviour

I think using QIcon::fromtheme will fix this ;) this way themes designers can provide a monochrome tray icon that will fit with the rest of their icons :+1:

ManuelSchneid3r commented 7 years ago

Is there a mechanism to differentiate tray icons from application icons on linux?

bilelmoussaoui commented 7 years ago

Just use a different icon name, as it's a Qt application, i guess that using albert-tray.svg would be fine! The icons also should be put on /hicolor/status folder ;) There's really no standards for the naming of tray icons! As few linux distros want to kill them. As they're from the past :) anyway, almost every Qt application uses *-tray!

ManuelSchneid3r commented 7 years ago

Okay the tray solution is too much of non-standard tweaking, but I agree that the icons should be themed.

bilelmoussaoui commented 7 years ago

Well, it's linux :p I can create a PR to fix this if you wish ;)

ManuelSchneid3r commented 7 years ago

@bil-elmoussaoui WIP

ManuelSchneid3r commented 7 years ago

3223a70856c08ee3177edefa5ea6034d7e7aeec2

bilelmoussaoui commented 7 years ago

well, using albert isn't a solution :( as it won't work for almost every theme. The Qt way to look for icons in themes that follow Freedesktop standards is quite broken :( using the same icon name for both the application and the tray icon will cause some issues. The application icon might be used instead of the tray icon even if the theme used ships both of theme... This issue is with every Qt application out there, i have been chasing those issues and fixing them for the last two weeks! Using a name like albert-tray or whatever you wish is better! I can show you some screenshots if you wish!

ManuelSchneid3r commented 7 years ago

Qt way to look for icons in themes that follow Freedesktop standards is quite broken

yes, that why I implemented it myself.

The application icon might be used instead of the tray icon even if the theme used ships both of theme

This is per spec. In the application context the generalization (albert-tray-icon -> albert-tray -> albert) is not applied.

This issue is with every Qt application out there

Which so use this tray solution?

bilelmoussaoui commented 7 years ago

https://github.com/MaartenBaert/ssr was using the same icon name as the tray icon name; the issue was fixed recently, the same for qBittorrent.

ManuelSchneid3r commented 7 years ago

Hmm I will think about it...

bilelmoussaoui commented 7 years ago

Any news?

ManuelSchneid3r commented 6 years ago

Is this tray thingy part of the standard?

bilelmoussaoui commented 6 years ago

It's not at all. But using the same icon name for both tray icon and the application icon is not the best idea at all :( If the application icon is shown somewhere as small resolution (a huge dock with a lot of apps) the icon shown will be the indicator one :( There's no possible way to force using a specific context (status or apps) in this case.

The best idea, and what's used by a lot of apps out there is to append a suffix to the icon name; something like -indicator or -tray or whatever you wish :) Or even using a different icon name, MegaSync in their latest version uses megalogging megapaused megasyncing as tray icon names!