Bug 36221: (follow-up) Style and markup fixes

This patch makes several changes to make the updated forms more
consistent with how other pages work:

- Form labels should have ":" after them.
- Form fields should have a hint when they are required.
  - Required fields should not be indicated with an asterisk.
- In the OPAC, <select> should have a "form-select" class if the form is
  otherwise being styled with Bootstrap classes.
- Primary submit buttons should have a "btn btn-primary" class.
- The Font Awesome 6 icon for a delete operation is "fa-trash-can."
- Error messages should be wrapped in <div class="alert alert-warning">.
- All language strings in JS files should be wrapped in __('').
- Terminology: "Library" instead of "Branch," "Patron" instead of
  "Member."

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

Koha/ILL/Backend/intra-includes/create.inc

@@ -5,11 +5,11 @@
 [% IF whole.status == 'missing_identifier' %]
 <p><em>Please Note:</em> Mandatory field Identifier is missing.</p>
 [% ELSIF whole.status == 'missing_branch' %]
-<p><em>Please Note:</em> Destination library is a mandatory field.</p>
+<p><em>Please Note:</em> Library is a mandatory field.</p>
 [% ELSIF whole.status == 'invalid_borrower' %]
 <p><em>Please Note:</em> The patron details you entered are invalid.</p>
 [% ELSIF whole.status == 'invalid_branch' %]
-<p><em>Please Note:</em> The destination library you chose is invalid.</p>
+<p><em>Please Note:</em> The library you chose is invalid.</p>
 [% ELSE %]
 <p>Unhandled error</p>
 [% END %]
@@ -87,8 +87,8 @@
         <label class="required" for="cardnumber">
           Card number, username or surname:
         </label>
-        <input type="text" name="cardnumber" id="cardnumber" autocomplete="off"
-               type="text" value="[% whole.value.other.cardnumber | html %]" />
+        <input type="text" name="cardnumber" id="cardnumber" autocomplete="off" type="text" value="[% whole.value.other.cardnumber | html %]" />
+        <span class="required">Required</span>
       </li>
       <li>
         <label class="required" for="branchcode">Destination library:</label>
@@ -104,6 +104,7 @@
             </option>
           [% END %]
         </select>
+        <span class="required">Required</span>
       </li>
     </ol>
   </fieldset>

Koha/ILL/Backend/intra-includes/migrate.inc

@@ -2,11 +2,11 @@
 [% IF whole.status == 'missing_identifier' %]
 <p><em>Please Note:</em> Mandatory field Identifier is missing.</p>
 [% ELSIF whole.status == 'missing_branch' %]
-<p><em>Please Note:</em> Destination library is a mandatory field.</p>
+<p><em>Please Note:</em> Library is a mandatory field.</p>
 [% ELSIF whole.status == 'invalid_borrower' %]
-<p><em>Please Note:</em> The patron details you entered are invalid.</p>
+<p><em>Please Note:</em> The patrons details you entered are invalid.</p>
 [% ELSIF whole.status == 'invalid_branch' %]
-<p><em>Please Note:</em> The destination library you chose is invalid.</p>
+<p><em>Please Note:</em> The library you chose is invalid.</p>
 [% ELSE %]
 <p>Unhandled error</p>
 [% END %]
@@ -20,8 +20,8 @@
     <ol id="general-standard-fields">
       <li>
         <label class="required" for="type">Type:</label>
-        <select name="type" id="type">
-          <option value=""/>
+        <select class="form-select" name="type" id="type">
+          <option value=""></option>
           [% IF whole.value.other.type.lower == "book" %]
           <option value="book" selected="selected">Book</option>
           [% ELSE %]
@@ -68,6 +68,9 @@
           <option value="resource">Generic resource</option>
           [% END %]
         </select>
+        [% IF required %]
+            <div class="required_label required">Required</div>
+        [% END %]
       </li>
     </ol>
   </fieldset>
@@ -84,19 +87,20 @@
         <label class="required" for="cardnumber">
           Card number or surname:
         </label>
