[JENKINS-62496] Convert arrow pngs to CSS #4757

EstherAF · 2020-05-29T23:31:33Z

See JENKINS-62496.

Fixes jenkinsci/dark-theme-plugin#24

Convert arrows used to display contextual menus, from background images to CSS triangles.

Screenshots of the change:

Down arrow

Before:

Breadcrumbs:
Build Execution:
Build Queue:
Jobs:

After:

Breadcrumbs:
Build Execution:
Build Queue:
Jobs:

Right arrow

Before

Breadcrumbs
Submenu

After

Breadcrumbs
Submenu

Proposed changelog entries

Entry 1: JENKINS-62496 Arrow icons changed from images to CSS

Proposed upgrade guidelines

N/A

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

@timja @fqueiruga

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).

oleg-nenashev

Looks good overall, thanks! I think we need to wait until #4752 is merged and then adds CSS variables for it

oleg-nenashev · 2020-05-30T00:47:05Z

core/src/main/resources/lib/layout/breadcrumbs.css

-#breadcrumbs LI.children:hover {
-    background-image: url(menu_right_arrow.png);
+#breadcrumbs LI.children:hover:after {
+    border-left-color: #4d545d;


Will need update after #4752 is merged so that we do not hardcode colors

And I think the variable should be --text-color.

oleg-nenashev · 2020-05-30T00:47:27Z

core/src/main/resources/lib/layout/breadcrumbs.css

+#menuSelector:after {
+    /* Down arrow */
+    content: "";
+    border-top: 0.30em solid #4d545d;


Needs a variable after #4752

oleg-nenashev · 2020-05-30T00:47:38Z

core/src/main/resources/lib/layout/breadcrumbs.css

+    border-bottom: 0;
+}
+#menuSelector.inverse:after {
+    border-top-color: #bcbcbc;


Needs a variable after #4752

Same as above

timja

I've played around with this with variables and I got it working, I'll send a PR here once the variables PR is merged,

All icons that I found except for one are working (the Manage Jenkins sub menu):

This appears to be the CSS

jenkins/war/src/main/webapp/scripts/yui/menu/assets/menu.css

Line 409 in 0e2fd16

background: url(menubaritem_submenuindicator.png) right center no-repeat;

Although I would suggest overriding it in the yui-compatibility less file: https://github.com/jenkinsci/jenkins/blob/49e501f37db848e69de31607a43df79bab4e77b0/war/src/main/less/base/yui-compatibility.less

timja · 2020-05-31T07:08:44Z

@EstherAF PR for the dark mode changes on the breadcrumbs: EstherAF#9

EstherAF · 2020-06-01T10:23:31Z

All icons that I found except for one are working (the Manage Jenkins sub menu):

I'm looking at it. Do you know where the class yuimenubaritem-hassubmenu is being used? I've only found usages of the yuimenuitem-hassubmenu.

timja · 2020-06-01T10:28:26Z

hassubmenu

can't see it, took me quite awhile to even find the css and selector that was causing the above issue

varyvol · 2020-06-04T09:49:32Z

@EstherAF it seems like the new arrows have a lighter dark than they previously had? I'm not against it, I just want to check it's something we've noticed and are fine with it.

Other than that, it looks good!

EstherAF · 2020-06-04T21:50:34Z

@EstherAF it seems like the new arrows have a lighter dark than they previously had? I'm not against it, I just want to check it's something we've noticed and are fine with it.

Yes! It is because now they are using the same color than the text, with the css variable. Before they were using just black (IIRC).

What do you think? I prefer this so the color palette used is simpler, and it is more integrated with the text of the link.

varyvol · 2020-06-05T07:50:10Z

@EstherAF it makes sense to me and I agree. As I said, I just wanted to note it just in case.

EstherAF · 2020-06-07T21:59:47Z

All icons that I found except for one are working (the Manage Jenkins sub menu)

@timja I've just pushed a fix for that.

I've faced a little problem here that I don't know how to solve it. The arrow with CSS is generated as another element inside the menu item, as a sibling of the menu link (the text). This is causing that the menu link is not 100% wide anymore, and when focused the borders are displayed and you can see that the size is different.
Here it is more clear, look at the red border:

Before

After. There is a space at the right of the red border.

It only happens when the menu item has the arrow at the right (when it has a submenu). In the other items, the border on focus remains like before.

I don't know if this is a big deal, WDYT?

timja

I've tried it out and wasn't able to come up with anything better,

thanks!

war/src/main/less/base/yui-compatibility.less

fqueiruga

I've suggested some changes that should fix the issue with the dropdown menu. Please test them locally befor committing :D

war/src/main/less/base/yui-compatibility.less

Co-authored-by: Félix Queiruga <felix.queiruga@gmail.com>

EstherAF · 2020-06-10T19:31:39Z

I've suggested some changes that should fix the issue with the dropdown menu. Please test them locally befor committing :D

Thanks @fqueiruga. It works perfectly!

timja

Tested, works great!

timja · 2020-06-11T07:13:33Z

Thanks for the PR!

This PR is now ready-for-merge, we will merge it after ~24 hours if there's no negative feedback

oleg-nenashev

Looks good, all comments are addressed. Thanks @EstherAF !

[JENKINS-62496] Convert arrow pngs to CSS

25fb932

oleg-nenashev reviewed May 30, 2020

View reviewed changes

timja reviewed May 30, 2020

View reviewed changes

Merge branch 'master' into JENKINS-62496

198a188

Breadcrumbs dark mode

3c783e1

EstherAF mentioned this pull request Jun 1, 2020

Convert arrow pngs to CSS jenkinsci/ui-ux-hackfest-2020#232

Closed

timja mentioned this pull request Jun 4, 2020

Different styles for highlighting followed jenkinsci/dark-theme-plugin#19

Closed

fqueiruga approved these changes Jun 5, 2020

View reviewed changes

timja added the web-ui The PR includes WebUI changes which may need special expertise label Jun 7, 2020

Convert arrow of yui submenus to CSS

212b381

EstherAF force-pushed the JENKINS-62496 branch from 257a5be to 212b381 Compare June 7, 2020 21:30

timja approved these changes Jun 8, 2020

View reviewed changes

timja requested a review from fqueiruga June 8, 2020 07:14

timja added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Jun 8, 2020

fqueiruga reviewed Jun 8, 2020

View reviewed changes

war/src/main/less/base/yui-compatibility.less Outdated Show resolved Hide resolved

fqueiruga reviewed Jun 8, 2020

View reviewed changes

war/src/main/less/base/yui-compatibility.less Outdated Show resolved Hide resolved

fqueiruga suggested changes Jun 8, 2020

View reviewed changes

war/src/main/less/base/yui-compatibility.less Outdated Show resolved Hide resolved

war/src/main/less/base/yui-compatibility.less Outdated Show resolved Hide resolved

Position arrow without moving elements around

27cc3f3

Co-authored-by: Félix Queiruga <felix.queiruga@gmail.com>

timja approved these changes Jun 10, 2020

View reviewed changes

timja requested review from fqueiruga and oleg-nenashev June 10, 2020 20:13

fqueiruga approved these changes Jun 10, 2020

View reviewed changes

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 Jun 11, 2020

oleg-nenashev approved these changes Jun 12, 2020

View reviewed changes

timja merged commit 2466805 into jenkinsci:master Jun 12, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-62496] Convert arrow pngs to CSS #4757

[JENKINS-62496] Convert arrow pngs to CSS #4757

EstherAF commented May 29, 2020 •

edited

Loading

oleg-nenashev left a comment •

edited

Loading

oleg-nenashev May 30, 2020

fqueiruga May 30, 2020

oleg-nenashev May 30, 2020

fqueiruga May 30, 2020

oleg-nenashev May 30, 2020

fqueiruga May 30, 2020

timja left a comment •

edited

Loading

timja commented May 31, 2020

EstherAF commented Jun 1, 2020

timja commented Jun 1, 2020

varyvol commented Jun 4, 2020

EstherAF commented Jun 4, 2020

varyvol commented Jun 5, 2020

EstherAF commented Jun 7, 2020

timja left a comment

fqueiruga left a comment

EstherAF commented Jun 10, 2020 •

edited

Loading

timja left a comment

timja commented Jun 11, 2020

oleg-nenashev left a comment

[JENKINS-62496] Convert arrow pngs to CSS #4757

[JENKINS-62496] Convert arrow pngs to CSS #4757

Conversation

EstherAF commented May 29, 2020 • edited Loading

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

oleg-nenashev left a comment • edited Loading

Choose a reason for hiding this comment

oleg-nenashev May 30, 2020

Choose a reason for hiding this comment

fqueiruga May 30, 2020

Choose a reason for hiding this comment

oleg-nenashev May 30, 2020

Choose a reason for hiding this comment

fqueiruga May 30, 2020

Choose a reason for hiding this comment

oleg-nenashev May 30, 2020

Choose a reason for hiding this comment

fqueiruga May 30, 2020

Choose a reason for hiding this comment

timja left a comment • edited Loading

Choose a reason for hiding this comment

timja commented May 31, 2020

EstherAF commented Jun 1, 2020

timja commented Jun 1, 2020

varyvol commented Jun 4, 2020

EstherAF commented Jun 4, 2020

varyvol commented Jun 5, 2020

EstherAF commented Jun 7, 2020

timja left a comment

Choose a reason for hiding this comment

fqueiruga left a comment

Choose a reason for hiding this comment

EstherAF commented Jun 10, 2020 • edited Loading

timja left a comment

Choose a reason for hiding this comment

timja commented Jun 11, 2020

oleg-nenashev left a comment

Choose a reason for hiding this comment

EstherAF commented May 29, 2020 •

edited

Loading

oleg-nenashev left a comment •

edited

Loading

timja left a comment •

edited

Loading

EstherAF commented Jun 10, 2020 •

edited

Loading