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

* [JENKINS-73187] - Do not apply icon size to text badges https://issues.jenkins.io/browse/JENKINS-73187 describes 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. Include a test for the icon class of short text. * Replace assets.css with 1.9.1 compatible styling The CSS file makes the short text badge better looking but also makes it use much more space. Better to switch back to the previous layout rather than disrupt existing users with a larger layout. * Use when/otherwise instead of if a / if !a in jelly file Makes it clear where the conditional is being applied. --------- Co-authored-by: strangelookingnerd <[email protected]>
jenkinsci · May 21, 2024 · db43b1b · db43b1b
1 parent 09368e2
commit db43b1b
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 55 deletions.
diff --git a/src/main/java/com/jenkinsci/plugins/badge/action/BadgeAction.java b/src/main/java/com/jenkinsci/plugins/badge/action/BadgeAction.java
@@ -200,7 +200,9 @@ public String getBorderColor() {
      */
     public String getIconClass() {
         List<String> classes = new LinkedList<>();
-        classes.add("icon-sm"); // control icon image size
+        if (!isTextOnly()) {
+            classes.add("icon-sm"); // control image size only when using an icon
+        }
 
         if (this.color != null) {
             if (this.color.startsWith("jenkins-!-")) {

diff --git a/src/main/resources/com/jenkinsci/plugins/badge/action/BadgeAction/badge.jelly b/src/main/resources/com/jenkinsci/plugins/badge/action/BadgeAction/badge.jelly
@@ -29,42 +29,45 @@ THE SOFTWARE.
   <j:if test="${it.iconPath != null}">
     <j:set var="badgeStyle" value=""/>
     <j:if test="${it.iconColorStyle != null}">
-      <j:set var="badgeStyle" value="${badgeStyle};color: ${it.iconColorStyle};"/>
-    </j:if>
-
-    <j:if test="${it.link == null}">
-      <l:icon src="${it.iconPath}" class="${it.iconClass}" style="${badgeStyle}" htmlTooltip="${it.text}" />
+      <j:set var="badgeStyle" value="color: ${it.iconColorStyle};"/>
     </j:if>
 
-    <j:if test="${it.link != null}">
-      <a href="${it.link}"><l:icon src="${it.iconPath}" class="${it.iconClass}" style="${badgeStyle}" htmlTooltip="${it.text}" /></a>
-    </j:if>
+    <j:choose>
+      <j:when test="${it.link == null}">
+        <l:icon src="${it.iconPath}" class="${it.iconClass}" style="${badgeStyle}" htmlTooltip="${it.text}" />
+      </j:when>
+      <j:otherwise>
+        <a href="${it.link}"><l:icon src="${it.iconPath}" class="${it.iconClass}" style="${badgeStyle}" htmlTooltip="${it.text}" /></a>
+      </j:otherwise>
+    </j:choose>
   </j:if>
 
   <j:if test="${it.iconPath == null}">
-    <st:adjunct includes="com.jenkinsci.plugins.badge.assets"/>
-    <j:set var="badgeStyle" value=""/>
+    <j:set var="badgeStyle" value="border: solid"/>
     <j:set var="badgeClass" value="badge-shortText"/>
     <j:if test="${it.border != null}">
-      <j:set var="badgeStyle" value="${badgeStyle};border: ${it.border} solid ${it.borderColor}"/>
-    </j:if>
-    <j:if test="${it.background != null}">
-      <j:set var="badgeStyle" value="${badgeStyle};background: ${it.background}"/>
-    </j:if>
-    <j:if test="${it.background == null}">
-      <j:set var="badgeClass" value="${badgeClass} badge-shortText--default-background"/>
+      <j:set var="badgeStyle" value="border: ${it.border} solid ${it.borderColor}"/>
     </j:if>
+    <j:choose>
+      <j:when test="${it.background == null}">
+        <j:set var="badgeClass" value="${badgeClass} badge-shortText--default-background"/>
+      </j:when>
+      <j:otherwise>
+        <j:set var="badgeStyle" value="${badgeStyle};background: ${it.background}"/>
+      </j:otherwise>
+    </j:choose>
     <j:if test="${it.iconColorStyle != null}">
       <j:set var="badgeStyle" value="${badgeStyle};color: ${it.iconColorStyle};"/>
     </j:if>
 
-    <j:if test="${it.link == null}">
-      <div style="${badgeStyle}" class="${it.iconClass} ${badgeClass}">${it.text}</div>
-    </j:if>
-
-    <j:if test="${it.link != null}">
-      <a href="${it.link}"><span style="${badgeStyle}" class="${it.iconClass} ${badgeClass}">${it.text}</span></a>
-    </j:if>
+    <j:choose>
+      <j:when test="${it.link == null}">
+        <span style="${badgeStyle}" class="${it.iconClass} ${badgeClass}">${it.text}</span>
+      </j:when>
+      <j:otherwise>
+        <a href="${it.link}"><span style="${badgeStyle}" class="${it.iconClass} ${badgeClass}">${it.text}</span></a>
+      </j:otherwise>
+    </j:choose>
   </j:if>
 
 </j:jelly>
diff --git a/src/main/resources/com/jenkinsci/plugins/badge/assets.css b/src/main/resources/com/jenkinsci/plugins/badge/assets.css
diff --git a/src/test/java/com/jenkinsci/plugins/badge/action/BadgeActionTest.java b/src/test/java/com/jenkinsci/plugins/badge/action/BadgeActionTest.java
@@ -66,6 +66,8 @@ void getIconClass(@SuppressWarnings("unused") JenkinsRule r) {
         assertEquals("icon-sm jenkins-!-color-blue", action.getIconClass());
         action = BadgeAction.createBadge("symbol-star plugin-ionicons-api", "jenkins-!-color-red", "", null);
         assertEquals("icon-sm jenkins-!-color-red", action.getIconClass());
+        action = BadgeAction.createShortText("Short text");
+        assertEquals("", action.getIconClass()); // Short text must not set icon-sm class
         // teal is not in the palette
         action = BadgeAction.createBadge("symbol-star plugin-ionicons-api", "teal", "", null);
         assertEquals("icon-sm", action.getIconClass());