Revamp Feed bar and Add/Edit description button to be consistent + correctly spaced #5664

janfaracik · 2021-08-14T10:08:59Z

Proposed changelog entries

Update appearance for feed bar and description button to be modern and consistent

Screenshots

Old

New

What's changed under the hood?

Added two new SVGs, one for the add/edit button and one for the feed button (both from Material Icons)
Added a buttons-row class which correctly spaces buttons in a row
The size buttons now read correctly when using a screenreader, eg button S will read as Small, M will read as Medium and L will read as Large
Breadcrumb bar chevron is now the same colour as the text that proceeds it, easier to see + accessibility

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

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

Localizations will be affected. Maybe it is better to avoid patching the keys and update the default localization property file

oleg-nenashev · 2021-08-14T10:25:13Z

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

@@ -194,7 +194,7 @@ THE SOFTWARE.
    <div id="page-head">
      <l:pageHeader title="${%title}"
                    logoAlt="${%jenkinshead.alt}"
-                    searchPlaceholder="${%search}"
+                    searchPlaceholder="${%Search}"


It will impact localizations. Would be nice to clean it up. You can also just update the english localization and keep the key in *.jelly intact

Good spot! I've moved any changed strings to the localisation files.

timja

looks good, I've tested via core-pr-tester

one thing we could consider since we're in this area anyway is updating the text from 'Submit' to 'Save' when editing the description?

Any thoughts?

jenkins/core/src/main/resources/hudson/model/AbstractModelObject/descriptionForm.jelly

Line 42 in 972c6fa

<f:submit />

daniel-beck · 2021-08-14T19:54:51Z

Breadcrumb bar chevron is now the same colour as the text that proceeds it, easier to see + accessibility

Could someone provide before/after screenshots?

timja · 2021-08-15T07:14:22Z

Breadcrumb bar chevron is now the same colour as the text that proceeds it, easier to see + accessibility

Could someone provide before/after screenshots?

The arrow is solid now rather than faded out

Before:

After:

On dark theme it looks identical

oleg-nenashev

The change itself looks really good, +1 from me. No concerns about the arrow changes. We will need to update the changelog a bit, but there are no major obstacles IMHO

Requesting changes because of the license header in the new file, happy to approve once fixed

core/src/main/resources/lib/hudson/editableDescription.properties

Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com>

…is empty

… into prioritize-svgs

core/src/main/resources/hudson/model/AbstractModelObject/editDescription.jelly

oleg-nenashev

Looks good to me. Thanks @janfaracik !
We are missing the merge window for the tomorrow's weekly anyway. I suggest merging on Thursday after the Wednesday's UX SIG meeting if no negative feedback

timja · 2021-08-16T20:06:56Z

Looks good to me. Thanks @janfaracik !
We are missing the merge window for the tomorrow's weekly anyway. I suggest merging on Thursday after the Wednesday's UX SIG meeting if no negative feedback

There's no ux sig meeting till 1st of September, let's merge sometime in the next week, no rush

uhafner

Thanks, the new layout looks much better!

(I still would prefer actual icons in the corresponding size as buttons rather than the letters S, M, and L but that is unrelated to your change and an existing usability problem.)

uhafner · 2021-08-16T21:18:52Z

core/src/main/resources/hudson/logging/LogRecorderManager/feeds.jelly

@@ -28,22 +28,22 @@ THE SOFTWARE.
    <div align="right">
        <a href="rss" class="yui-button link-button">
            <span class="leading-icon">
-                <l:svgIcon class="icon-small" tooltip="Atom feed"
-                           href="${imagesURL}/material-icons/svg-sprite-communication-symbol.svg#ic_rss_feed_24px"/>
+                <l:svgIcon class="icon-small" tooltip="Atom feed" viewBox="viewbox (default 0 0 16 16)"


Is there a way to avoid providing a fixed view box every time?

I think there should be an alternative to the viewbox changes here. Maybe a transform: scale

The syntax seems to be wrong, I now get several console errors;

https://issues.jenkins.io/browse/JENKINS-66472

Noticed that whilst working on something else, I've got a fix for that in another feature PR - https://github.com/jenkinsci/jenkins/pull/5692/files - happy to pull the fix out into a separate bug PR if that makes things easier :)

fqueiruga

Changing default padding for buttons is dangerous IMO. I'd suggest you create custom CSS for the feed bar.

I'm also not sure whether the .jenkins-buttons-row is generic enough that it can be used elsewhere. I wouldn't mind some component-specific naming like .feed-buttons-row or something like that.

fqueiruga · 2021-08-17T08:01:24Z

war/src/main/less/modules/buttons.less

