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

Add support for "Icon Packs" #13

Open
AObuchow opened this issue Apr 19, 2022 · 11 comments · May be fixed by #14
Open

Add support for "Icon Packs" #13

AObuchow opened this issue Apr 19, 2022 · 11 comments · May be fixed by #14
Labels
enhancement New feature or request

Comments

@AObuchow
Copy link
Contributor

AObuchow commented Apr 19, 2022

This is a continuation/migration of Bug 562174 - Add support for "icon packs" in Eclipse IDE.

It'd be nice for to have an officially supported way to replace the icons used in Eclipse.

There is a lot of discussion to be held on how this should be handled (and there was already a lot of discussion on the original BZ thread), so I'm going to leave this relatively open-ended.

IIRC the approach I had initially tried was:

  • Allow custom/replacement icons to be provided by a plugin fragment, by placing them in a "theme" directory of the plugin fragment
    • So in order to replace one of the "debug" icons, you must make a plugin fragment where the host plugin is "eclipse.debug.ui" and place the icon within a "theme" directory, with the same file path as the original icon
  • When loading an icon from a plugin, check if the plugin provides an alternative path to the icon that is prefixed by "/themes/"
  • If so, load it
  • Otherwise, use the default icon

There are probably some limitations to this approach, for instance:

  • The icon-provider must create a plugin fragment for each plugin which they wish to replace the icons for. This is very tedious.
  • How do we handle switching between different icon packs, if each plugin fragment uses the same "themes" directory?

It might be nicer to set some JFace preference which points to a specific plugin path (where the plugin is the icon pack provider plugin), and then alternative icons are checked from that plugin before loading the default icon.

I am going to submit a WIP PR that shows my current implementation, as well as an example plugin fragment that provides icon replacements.

@AObuchow AObuchow added the enhancement New feature or request label Apr 19, 2022
@laeubi
Copy link
Contributor

laeubi commented Apr 19, 2022

I think a preference would be a good idea in any case to prevent excessive lookups, even if they are fast they are not for free.

t might be nicer to set some JFace preference which points to a specific plugin path

There is already org.eclipse.e4.ui.css.swt.theme/themeid I think it would be good to reuse that, lets assume the themeid is 'my.custom.theme' jface could simply prefix all icons with that id eg icon/cross.png -> icon/my.custom.theme/cross.png with a fallback to the original icon as the "error icon".

@mickaelistria
Copy link
Contributor

There is already org.eclipse.e4.ui.css.swt.theme/themeid

While it's good to reuse it if possible, I think it's important that this particular issue of "icon packs" can be built-in JFace without dependency of e4 or higher-level platform.
Then the integration of icon packs in e4 themes can decide to rely on this themeid, but that discussion is IMO a step further.

@laeubi
Copy link
Contributor

laeubi commented Apr 19, 2022

JFace without dependency of e4 or higher-level platform.

JFace don't need to depend on e4, just reading that preferences with that ID, I actually won't care much about that in contrast to inventing another "icon pack id" that then must match the themeId or similar ...

Actually it makes most sense to have a icon pack for a theme than having different icons with the standard theme.

@mickaelistria
Copy link
Contributor

Actually it makes most sense to have a icon pack for a theme than having different icons with the standard theme.

I disagree here. Also SWT/JFace do not apply only to Eclipse IDE nor workbench, there are other apps that may want icon/image theme without dragging the e4 stuff.
Let's fix the issue where it is (in JFace) without considering the legacy downstream (e4), that will lead to the best design.

@laeubi
Copy link
Contributor

laeubi commented Apr 19, 2022

You don't need to drag in anything. This is just a preference and we can access it in any bundle we like without any dependency. As SWT does not allow for any theming, it might then even be better to not do this in swt/jface but in the eclipse theming framework to handle that case.

@BeckerWdf
Copy link
Contributor

  • and place the icon within a "theme" directory, with the same file path as the original icon

So this implies only icons that originally come from a file can be replaced? What out other icons (that are e.g. drawn via GC operations) like these:
image

@laeubi
Copy link
Contributor

laeubi commented Apr 21, 2022

What out other icons (that are e.g. drawn via GC operations) like these:

These are obviously not Image-Icons :-)

@BeckerWdf
Copy link
Contributor

Will this new feature allow plugin developers to use different icons for the light and the dark theme? Currently it's quite hard to design icons that have good contrast on light and dark background. So providing alternative icons for the dark theme is a much needed feature.

These are obviously not Image-Icons :-)

So if the answer to the above question is yes: We might also want to provide different versions of these "none-image-icons". What would you propose as as solution for this? Having IFs in the drawing code that check for theme-names or brightness of the background color?

@laeubi
Copy link
Contributor

laeubi commented Apr 22, 2022

Will this new feature allow plugin developers to use different icons for the light and the dark theme?

Not in its current form, thats why I requested cahnges and suggested to better integrate this with the theming framework instead of an additional soloution just for SWT.

So if the answer to the above question is yes: We might also want to provide different versions of these "none-image-icons".

none-image-icons are custom painted components as as such are best supporting CSS styling instead of loading alternative images. see:
https://wiki.eclipse.org/E4/CSS/SWT_Mapping
https://wiki.eclipse.org/Eclipse4/RCP/CSS#Extending_CSS_to_handle_new_widgets

@BeckerWdf
Copy link
Contributor

Not in its current form, thats why I requested changes and suggested to better integrate this with the theming framework instead of an additional solution just for SWT.

👍

@mickaelistria
Copy link
Contributor

Will this new feature allow plugin developers to use different icons for the light and the dark theme?

Ultimately yes, but it will also require change in the theme support once we're done with SWT part. The theme would be able to define an icon set to use and configure SWT; so SWT would then load different images.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants