jenkinsci / badge-plugin

Jenkins Badge plugin
https://plugins.jenkins.io/badge/
MIT License
32 stars 43 forks source link

[JENKINS-73187] - Do not apply icon size to text badges #153

Closed MarkEWaite closed 4 months ago

MarkEWaite commented 4 months ago

JENKINS-73187 - Do not apply icon size to text badges

This is a prototype that I'd like to use for discussion. I think that it may go "too far" along the path to compatibility with the 1.9.1 release of the badge plugin, but I think it is worth discussion from others who are more expert in CSS and page layout.

JENKINS-73187 describes result of the mistake that I made when setting the icon class to always include "icon-sm". Short text badges do not need to be limited by that class because it causes the text field to be too small for any text to fit.

Fixes #152 as well.

Includes a test for the icon class of short text.

That change (in the first commit in the pull request) resolves the most glaring issue, but then I started comparing the new layout with the 1.9.1 layout. The second commit restores the short text layout from the 1.9.1 release and removes the CSS styling of the short text box. The Jenkins design library probably has much better ways of styling that text box, but I've not explored those techniques.

The third commit in the pull request replaces paired "if / if not" conditions with paireed "when / otherwise" conditions. I think it makes the jelly file a little easier to read.

Testing done

Compared the layout of short text badges in Jenkins 2.452.1 with plugin version 1.9.1 and with this pull request.

Results for comparison will be uploaded to the pull request as images

### Submitter checklist
- [x] Make sure you are opening from a **topic/feature/bugfix branch** (right side) and not your main branch!
- [x] Ensure that the pull request title represents the desired changelog entry
- [x] Please describe what you did
- [x] Link to relevant issues in GitHub or Jira
- [x] Link to relevant pull requests, esp. upstream and downstream changes
- [x] Ensure you have provided tests - that demonstrates feature works or fixes the issue

Testing details

Compared the results with badge plugin 1.9.1 and this pull request

Short text

With the Pipeline script:

addShortText text: 'Less text'

Badge 1.9.1

Screenshot 2024-05-18 085623

Badge PR-153

Screenshot 2024-05-18 085956

Short text with yellow background

With the Pipeline script:

addShortText text: 'few words', background: 'yellow'

Badge 1.9.1

Screenshot 2024-05-18 090359

Badge PR-153

Screenshot 2024-05-18 090423

strangelookingnerd commented 4 months ago

I like the idea and followed a quite similar one in #151 as well. I expose the style and class attributes of an enclosing <span> for the text / icon and let the user have all the freedom they desire. For the icons itself I still hard-coded icon-sm as class, just to not render huge images by default. This may go a little too far for this issue here.

I can see some minor differences in the screenshots you provided that I'd like to investigate a little further and see how some edge cases behave (larger text, multiple rows, etc.).

mPokornyETM commented 4 months ago

@MarkEWaite do you want to merge it?

MarkEWaite commented 4 months ago

Thanks @strangelookingnerd . I'll merge it and let you run the release.