Martchus / syncthingtray

Tray application and Dolphin/Plasma integration for Syncthing
https://martchus.github.io/syncthingtray/
Other
1.49k stars 42 forks source link

Add "Use presets" entry to follow desktop theme #270

Open financelurker opened 2 weeks ago

financelurker commented 2 weeks ago

Relevant components

(I guess this selection is correct... Please give feedback, if otherwise.)

Is your feature request specific to a certain platform/environment? Please specify. I guess KDE plasma? (I am using KDE Plasma 6.0.5 with Qt 6.7.1 as of now)

Is your feature request related to a problem? Please describe. The current situation (with Syncthing Tray v1.5.4) is, that one can set "Use preset" for colorful icons and a specific setting for bright or dark themes. See: Screenshot_20240617_101530

When having the setting to "... (for bright themes)" and then switching to dark mode (using koi or something similar) the Icon isn't very visible anymore. The same thing vice-versa, when having "... (for dark themes)" set and switching to light mode. Somehow almost all icons from other applications switch their color/visibility when the themes change, but not syncthingtray-qt6.

Describe the solution you'd like It would be awesome that the tray icon can have a setting, where it automatically adapts to the current selected light- or dark-theme, like almost all other application tray icons do.

Additional context Like mentioned above, there seems to be an existing mechanism on how monochrome tray icons can automatically adapt to which theme (light or dark) is enabled.

Martchus commented 2 weeks ago

like most application tray icons do

That is not ture. Most apps (not Plasmoids, normal apps that have a system tray icon) I know, e.g. Stream, QMPlay2, qwgraph, Konversation and Wayland to X11 Video bridge just set a static SVG/PNG icon as their tray icon. The icon is usually just the normal application icon and usually looks ok in light and dark mode. That's also what Syncthing Tray is doing with its default colorful icon.

Somehow almost all icons from other applications switch their color/visibility when the themes change, but not syncthingtray-qt6.

Those are probably official Plasmoids or very popular applications covered by the Breeze icon theme. These official Plasmoids are special in the sense that they use an icon from the Plasma theme (or the regular Breeze icon theme as of Plasma 6 when I remember correctly). Popular applications might also be covered by Breeze. Other applications like Syncthing Tray have to bring their own application icon, though (and providing one for every light/dark icon theme out there is not really feasible). That's especially true for Syncthing Tray as it also needs different icons for different states - so icons are therefore rendered dynamically.


But yes, it would make sense to have an option that adapts automatically. However, I would really like to emphasize that this is additional work; this is simply not an official Plasmoid that can rely on icons from Breeze themes (or a very popular app with static icon that is also covered by Breeze or whatever icon themes your use). So I am removing the "like most applications" part.

Martchus commented 2 weeks ago

Note that this is not that easy - otherwise I would have already implemented this feature.

The problem is that Syncthing Tray simply doesn't know the panel's background color. I could just make it use the normal foreground color but it would break if one has e.g. light application styling combined with a dark panel (e.g. like the global KDE theme Breeze Dusk). I guess I could still add an option to use the foreground color of the current palette - even though it breaks in those cases of mixed styling.

And I could maybe do better for the Plasmoid where one can probably read the foreground color to use specifically via some KDE color scheme API. This is of course a bit annoying to implement because the colors/icons are dealt with in non-KDE-specific code.

financelurker commented 2 weeks ago

I am investigating the code of koi right now ( https://github.com/baduhai/Koi ) - they have a very small code base and as far as I could see they don't have any specific handling for their icons (but it also has that behavior of switching).

Although: In koi it's in so far limited, as they don't have the other configurations for icons (like different colors for different states, like switchthingtray has)

(take that with a grain of salt - I am totally new to Qt and all that stuff)

Martchus commented 2 weeks ago

They're indeed doing something color-scheme aware in their SVG-XML: https://github.com/baduhai/Koi/blob/master/src/resources/icons/koi_tray.svg?short_path=828a542

I'm not exactly sure how this works and whether it would also work in other environments than KDE but it is probably worth a look. (If it only works on KDE then I'm probably better off determining the foreground color in C++ code as I'm generating the SVG code there dynamically anyway.)

financelurker commented 2 weeks ago

Ah, alright - I missed that, because I only looked at the code where they handle QSystemTrayIcon and QIcon. Reading https://techbase.kde.org/Development/Tutorials/Plasma4/ThemeDetails the ColorScheme-Text SVG style attribute seems to be something KDE specific and probably won't work somehwere else... (although this is "Plasma4" documentation - but I guess this still works in Plasma 6)

Martchus commented 2 weeks ago

I implemented an option for this you might want to test. You can find it in the menu show in the screenshot you put into the ticket description.

financelurker commented 2 weeks ago

Wow, that was surprisingly fast... didn't expect that 👌🏻

So, I switched from aur/syncthingtray-qt6 to aur/syncthingtray-git (since I saw in the PKGBUILD that this package uses HEAD on the git repository).

You now have an additional option in the menu, see: Screenshot_20240618_071015

When selecting this option and switching themes (using koi, in my setup), well, it just works as expected and the tray icons now behaves like others in my sys-tray and adapt it's color-schema ✅

Just for the record, if anyone stumbles over some issues -> Here's my system setup, where this worked:

Operating System: EndeavourOS 
KDE Plasma Version: 6.0.5
KDE Frameworks Version: 6.3.0
Qt Version: 6.7.1
Kernel Version: 6.9.5-zen1-1-zen (64-bit)
Graphics Platform: Wayland
Graphics Processor: NVIDIA (proprietary drivers, 550.~)
Martchus commented 2 weeks ago

Glad to hear it works. I guess there's one tweak I could still do: In some themes the foreground/text color is actually not black or white but slightly gray. The koi icon would adapt to that and I think Syncthing Tray should do that as well for the black/white cases. And I need to test whether the Plasmoid works when the Plasma palette doesn't match the app palette.

financelurker commented 2 weeks ago

I can give it a try to test it, when you're ready with that tweak. Although I would need some detailed instructions on what to test exactly (I only have a top-level understanding of KDE and themes).

Martchus commented 2 weeks ago

Thanks. I guess there's no need for you to test these corner cases, though. I already implemented and tested them yesterday with the grayish Arc color theme. The case when the Plasma theme doesn't match the app theme was actually already working. Now there's just one bug left and that is that the rendering size and border thickness are not taken into account but I'll fix that, too.