Attachment 174385 Details for Bug 36221 – Bug 36221: (follow-up) Style and markup fixes

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

Bug-36221-follow-up-Style-and-markup-fixes.patch (text/plain), 13.07 KB, created by Owen Leonard on 2024-11-11 18:18:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2024-11-11 18:18:45 UTC

Size: 13.07 KB

patch

obsolete

>From 26cadb15ffccd6fc2be6244740de0b730a677e6c Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Mon, 11 Nov 2024 18:17:13 +0000
>Subject: [PATCH] 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."
>---
> Koha/ILL/Backend/intra-includes/create.inc    | 13 +++---
> Koha/ILL/Backend/intra-includes/migrate.inc   | 22 ++++++----
> Koha/ILL/Backend/opac-includes/create.inc     | 36 +++++++++-------
> .../shared-includes/form_input_helpers.inc    | 42 +++++++++++--------
> Koha/ILL/Backend/shared-includes/shared.js    |  7 ++--
> 5 files changed, 70 insertions(+), 50 deletions(-)
>
>diff --git a/Koha/ILL/Backend/intra-includes/create.inc b/Koha/ILL/Backend/intra-includes/create.inc
>index 1df44f2974..67af446b95 100644
>--- a/Koha/ILL/Backend/intra-includes/create.inc
>+++ b/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> Branch 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 borrower details you entered are invalid.</p>
>+<p><em>Please Note:</em> The patron details you entered are invalid.</p>
> [% ELSIF whole.status == 'invalid_branch' %]
>-<p><em>Please Note:</em> The branch 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,11 +104,12 @@
>             </option>
>           [% END %]
>         </select>
>+        <span class="required">Required</span>
>       </li>
>     </ol>
>   </fieldset>
>   <fieldset class="action">
>-    <input id="ill-submit" type="submit" value="Create" disabled />
>+    <input id="ill-submit" class="btn btn-primary" type="submit" value="Create" disabled />
>     <a class="cancel" href="/cgi-bin/koha/ill/ill-requests.pl">Cancel</a>
>   </fieldset>
>   <input type="hidden" name="method" value="create" />
>diff --git a/Koha/ILL/Backend/intra-includes/migrate.inc b/Koha/ILL/Backend/intra-includes/migrate.inc
>index ea85d7e1e8..0fbbe91aab 100644
>--- a/Koha/ILL/Backend/intra-includes/migrate.inc
>+++ b/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> Branch 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 borrower 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 branch 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>
>diff --git a/Koha/ILL/Backend/opac-includes/create.inc b/Koha/ILL/Backend/opac-includes/create.inc
>index 13078deae2..3e8f5581c4 100644
>--- a/Koha/ILL/Backend/opac-includes/create.inc
>+++ b/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> Branch 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 borrower 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 branch 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 %]
>\ No newline at end of file
>+[% END %]
>diff --git a/Koha/ILL/Backend/shared-includes/form_input_helpers.inc b/Koha/ILL/Backend/shared-includes/form_input_helpers.inc
>index 63b42a3482..5ac8be9b86 100644
>--- a/Koha/ILL/Backend/shared-includes/form_input_helpers.inc
>+++ b/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 %]
>\ No newline at end of file
>+[% END %]
>diff --git a/Koha/ILL/Backend/shared-includes/shared.js b/Koha/ILL/Backend/shared-includes/shared.js
>index a341464e83..12db43984d 100644
>--- a/Koha/ILL/Backend/shared-includes/shared.js
>+++ b/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-delete">' +
>-            '</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 {
>-- 
>2.39.5

Actions: View | Diff | Splinter Review

Attachments on bug 36221: 162715 | 162716 | 162933 | 162934 | 162948 | 162949 | 162970 | 162971 | 165462 | 165463 | 165464 | 165465 | 173706 | 173752 | 173753 | 173754 | 173755 | 173756 | 173939 | 173940 | 173941 | 173965 | 173969 | 174017 | 174018 | 174019 | 174020 | 174021 | 174022 | 174023 | 174024 | 174025 | 174026 | 174027 | 174385 | 174398 | 174454