Modernize build status and weather icons #5065

timja · 2020-11-20T16:50:22Z

Try me out with:

docker run --rm -ti -p 8080:8080 -e ID=5065 -e MERGE_WITH=master jenkins/core-pr-tester

Proposed changelog entries

Entry 1: Add modern icons: build status, weather
Entry 2: Developer: Add support for plugins to use external SVG sprites in their icons, see github-branch-source-plugin/pull/393 for an example
...

Proposed upgrade guidelines

Green Balls Plugin is no longer compatible with the new icons. If you use the plugin on your instances, it can be disabled or removed.

Submitter checklist

(If applicable) Jira issue is well described
Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change). Examples
- Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
Appropriate autotests or explanation to why this change has no tests
For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

There are at least 2 approvals for the pull request and no outstanding requests for change
Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
Changelog entries in the PR title and/or Proposed changelog entries are correct
Proper changelog labels are set so that the changelog can be generated automatically
If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

core/src/main/java/org/jenkins/ui/icon/Icon.java

core/src/main/resources/jenkins/model/Jenkins/manage.jelly

core/src/main/resources/lib/layout/icon.jelly

fqueiruga · 2020-11-20T17:48:22Z

Looks great! I've got some quick feedback before being able to take a deeper look into it:

I think it would be best to separate the inlining of the icon-set library from the actual changes, in different PRs. That way it would be cleaner to see the modifications. The first one could also go through quicker.
I have some ideas in mind regarding how to support the action icons in a backwards-compatible way, just modifying IconSet to avoid needing to modify calls to l:icon that pass icon classes. I also think the way to go is make use of svg sprites and the l:svgIcon tag within the l:icon template. I could try to flesh this out more but I think it would be easier on a different branch / PR than the library inlining.

Amazing PR anyway 🎉

fqueiruga · 2020-11-23T12:05:08Z

I'm running the PR locally. I have noticed that the credentials icon under the /manage page are missing. I think it has to do with them being set on the credentials-plugin using IconSet. the l:icon helper is setting the proper IRC. Can this be a possibility?

timja · 2020-11-23T13:20:01Z

I'm running the PR locally. I have noticed that the credentials icon under the /manage page are missing. I think it has to do with them being set on the credentials-plugin using IconSet. the l:icon helper is setting the proper IRC. Can this be a possibility?

using this approach the plugin would need to be updated to provide an SVG and use that instead of the image it's currently using, (it could check the jenkins version to decide which one to use)

fqueiruga · 2020-11-23T13:25:18Z

Is there no way we can make it backwards compatible and not require changes plugin-side? There are quite a few Plugins that do use IconSet

timja · 2020-11-23T13:43:10Z

Is there no way we can make it backwards compatible and not require changes plugin-side? There are quite a few Plugins that do use IconSet

yeah I haven't really worked on the /manage side yet, that was just a quick 5 minute job.
I mostly worked on the icons in the sidebar

It can definitely be fixed just a POC atm.

I expect changes on plugin side will still be needed to start using svgs, but it'll be backwards compatible, as in they can keep using pngs / gifs if they want

fqueiruga · 2020-11-23T13:44:18Z

Got it 🎉

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

…nline-icon-set-lib

timja · 2020-11-28T22:26:30Z

the diff of just this PR can be seen at: timja#29

daniel-beck

The current state is a reasonable start that doesn't look unfinished more than it needs to.

Everything that's currently slightly off (see previous comment) should be straightforward enough to amend without requiring any expensive drawing of new icons, so 👍 from me.

oleg-nenashev

Looks good in the PR tester. Note that the status icons are also used in Job type logos. Would be nice to eventually update them.

Compatibility notes

The change makes the Green Balls plugin defunct because of the filter implementation in https://github.com/jenkinsci/greenballs-plugin/blob/master/src/main/java/hudson/plugins/greenballs/GreenBallFilter.java . It is not a blocker in any means, but it worth documenting the impact in the upgrade guidelines (that the plugin can be disabled, unless it is adapted to the new format). CC @nfalco79

timja · 2021-03-08T10:27:13Z

Green balls is no longer needed after this change really, this implements what the plugin does in core at least for the 'balls'

nfalco79 · 2021-03-08T10:27:46Z

I see from screenshot the plugin provides green checks for passed build so the "Green Balls" plugin became useless

oleg-nenashev · 2021-03-08T10:51:54Z

Yeah. Greenballs equivalent could become a part of the "Classic Jenkins Theme" if someone creates it

…

On Mon, Mar 8, 2021 at 11:28 AM Nikolas Falco ***@***.***> wrote: I see from screenshot the plugin provides green checks for passed build so the "Green Balls" plugin became useless — You are receiving this because your review was requested. Reply to this email directly, view it on GitHub <#5065 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAW4RIFGT2I46BDYEMOOS23TCSRDHANCNFSM4T5AJ5JQ> .

timja · 2021-03-11T14:37:37Z

core/src/main/java/org/jenkins/ui/icon/IconSet.java

 icons.addIcon(new Icon("icon-secure icon-lg", "32x32/secure.png", Icon.ICON_LARGE_STYLE));
 icons.addIcon(new Icon("icon-setting icon-lg", "32x32/setting.png", Icon.ICON_LARGE_STYLE));
 icons.addIcon(new Icon("icon-star-gold icon-lg", "32x32/star-gold.png", Icon.ICON_LARGE_STYLE));
 icons.addIcon(new Icon("icon-star icon-lg", "32x32/star.png", Icon.ICON_LARGE_STYLE));
- icons.addIcon(new Icon("icon-user icon-lg", "32x32/user.png", Icon.ICON_LARGE_STYLE));
- icons.addIcon(new Icon("icon-yellow icon-lg", "32x32/yellow.png", Icon.ICON_LARGE_STYLE));
+ icons.addIcon(new Icon("icon-user icon-lg", "32x32/user.gif", Icon.ICON_LARGE_STYLE));


timja · 2021-03-11T14:38:19Z

core/src/main/java/org/jenkins/ui/icon/IconSet.java

 icons.addIcon(new Icon("icon-installer icon-xlg", "48x48/installer.png", Icon.ICON_XLARGE_STYLE));
 icons.addIcon(new Icon("icon-lock icon-xlg", "48x48/lock.png", Icon.ICON_XLARGE_STYLE));
 icons.addIcon(new Icon("icon-monitor icon-xlg", "48x48/monitor.png", Icon.ICON_XLARGE_STYLE));
 icons.addIcon(new Icon("icon-network icon-xlg", "48x48/network.png", Icon.ICON_XLARGE_STYLE));
- icons.addIcon(new Icon("icon-nobuilt icon-xlg", "48x48/nobuilt.png", Icon.ICON_XLARGE_STYLE));
- icons.addIcon(new Icon("icon-notepad icon-xlg", "48x48/notepad.png", Icon.ICON_XLARGE_STYLE));
+ icons.addIcon(new Icon("icon-notepad icon-xlg", "48x48/notepad.gif", Icon.ICON_XLARGE_STYLE));


uhafner · 2021-03-11T16:49:48Z

I have just seen that we forgot to change BallColor as well.

timja · 2021-03-11T16:51:25Z

I have just seen that we forgot to change BallColor as well.

What would get changed there? I think I remember you mentioning in a ux-sig meeting at some point that it was used to build a palette somewhere.

uhafner · 2021-03-11T16:54:00Z

It provides the build status images as Java API. Example output of Jenkins 2.283:

They should map to the new images as well.

jglick · 2021-03-11T17:24:32Z

At least

jenkins/core/src/main/java/hudson/model/BallColor.java

Line 115 in b252819

 return Stapler.getCurrentRequest().getContextPath()+ Jenkins.RESOURCE_PATH+"/images/"+size+'/'+image; 

timja · 2021-03-11T20:27:41Z

The new ones aren't images though they are styled svgs, possibly the old APIs need deprecating and new ones created, should be tracked in Jira as a follow up enhancement I think

chinhodado · 2021-03-12T14:43:22Z

I just upgraded Jenkins to 2.283 and I can't say I'm a fan of this change.

Before, I can easily tell the icons from each other at a glance since they have different colors:

Now it's just the same shade of blue:

Can we make these icons have different colors please? Or add an option to have the old icons back. There are serious usability issue with the new icons when they're all the same color.

apottere · 2021-03-15T15:00:29Z

Agree with @chinhodado, I love the new build status icons but the weather ones need some color variation, even just a little.

Edit: Maybe just make the clouds gray and increasingly darker gray as they get worse? They are storm clouds after all 🌧️

daniel-beck · 2021-03-15T15:39:18Z

Please provide feedback for changes through the issue tracker.

apottere · 2021-03-15T16:39:03Z

@daniel-beck the ticket linked in the PR is https://issues.jenkins.io/browse/JENKINS-XXXXX, which doesn't seem to exist (understandably). Did you mean I should instead open a new issue for this feedback?

daniel-beck · 2021-03-15T17:07:35Z

@apottere Yes, please, if there isn't one already.

apottere · 2021-03-15T19:39:08Z

Created: https://issues.jenkins.io/browse/JENKINS-65124

Hedede · 2021-03-23T09:47:36Z

Please let me have the old icons back.

timja commented Nov 20, 2020

View reviewed changes

core/src/main/java/org/jenkins/ui/icon/Icon.java Outdated Show resolved Hide resolved

core/src/main/resources/jenkins/model/Jenkins/manage.jelly Outdated Show resolved Hide resolved

core/src/main/resources/lib/layout/icon.jelly Show resolved Hide resolved

timja changed the title ~~Add support for svg icons in actions~~ Add support for svg icons in l:icon and update management links Nov 20, 2020

Inline icon-set lib

4c27438

timja mentioned this pull request Nov 23, 2020

Inline icon-set lib #5072

Merged

10 tasks

timja and others added 7 commits November 23, 2020 14:26

Add @SInCE 2.0

d556c74

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

Merge branch 'master' into inline-icon-set-lib

472804c

Merge branch 'master' into inline-icon-set-lib

8ea006f

Merge branch 'inline-icon-set-lib' of github.com:timja/jenkins into i…

d4d40a1

…nline-icon-set-lib

Downgrade iconset to 1.0.5 version

7206870

Fix javadoc

913c52b

Missed one javadoc error

b07e165

timja force-pushed the svg-icons branch from bd68134 to a2f3dd3 Compare November 28, 2020 20:36

timja changed the title ~~Add support for svg icons in l:icon and update management links~~ Add support for svg icons in l:icon Nov 28, 2020

timja force-pushed the svg-icons branch from a2f3dd3 to dd1b73a Compare November 28, 2020 22:04

Add support for svg icons in actions

43b1e7c

timja force-pushed the svg-icons branch from dd1b73a to 43b1e7c Compare November 28, 2020 22:13

Change help icon

e128508

timja and others added 6 commits November 28, 2020 22:59

Fix large SVG icons

63763e2

Experiment with font awesome

61c3dec

Add /manage support

6937e27

Merge branch 'master' into svg-icons

b1a76e3

Use material sprites (#30)

dd8ea43

Merge branch 'master' into pr/5065

974fd7d

daniel-beck approved these changes Mar 8, 2021

View reviewed changes

Revert missed icon clipboard

1a8cedc

timja added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Mar 8, 2021

oleg-nenashev approved these changes Mar 8, 2021

View reviewed changes

oleg-nenashev added the upgrade-guide-needed This changes might be breaking in rare circumstances, an entry in the LTS upgrade guide is needed label Mar 8, 2021

timja merged commit 037e137 into jenkinsci:master Mar 9, 2021

timja deleted the svg-icons branch March 9, 2021 08:09

timja changed the title ~~Initial new icons~~ Modernize build status and weather icons Mar 9, 2021

dependabot bot mentioned this pull request Mar 11, 2021

Bump various/jenkins from ee3b6d5 to b252819 batmat/dependabot-testing#32

Closed

timja commented Mar 11, 2021

View reviewed changes

kjyv mentioned this pull request Mar 11, 2021

Broken animated icon for running jobs TobiX/jenkins-neo2-theme#40

Closed

res0nance mentioned this pull request Mar 11, 2021

Icon typo fixes (used non transparent gif by accident) #5354

Merged

10 tasks

daniel-beck mentioned this pull request Mar 17, 2021

Update at-since annotations up to 2.284 #5364

Merged

10 tasks

jenkinsci locked as off-topic and limited conversation to collaborators Mar 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modernize build status and weather icons #5065

Modernize build status and weather icons #5065

timja commented Nov 20, 2020 •

edited

Loading

fqueiruga commented Nov 20, 2020

fqueiruga commented Nov 23, 2020

timja commented Nov 23, 2020

fqueiruga commented Nov 23, 2020 •

edited

Loading

timja commented Nov 23, 2020 •

edited

Loading

fqueiruga commented Nov 23, 2020

timja commented Nov 28, 2020

daniel-beck left a comment

oleg-nenashev left a comment

timja commented Mar 8, 2021

nfalco79 commented Mar 8, 2021

oleg-nenashev commented Mar 8, 2021 via email

timja Mar 11, 2021

timja Mar 11, 2021

uhafner commented Mar 11, 2021

timja commented Mar 11, 2021

uhafner commented Mar 11, 2021

jglick commented Mar 11, 2021

timja commented Mar 11, 2021

chinhodado commented Mar 12, 2021 •

edited

Loading

apottere commented Mar 15, 2021 •

edited

Loading

daniel-beck commented Mar 15, 2021

apottere commented Mar 15, 2021

daniel-beck commented Mar 15, 2021 •

edited

Loading

apottere commented Mar 15, 2021

Hedede commented Mar 23, 2021

Modernize build status and weather icons #5065

Modernize build status and weather icons #5065

Conversation

timja commented Nov 20, 2020 • edited Loading

Try me out with:

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

fqueiruga commented Nov 20, 2020

fqueiruga commented Nov 23, 2020

timja commented Nov 23, 2020

fqueiruga commented Nov 23, 2020 • edited Loading

timja commented Nov 23, 2020 • edited Loading

fqueiruga commented Nov 23, 2020

timja commented Nov 28, 2020

daniel-beck left a comment

Choose a reason for hiding this comment

oleg-nenashev left a comment

Choose a reason for hiding this comment

Compatibility notes

timja commented Mar 8, 2021

nfalco79 commented Mar 8, 2021

oleg-nenashev commented Mar 8, 2021 via email

timja Mar 11, 2021

Choose a reason for hiding this comment

timja Mar 11, 2021

Choose a reason for hiding this comment

uhafner commented Mar 11, 2021

timja commented Mar 11, 2021

uhafner commented Mar 11, 2021

jglick commented Mar 11, 2021

timja commented Mar 11, 2021

chinhodado commented Mar 12, 2021 • edited Loading

apottere commented Mar 15, 2021 • edited Loading

daniel-beck commented Mar 15, 2021

apottere commented Mar 15, 2021

daniel-beck commented Mar 15, 2021 • edited Loading

apottere commented Mar 15, 2021

Hedede commented Mar 23, 2021

timja commented Nov 20, 2020 •

edited

Loading

fqueiruga commented Nov 23, 2020 •

edited

Loading

timja commented Nov 23, 2020 •

edited

Loading

chinhodado commented Mar 12, 2021 •

edited

Loading

apottere commented Mar 15, 2021 •

edited

Loading

daniel-beck commented Mar 15, 2021 •

edited

Loading