-        <input type="text" name="cardnumber" id="cardnumber"
-               type="text" value="" />
+        <input type="text" name="cardnumber" id="cardnumber" type="text" value="" />
+            <div class="required_label required">Required</div>
       </li>
       <li>
-        <label class="required" for="branchcode">Destination library:</label>
+        <label class="required" for="branchcode">Destination library:</label>xxx
         <select id="branchcode" name="branchcode">
-          <option value="" />
+          <option value=""></option>
           [% FOREACH branch IN branches %]
             <option value="[% branch.branchcode | html %]">
               [% branch.branchname | html %]
             </option>
           [% END %]
         </select>
+            <div class="required_label required">Required</div>
       </li>
     </ol>
   </fieldset>

Koha/ILL/Backend/opac-includes/create.inc

@@ -3,15 +3,25 @@
 [% PROCESS "${cwd}/shared-includes/form_input_helpers.inc" %]
 [% IF whole.error %]
 [% IF whole.status == 'missing_identifier' %]
-<p><em>Please Note:</em> Mandatory field Identifier is missing.</p>
+    <div class="alert alert-warning">
+        <strong>Please note:</strong> Mandatory field Identifier is missing.
+    </div>
 [% ELSIF whole.status == 'missing_type' %]
-<p><em>Please Note:</em> Type is a mandatory field.</p>
+    <div class="alert alert-warning">
+        <strong>Please note:</strong> Type is a mandatory field.
+    </div>
 [% ELSIF whole.status == 'missing_branch' %]
-<p><em>Please Note:</em> Destination library is a mandatory field.</p>
+    <div class="alert alert-warning">
+        <strong>Please note:</strong> Library is a mandatory field.
+    </div>
 [% ELSIF whole.status == 'invalid_borrower' %]
-<p><em>Please Note:</em> The patron details you entered are invalid.</p>
+    <div class="alert alert-warning">
+        <strong>Please note:</strong> The patron details you entered are invalid.
+    </div>
 [% ELSIF whole.status == 'invalid_branch' %]
-<p><em>Please Note:</em> The destination library you chose is invalid.</p>
+    <div class="alert alert-warning">
+        <strong>Please note:</strong> The library you chose is invalid.
+    </div>
 [% ELSE %]
 <p>Unhandled error</p>
 [% END %]
@@ -25,12 +35,11 @@
     <fieldset class="rows">
     <legend>Patron options</legend>
     <ol>
-      <li>
         [% WRAPPER ill_select_field required = 1 id = 'branchcode' label = t('Destination library') %]
-          <option value="" />
+          <option value=""></option>
           [% FOREACH branch IN branches %]
             [% IF whole.value.other.branchcode && branch.branchcode == whole.value.other.branchcode %]
-            <option value="[% branch.branchcode | html %]" selected>
+            <option value="[% branch.branchcode | html %]" selected="selected">
               [% branch.branchname | html %]
             </option>
             [% ELSE %]
