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

Replace PNG with SVG icons #89

Closed

Conversation

NotMyFault
Copy link
Member

@NotMyFault NotMyFault commented Aug 23, 2021

This pull request aims to replace existing PNG images with modern SVG ionicons, like they are already used partly in core:

Current look light theme 1/2

Current look light theme 2/2

Current look dark theme 1/2

Current look dark theme 2/2


Proposed change light theme 1/2

Proposed change light theme 2/2

Proposed change dark theme 1/2

Proposed change dark theme 2/2

Also fixes a little typo when overriding icons.

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira: https://issues.jenkins.io/browse/JENKINS-66438
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

@NotMyFault
Copy link
Member Author

@batmat Do you mind taking a look over the PR? :)

@batmat
Copy link
Member

batmat commented Feb 9, 2022

Sorry for taking so long. The main reason is that I'm unsure I want this. This plugin is my first Jenkins plugin and kind of my baby you know :). I've done some of these icons myself (ok, they probably don't look awesome, but they were done with love 😁).

I see the point on consistency, for sure. But I need to look at these icons to see if I really agree with the replacement.

If you don't mind, I would appreciate a side by side comparison of before/after icons.

Sorry again for the lag.

@NotMyFault
Copy link
Member Author

Sorry for taking so long. The main reason is that I'm unsure I want this. This plugin is my first Jenkins plugin and kind of my baby you know :). I've done some of these icons myself (ok, they probably don't look awesome, but they were done with love 😁).

I can understand that for sure :P

Some time passed by and the PR description isn't quite up to date anymore.
To clarify, the replacement icons I've chosen are ionicons, which core will utilize very soon, instead of tango desktop or other icon sources. Yeah, that would be the point for consistency with the modern appearance of core.

However, when I created the PR initially, the ionicion stuff was still pretty new. Currently, APIs are work in progress that allow plugin developers to utilize the ionicons provided by core in the same way how you can utilize the current icons.
Additionally, coloring ionicons based on the color scheme the user requests.
I messed a bit around with it before :P

If you don't mind, I would appreciate a side by side comparison of before/after icons.

For sure, I'll see if I can attach one tomorrow.

@NotMyFault
Copy link
Member Author

Check the PR description, I added a bunch of screenshots for the current look and the proposed changed, in light and dark mode.

@oalagtash
Copy link

oalagtash commented Sep 24, 2022

I was about to open a bug report because some icons cannot be seen in dark mode. But this PR already fixed that, and the icons look great! They also go well with the icons design in Jenkins.
image
after highlighting, I can see the badge:
image

So yea, vote up! :)

@mawinter69
Copy link

mawinter69 commented May 14, 2023

Now with symbols in place we could use the ionicons plugin instead.
Plugins could then provide a symbol or an image and
Might need a way to distinguish if a plugin provides an image or a symbol. There are only 2 plugins currently providing own images both haven't been released for long time (https://www.jenkins.io/doc/developer/extensions/buildtriggerbadge/)

mawinter69 added a commit to mawinter69/buildtriggerbadge-plugin that referenced this pull request May 14, 2023
alternative to jenkinsci#89

refer to ioniocons-api plugin when a suitable symbol is available
Use <l:icon instead of <img
This works for both symbols and images provided by plugins

bumps core to 2.361.4 which requires java11
mawinter69 added a commit to mawinter69/buildtriggerbadge-plugin that referenced this pull request May 14, 2023
alternative to jenkinsci#89

refer to ioniocons-api plugin when a suitable symbol is available
Use <l:icon instead of <img
This works for both symbols and images provided by plugins

bumps core to 2.361.4 which requires java11
@NotMyFault
Copy link
Member Author

Closing in favor of #159

@NotMyFault NotMyFault closed this Jul 20, 2023
@NotMyFault NotMyFault deleted the feature/master/svg-icons branch July 20, 2023 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants