Navigation Menu

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

[JENKINS-66708] Prepare removal of sunset icons from core #146

Merged
merged 1 commit into from Jan 19, 2022

Conversation

NotMyFault
Copy link
Member

@NotMyFault NotMyFault commented Sep 23, 2021

Preparation for core sunsetting dated icons: jenkinsci/jenkins#5778

  • 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

@NotMyFault NotMyFault changed the title [JENKINS-66708] Exchange GIFs with SVGs [JENKINS-66708] Exchange PNGs with SVGs Sep 23, 2021
pom.xml Outdated Show resolved Hide resolved
@NotMyFault NotMyFault force-pushed the feature/master/exchange-icons branch 2 times, most recently from 3885bf9 to 52ce416 Compare October 25, 2021 10:25
@alecharp alecharp closed this Nov 16, 2021
@alecharp
Copy link
Member

Incorrectly closed. Sorry for that. I'll merge the PR soon

@alecharp alecharp reopened this Nov 16, 2021
Copy link
Member

@alecharp alecharp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see the attached screenshot I just took. The icons doesn't quite show as in your screenshot.

I'm on Firefox 94.0, Fedora 34.

Screenshot from 2021-11-16 15-39-13
Screenshot from 2021-11-16 15-38-47

@NotMyFault
Copy link
Member Author

Please see the attached screenshot I just took. The icons doesn't quite show as in your screenshot.

I'm on Firefox 94.0, Fedora 34.

Screenshot from 2021-11-16 15-39-13 Screenshot from 2021-11-16 15-38-47

That indeed does look out of place, would you be opposed against a material icon that utilizes Jenkins' current color palette?
For example something like:

@ianwilliams1
Copy link

Screenshot from 2021-11-16 15-38-47

That indeed does look out of place, would you be opposed against a material icon that utilizes Jenkins' current color palette? For example something like:

I'm no expert on (or fan of) material theme or graphic design and I do acknowledge changing the icon can be disruptive, so I'd rather see a straight .png - .svg conversion; so a scripted lettering.

Nevertheless, if changing to a material design, the icon should reflect the utility, not just another "page icon".

Given this purpose of the plugin is 1) Configuration 2) File 3) Management, I'd suggest something with the representative icons for each. I cut and pasted a couple material icons and came up with this:
image

It immediately tells me it's a controlled settings file., which is pretty close to "config-file-provide". Thoughts ?

Maybe an SVG expert like @NotMyFault Alex can make compliant / and add the appropriate flair consistent w/other icons (ie: combine the Configure System gear and the Security lock)

viewBox="0 0 24 24"
<path d="
 M15,22H6c-1.1,0-2-0.9-2-2V4c0-1.1,0.9-2,2-2h8l6,6v6h-2V9h-5V4H6v16h9V22z
"/>
<path fill="#999999" d="
 M15.78 12.75l-1.07-.83c.02-.16.04-.32.04-.49 0-.17-.01-.33-.04-.49l1.06-.83c.09-.08.12-.21.06-.32l-1-1.73c-.06-.11-.19-.15-.31-.11l-1.24.5c-.26-.2-.54-.37-.85-.49l-.19-1.32c-.01-.12-.12-.21-.24-.21h-2c-.12 0-.23.09-.25.21l-.19 1.32c-.3.13-.59.29-.85.49l-1.24-.5c-.11-.04-.24 0-.31.11l-1 1.73c-.06.11-.04.24.06.32l1.06.83c-.02.16-.03.32-.03.49 0 .17.01.33.03.49l-1.06.83c-.09.08-.12.21-.06.32l1 1.73c.06.11.19.15.31.11l1.24-.5c.26.2.54.37.85.49l.19 1.32c.02.12.12.21.25.21h2c.12 0 .23-.09.25-.21l.19-1.32c.3-.13.59-.29.84-.49l1.25.5c.11.04.24 0 .31-.11l1-1.73c.06-.11.03-.24-.06-.32z
 m-4.78.18c-.83 0-1.5-.67-1.5-1.5s.67-1.5 1.5-1.5 1.5.67 1.5 1.5-.67 1.5-1.5 1.5z
"/>
<path fill="#FF0000" d="
 M20,17v-1c0-1.1-0.9-2-2-2 s-2,0.9-2,2v1c-0.55,0-1,0.45-1,1v3c0,0.55,0.45,1,1,1h4c0.55,0,1-0.45,1-1v-3C21,17.45,20.55,17,20,17z
 M19,17h-2v-1 c0-0.55,0.45-1,1-1s1,0.45,1,1V17z
"/>

@NotMyFault
Copy link
Member Author

I'm no expert on (or fan of) material theme or graphic design and I do acknowledge changing the icon can be disruptive, so I'd rather see a straight .png - .svg conversion; so a scripted lettering.
Nevertheless, if changing to a material design, the icon should reflect the utility, not just another "page icon".

Core has expressed some interest in replacing tango icons with more modern ones from sources like Iconic, font awesome or material while maintaining a color palette striving through its components, e.g. the font color from my screenshot above or the refurbished table design in this week's release. That's why I didn't pull in an icon of these "legacy" sources.
My latter approach would remedy that and propose a more future proof change that utilizes named palette above.

@ianwilliams1
Copy link

Hi Alex,

What I am saying is the proposed change should simply be from PNG to SVG ( identical image, same marker pen font ) as you've done so well for other plugins.

I think what @alecharp is saying is there are some issues with the legacy SVG, specifically under Linux that would need to be addressed, preferably as part of this PR..


But if proposing to move a skeuomorphic / "material theme" design, the plugin purpose should be somewhat representative in the iconography, something that often seems to get lost in translation, especially with computing artifacts. A "paper" icon is not suggestive of anything and potentially confusing.

That's where my proposal [ gear + lock + paper ] aggregate icon comes in. It would be more representative than the existing "C - F - G " marker pen icon, if the Plugin owner/maintainer (@olamy / @alecharp ) are amenable to change. This would be a separate PR.

image

@NotMyFault NotMyFault changed the title [JENKINS-66708] Exchange PNGs with SVGs [JENKINS-66708] Exchange PNGs with SVGs and allow file override Nov 17, 2021
@NotMyFault NotMyFault changed the title [JENKINS-66708] Exchange PNGs with SVGs and allow file override [JENKINS-66708] Exchange PNGs with SVGs where applicable Jan 2, 2022
@NotMyFault
Copy link
Member Author

@jenkinsci/config-file-provider-plugin-developers any chance we could get this merged and released please? I reverted the initial concerns from alecharp for the time being so that this PR now does address the sunset icons for removal only.

@NotMyFault NotMyFault changed the title [JENKINS-66708] Exchange PNGs with SVGs where applicable [JENKINS-66708] Prepare removal of sunset icons from core Jan 6, 2022
@timja
Copy link
Member

timja commented Jan 7, 2022

ping @jenkinsci/config-file-provider-plugin-developers

@jmMeessen
Copy link
Contributor

Hello @alecharp, can we have a look together and discuss it ?

@NotMyFault
Copy link
Member Author

Hello @alecharp, can we have a look together and discuss it ?

That would be much appreciated <3

@alecharp
Copy link
Member

@NotMyFault Reviewed with @jmMeessen and it's good. I'm merging this. Thank you for the contribution.

Note: please try not to force push on PR when someone already review its content. It removes all comments and all, so we lose the "this has been fixed" from previous review.

@alecharp alecharp merged commit dadb5a4 into jenkinsci:master Jan 19, 2022
@NotMyFault NotMyFault deleted the feature/master/exchange-icons branch January 19, 2022 09:40
@NotMyFault
Copy link
Member Author

@NotMyFault Reviewed with @jmMeessen and it's good. I'm merging this. Thank you for the contribution.

Note: please try not to force push on PR when someone already review its content. It removes all comments and all, so we lose the "this has been fixed" from previous review.

Oops, didn't think about that, sorry! Hope that didn't cause too much inconveniences.
But thanks for merging, do you mind releasing it as well please?

@timja
Copy link
Member

timja commented Jan 19, 2022

or enabling CD 😄

@jmMeessen
Copy link
Contributor

💯
Enabling CD is on the top of our improvement list as soon as we have these PR rolling.

@Riliane
Copy link

Riliane commented Jul 5, 2022

We are going to finalize this icon proposed above into an SVG image.

@ianwilliams1
Copy link

@Riliane, do you intend to proceed with a revised "C-F-G" (SVG) icon or per the skeuomorphic proposal ( my preference 😄 ) ? should the skeuomorphic design align with the new fontawesome/ionicon representations?
Is there are link to the separated issue/PR for the icon fix as this PR appears already merged?

@Riliane
Copy link

Riliane commented Jul 15, 2022

A Material based icon I consider the better choice and actually we have now considered existing Material + community-made icons, and are considering something like this - https://materialdesignicons.com/icon/file-cog-outline
It already has the file and the cog, neat looking and conveys both the meaning that a) the plugin manages files an b) the files contain configuration.
Or the copy file Material icon https://fonts.gstatic.com/s/i/short-term/release/materialsymbolsoutlined/file_copy/default/48px.svg as it copies over the configurations.
This will be a separate PR - will be linked later. The Jira issue is however still open - should this also go under this issue, then?

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