Refine Add button for repeatable lists (#10913)

* Refine repeatable list Add button

* Update hetero-list.js

* Update HeteroListTest.java

* Update RepeatableTest.java

* Trigger Build

* Update ath.sh

Author: janfaracik

core/src/main/resources/lib/form/hetero-list.jelly

@@ -154,7 +154,7 @@ THE SOFTWARE.
 
     <j:if test="${!readOnlyMode}">
       <div>
-        <button type="button" class="jenkins-button hetero-list-add" menualign="${attrs.menuAlign}" suffix="${attrs.name}">${attrs.addCaption?:'%Add'}<l:icon src="symbol-chevron-down"/>
+        <button type="button" class="jenkins-button hetero-list-add" menualign="${attrs.menuAlign}" suffix="${attrs.name}"><l:icon src="symbol-add"/>${attrs.addCaption?:'%Add'}
         </button>
       </div>
     </j:if>

core/src/main/resources/lib/form/repeatable.jelly

@@ -23,7 +23,7 @@ THE SOFTWARE.
 -->
 
 <?jelly escape-by-default='true'?>
-<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define">
+<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout">
   <st:documentation> <![CDATA[
     Repeatable blocks used to present UI where the user can configure multiple entries
     of the same kind (see the Java installations configuration in the system config.)
@@ -143,6 +143,7 @@ THE SOFTWARE.
     </div>
     <j:if test="${!empty(items) and !attrs.noAddButton and attrs.enableTopButton}">
       <button type="button" class="jenkins-button repeatable-add repeatable-add-top">
+        <l:icon src="symbol-add"/>
         ${attrs.add?:'%Add'}
       </button>
     </j:if>
@@ -173,6 +174,7 @@ THE SOFTWARE.
     <div class="repeatable-insertion-point" />
     <j:if test="${!attrs.noAddButton}">
       <button type="button" class="jenkins-button repeatable-add">
+        <l:icon src="symbol-add"/>
         ${attrs.add?:'%Add'}
       </button>
     </j:if>

src/main/js/components/dropdowns/hetero-list.js

@@ -167,6 +167,23 @@ function generateButtons() {
 
       let oneEach = e.classList.contains("one-each");
 
+      /**
+       * Disable the Add button if there are no more items to add
+       */
+      function toggleButtonState() {
+        const templateCount = templates.length;
+        const selectedCount = Array.from(e.children).filter((e) =>
+          e.classList.contains("repeated-chunk"),
+        ).length;
+
+        btn.disabled = oneEach && selectedCount === templateCount;
+      }
+      const observer = new MutationObserver(() => {
+        toggleButtonState();
+      });
+      observer.observe(e, { childList: true });
+      toggleButtonState();
+
       generateDropDown(btn, (instance) => {
         let menuItems = [];
         for (let i = 0; i < templates.length; i++) {

src/main/scss/components/_buttons.scss

@@ -230,8 +230,7 @@
   }
 }
 
-.advanced-button,
-.hetero-list-add {
+.advanced-button {
   svg {
     width: 0.875rem;
     height: 0.875rem;

src/main/scss/components/_dropdowns.scss

@@ -117,11 +117,11 @@ $dropdown-padding: 0.375rem;
   &__disabled {
     color: var(--text-color-secondary) !important;
     font-size: 0.8125rem;
-    opacity: 0.8;
+    opacity: 0.5;
     display: inline-flex;
     align-items: center;
     margin: 0;
-    cursor: default;
+    cursor: not-allowed;
   }
 
   &__item {

test/src/test/java/lib/form/HeteroListTest.java

@@ -86,7 +86,7 @@ void xssPrevented_heteroList_usingDescriptorDisplayName() throws Exception {
         assertThat(result, instanceOf(HTMLButtonElement.class));
         HTMLButtonElement menuItem = (HTMLButtonElement) result;
         String menuItemContent = menuItem.getInnerHTML();
-        assertThat(menuItemContent, not(containsString("<")));
+        assertThat(menuItemContent, not(containsString("<img")));
     }
 
     @Test
@@ -100,7 +100,7 @@ void xssPrevented_usingToolInstallation_repeatableAddExisting() throws Exception
         Object result = page.executeJavaScript("Array.from(document.querySelectorAll('button')).filter(b => b.textContent.indexOf('Add XSS') !== -1)[0].innerHTML").getJavaScriptResult();
         assertThat(result, instanceOf(String.class));
         String resultString = (String) result;
-        assertThat(resultString, not(containsString("<")));
+        assertThat(resultString, not(containsString("<img")));
     }
 
     // only possible after a partial fix
@@ -121,7 +121,7 @@ void xssPrevented_usingToolInstallation_repeatableAddAfterClick() throws Excepti
         Object result = page.executeJavaScript("Array.from(document.querySelectorAll('button')).filter(b => b.textContent.indexOf('Add XSS') !== -1)[0].innerHTML").getJavaScriptResult();
         assertThat(result, instanceOf(String.class));
         String resultString = (String) result;
-        assertThat(resultString, not(containsString("<")));
+        assertThat(resultString, not(containsString("<img")));
     }
 
     @Test

test/src/test/java/lib/form/RepeatableTest.java

@@ -647,7 +647,7 @@ private static HtmlButton getHtmlButton(HtmlForm form, String buttonCaption, boo
      */
     private static List<?> getButtonsList(HtmlForm form, String buttonCaption) {
         return form.getByXPath(
-                String.format("//button[text() = '%s'] | //button[@tooltip = '%s']", buttonCaption, buttonCaption)
+                String.format("//button[normalize-space(string(.)) = '%s'] | //button[@tooltip = '%s']", buttonCaption, buttonCaption)
         );
     }