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

Use icons from KDE theme #121

Closed
1 of 6 tasks
mrpink17 opened this issue Dec 15, 2021 · 14 comments
Closed
1 of 6 tasks

Use icons from KDE theme #121

mrpink17 opened this issue Dec 15, 2021 · 14 comments

Comments

@mrpink17
Copy link

Relevant components

  • Standalone tray application (based on Qt Widgets)
  • Plasmoid/applet for Plasma desktop
  • Dolphin integration
  • Command line tool (syncthingctl)
  • Integrated Syncthing instance (libsyncthing)
  • Backend libraries

Is your feature request specific to a certain platform/environment? Please specify.
KDE Plasma

Is your feature request related to a problem? Please describe.
The app uses custom icons that don't look "standard" in KDE Plasma, they are also a bit blurry

Describe the solution you'd like
Use icons from KDE theme

Additional context
Screenshot_20211215_231815

@Martchus
Copy link
Owner

Martchus commented Dec 16, 2021

I'm using ForkAwesome icons in consistency with Syncthing's official GUI. Especially for the icons within the expandable list items that makes also most sense in my opinion. For consistency with that I also like to use ForkAwesome for the icons close to that list view. That has also the advantage (over the system icon theme) that I can rely on the icons being actually present. The ForkAwesome icons are quite neutral themselves so they should fit in most icon themes (and follow your color settings which has been recently improved by 3e38a99).

Maybe it could be made configurable, e.g. adding an option to use the system's icon theme as much as possible. Note that I personally prefer the ForkAwesome icons so I'm not likely implementing this soon (as it is likely quite some effort).

@mrpink17
Copy link
Author

Yes, it would be great!

@stale
Copy link

stale bot commented Feb 14, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added stale and removed stale labels Feb 14, 2022
@sten0
Copy link
Contributor

sten0 commented Apr 5, 2022

Because Syncthing is a "killer app", I think that in the future the KDE project may be amenable to a PR that adds the required SVG icons to the base set. This would take care of question of icon availability. If a new enough version of Plasma is not detected at compile time, then qtforkawesome would be the only available option. It also seems like Qtforkawesome must be retained for use of the tray icon on non KDE Plasma systems.

I worry that mixing the two might cause scaling issues on HiDPI displays that use a non-default scaling factor. More large-scale use will reveal if users want sharper icons, and HiDPI users can be quite passionate about this sort of thing.

No comment on aesthetics, and I believe there's an argument to be made about how using the same icons as the Syncthing web interface makes the following semantic statement: it makes the user aware they're interacting with a web thing rather than a desktop thing. That can be a good or a bad thing, depending on one's perspective.

@Martchus
Copy link
Owner

Martchus commented Apr 5, 2022

Note that currently my approach is to use icons from the normal icon theme everywhere (dialogs, context menus, …) except for the main tray dialog where I use ForkAwesome icons. Especially for the view it just makes more sense to be consistent with Syncthing's normal GUI. Note that ForkAwesome is quite neutral and look good together with most other icon themes (and Plasma themes) and the icons will follow the system's color palette. HiDPI should not be a concern because the font icons also scale well.

However, I'm also open for implementing a config switch to use system icons as aggressively as possible. I just haven't implemented it myself because I personally don't see it as better.

@sten0
Copy link
Contributor

sten0 commented Apr 21, 2022 via email

@Martchus
Copy link
Owner

Does Qt ForkAwesome already override desktop preferences like these?

It renders the glyphs like any other text would be rendered within the Qt application. Normally Qt is using freetype2 to render fonts (at least under GNU/Linux) so settings affecting freetype2 should have effect. The settings done via systemsettings5 definitely have effect (although you need to restart the application, in case of the Plasmoid that's plasmashell itself).

And yes, the screenshot looks blurry (including the normal text rendering).

@stale
Copy link

stale bot commented Jun 20, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jun 20, 2022
@sten0
Copy link
Contributor

sten0 commented Jun 20, 2022 via email

@stale stale bot removed the stale label Jun 20, 2022
@stale
Copy link

stale bot commented Aug 30, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Aug 30, 2022
@sten0
Copy link
Contributor

sten0 commented Aug 31, 2022 via email

@stale stale bot removed the stale label Aug 31, 2022
@Martchus
Copy link
Owner

Adding the enhancement label so stale bot won't close it.


Note that the main problem is that the free desktop specification doesn't provide all icons we need so we'd still end up with a mix of KDE and ForkAwesome icons. Therefore I don't think that this is very worthwhile. At least one needs to ensure that the mixup won't be too bad (e.g. still use ForkAwesome consistently in the expandable details). Of course it is actually already a mixup but at least with a clear distinction where which icons are used (see #121 (comment)).

Martchus added a commit to Martchus/qtforkawesome that referenced this issue Sep 18, 2022
Martchus added a commit to Martchus/qtforkawesome that referenced this issue Oct 3, 2022
Martchus added a commit that referenced this issue Oct 3, 2022
* Allow using icons from freedesktop.org icon theme for most ForkAwesome
  icons if available
* Let's not use it for the nested list of details in the models for now
* See #121
Martchus added a commit that referenced this issue Oct 6, 2022
* Allow using icons from freedesktop.org icon theme for most ForkAwesome
  icons if available
* Let's not use it for the nested list of details in the models for now
* See #121
Martchus added a commit that referenced this issue Oct 6, 2022
* Allow using icons from freedesktop.org icon theme for most ForkAwesome
  icons if available
* Let's not use it for the nested list of details in the models for now
* See #121
@Martchus
Copy link
Owner

Martchus commented Oct 6, 2022

Implemented for the Qt Widgets version and the Plasmoid as optional feature. On the Plasmoid it looks rather ugly when the current freedesktop icon theme (e.g. Breeze light) doesn't match the Plasma theme (e.g. some dark theme). I suppose I actually needed to use the icons from the Plasma theme here (and not from the freedesktop icon theme).

@Martchus
Copy link
Owner

Martchus commented Aug 5, 2023

I suppose I actually needed to use the icons from the Plasma theme here (and not from the freedesktop icon theme).

It looks like this aspect is not going to be relevant in Plasma 6 anymore (see https://pointieststick.com/2023/07/26/what-we-plan-to-remove-in-plasma-6). Since it was the only improvement left to do I'm closing this issue.

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

No branches or pull requests

3 participants