@@ -40,15 +49,13 @@
             [% END %]
           [% END %]
         [% END #ill_select_field %]
-      </li>
     </ol>
   </fieldset>
   <fieldset class="rows">
     <legend>General details</legend>
     <ol id="general-standard-fields">
-    [% WRAPPER ill_select_field required = 1 id = 'type' label = t('Type') %]
-      <li>
-          <option value=""/>
+        [% WRAPPER ill_select_field required = 1 id = 'type' label = t('Type') %]
+          <option value=""></option>
           [% IF whole.value.other.type.lower == "book" %]
           <option value="book" selected="selected">Book</option>
           [% ELSE %]
@@ -94,8 +101,7 @@
           [% ELSE %]
           <option value="resource">Generic resource</option>
           [% END %]
-        </li>
-    [% END #ill_select_field %]
+        [% END #ill_select_field %]
     </ol>
   </fieldset>
   [% type = whole.value.other.type %]
@@ -122,4 +128,4 @@
     [% INCLUDE "${cwd}/shared-includes/shared.js" %]
     // ]]>
 </script>
-[% END %]
+[% END %]

Koha/ILL/Backend/shared-includes/form_input_helpers.inc

@@ -1,25 +1,33 @@
 [% BLOCK ill_text_input_field %]
     <li>
-      <label for="[% id | html %]">[% label | html %][% IF required %]*[% END %]</label>
-      [% IF opac %]
-        <input class="form-control input-fluid" type="text" name="[% id | html %]" id="[% id | html%]" value="[% value | html %]" />
-      [% ELSE %]
-        <input type="text" name="[% id | html %]" id="[% id | html%]" value="[% value | html %]" />
-      [% END %]
+        <label for="[% id | html %]">[% label | html %]:</label>
+        [% IF opac %]
+            <input class="form-control input-fluid" type="text" name="[% id | html %]" id="[% id | html%]" value="[% value | html %]" />
+        [% ELSE %]
+            <input type="text" name="[% id | html %]" id="[% id | html%]" value="[% value | html %]" />
+        [% END %]
+        [% IF required %]
+            <div class="required_label required">Required</div>
+        [% END %]
+        [% SET required = 0 %]
     </li>
-    [% SET required = 0 %]
 [% END %]
 
 [% BLOCK ill_select_field %]
-      <label for="[% id | html %]">[% label | html %][% IF required %]*[% END %]</label>
-      [% IF opac %]
-        <select class="form-control input-fluid" name="[% id | html %]" id="[% id | html %]">
-      [% ELSE %]
-        <select name="[% id | html %]" id="[% id | html %]">
-      [% END %]
-        [% content | $raw %]
-      </select>
-      [% SET required = 0 %]
+    <li>
+        <label for="[% id | html %]">[% label | html %]:</label>
+        [% IF opac %]
+            <select class="form-select input-fluid" name="[% id | html %]" id="[% id | html %]">
+        [% ELSE %]
+            <select name="[% id | html %]" id="[% id | html %]">
+        [% END %]
+            [% content | $raw %]
+        </select>
+        [% IF required %]
+            <div class="required_label required">Required</div>
+        [% END %]
+        [% SET required = 0 %]
+    </li>
 [% END %]
 
 [% BLOCK book_text_input_fields %]
@@ -41,4 +49,4 @@
   [% PROCESS ill_text_input_field id = 'issue' label = t('Issue number') value = whole.value.other.issue %]
   [% PROCESS ill_text_input_field id = 'year' label = t('Year') value = whole.value.other.year %]
   [% PROCESS ill_text_input_field id = 'issn' label = t('ISSN') value = whole.value.other.issn %]
-[% END %]
+[% END %]

Koha/ILL/Backend/shared-includes/shared.js

@@ -12,8 +12,9 @@ document.addEventListener('DOMContentLoaded', function() {
             ( opac ? 'form-control input-fluid custom-field-input' : '') +'" placeholder="'+__('value')+'"> ' +
             '<button type="button" class="btn btn-danger btn-sm ' +
             'delete-new-field">' +
-            '<span class="fa fa-trash">' +
-            '</span> '+__('Delete')+'</button></li>';
+            '<span class="fa fa-trash-can"></span> ' +
+            __('Delete') +
+            '</button></li>';
         $('#standard-fields').append(row);
     });
     $('#standard-fields').on('click', '.delete-new-field',
@@ -31,7 +32,7 @@ document.addEventListener('DOMContentLoaded', function() {
     $('#standard-fields').on('keyup', '.custom-name', function() {
         var val = $(this).val();
         if (core.indexOf(val.toLowerCase()) > -1) {
-            $('#custom-warning').text(_('The name "' + val + '" is not permitted')).show();
+            $('#custom-warning').text(__('The name "' + val + '" is not permitted')).show();
             $('#ill-submit').attr('disabled', true);
             $('#add-new-fields').attr('disabled', true);
         } else {