@@ -123,7 +123,7 @@ a.yui-button:visited {
  box-sizing: border-box;
  // vertical padding:
  // 0.375rem == 6px == 32px (target height) - 4px (borders) - 16 (line) / 2
-  padding: 0.375rem 1rem;
+  padding: 0.375rem 0.5rem;


I think this change should not take place because it will affect all buttons and it can be have unexpected . Any specific modifications could be done using custom CSS.

fqueiruga · 2021-08-17T08:01:30Z

war/src/main/less/modules/buttons.less

@@ -167,11 +167,11 @@ a.yui-button:visited {
  }

  .leading-icon {
-    margin-right: 0.25rem;
+    margin-right: 0.5rem;


Ditto about custom CSS

fqueiruga · 2021-08-17T08:01:37Z

war/src/main/less/modules/buttons.less

  }

  .trailing-icon {
-    margin-left: 0.25rem;
+    margin-left: 0.5rem;


Ditto about custom CSS

fqueiruga · 2021-08-17T08:03:15Z

core/src/main/resources/hudson/logging/LogRecorderManager/feeds.jelly

@@ -28,22 +28,22 @@ THE SOFTWARE.
    <div align="right">
        <a href="rss" class="yui-button link-button">
            <span class="leading-icon">
-                <l:svgIcon class="icon-small" tooltip="Atom feed"
-                           href="${imagesURL}/material-icons/svg-sprite-communication-symbol.svg#ic_rss_feed_24px"/>
+                <l:svgIcon class="icon-small" tooltip="Atom feed" viewBox="viewbox (default 0 0 16 16)"


I think there should be an alternative to the viewbox changes here. Maybe a transform: scale

Jan Faracik added 4 commits August 13, 2021 23:46

Rework buttons/icon/rss bar

1f2c1b1

Remove SVG, update feed icon

58fdefb

Update editableDescription.jelly

b1da7ae

Make icons more accessible

1906bf1

oleg-nenashev added web-ui The PR includes WebUI changes which may need special expertise rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted labels Aug 14, 2021

oleg-nenashev reviewed Aug 14, 2021

View reviewed changes

Jan Faracik added 4 commits August 14, 2021 12:37

Localize

f06c362

Update editableDescription.jelly

c54c830

Create editableDescription.properties

bea529a

localize more things

f50ac41

timja approved these changes Aug 14, 2021

View reviewed changes

timja requested review from fqueiruga, a team and oleg-nenashev August 14, 2021 17:42

oleg-nenashev requested changes Aug 15, 2021

View reviewed changes

core/src/main/resources/lib/hudson/editableDescription.properties Outdated Show resolved Hide resolved

Update core/src/main/resources/lib/hudson/editableDescription.properties

2991b17

Co-authored-by: Oleg Nenashev <o.v.nenashev@gmail.com>

timja requested a review from oleg-nenashev August 15, 2021 11:17

Jan Faracik added 3 commits August 15, 2021 12:22

Change button submit or save depending on whether or not description …

a8eb2ae

…is empty

Update editDescription.jelly

d3f96f9

Merge branch 'prioritize-svgs' of https://github.com/janfaracik/jenkins…

b9f2cd0

… into prioritize-svgs

timja reviewed Aug 15, 2021

View reviewed changes

core/src/main/resources/hudson/model/AbstractModelObject/editDescription.jelly Outdated Show resolved Hide resolved

Jan Faracik added 2 commits August 15, 2021 12:54

Update rssBar.properties

f266bc1

Undo change, keep 'Save'

8b0542c

timja approved these changes Aug 15, 2021

View reviewed changes

timja requested review from uhafner and romenrg August 15, 2021 19:29

oleg-nenashev approved these changes Aug 16, 2021

View reviewed changes

oleg-nenashev added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 16, 2021

uhafner approved these changes Aug 16, 2021

View reviewed changes

fqueiruga reviewed Aug 17, 2021

View reviewed changes

Update buttons.less

71c25c3

oleg-nenashev requested a review from fqueiruga August 22, 2021 20:19

timja merged commit 04a98ff into jenkinsci:master Aug 23, 2021

janfaracik deleted the prioritize-svgs branch January 5, 2022 15:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revamp Feed bar and Add/Edit description button to be consistent + correctly spaced #5664

Revamp Feed bar and Add/Edit description button to be consistent + correctly spaced #5664

janfaracik commented Aug 14, 2021 •

edited

Loading

oleg-nenashev left a comment

oleg-nenashev Aug 14, 2021

janfaracik Aug 14, 2021

timja left a comment

daniel-beck commented Aug 14, 2021

timja commented Aug 15, 2021

oleg-nenashev left a comment

oleg-nenashev left a comment

timja commented Aug 16, 2021 •

edited

Loading

uhafner left a comment

uhafner Aug 16, 2021

fqueiruga Aug 17, 2021

uhafner Aug 27, 2021

janfaracik Aug 27, 2021

fqueiruga left a comment

fqueiruga Aug 17, 2021

fqueiruga Aug 17, 2021

fqueiruga Aug 17, 2021

fqueiruga Aug 17, 2021

Revamp Feed bar and Add/Edit description button to be consistent + correctly spaced #5664

Revamp Feed bar and Add/Edit description button to be consistent + correctly spaced #5664

Conversation

janfaracik commented Aug 14, 2021 • edited Loading

Proposed changelog entries

Screenshots

What's changed under the hood?

Proposed upgrade guidelines

Submitter checklist

Maintainer checklist

oleg-nenashev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timja left a comment

Choose a reason for hiding this comment

daniel-beck commented Aug 14, 2021

timja commented Aug 15, 2021

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev left a comment

Choose a reason for hiding this comment

timja commented Aug 16, 2021 • edited Loading

uhafner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fqueiruga left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

janfaracik commented Aug 14, 2021 •

edited

Loading

timja commented Aug 16, 2021 •

edited

Loading