Attachment 134469 Details for Bug 30579 – Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer

[patch] Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer

Bug-30579-follow-up-Update-layout-to-make-hold-any.patch (text/plain), 19.22 KB, created by Owen Leonard on 2022-05-02 16:14:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2022-05-02 16:14:09 UTC

Size: 19.22 KB

patch

obsolete

>From 9a178077dbcb80c364e633c4540c1234ea594112 Mon Sep 17 00:00:00 2001
>From: Owen Leonard <oleonard@myacpl.org>
>Date: Mon, 2 May 2022 12:48:32 +0000
>Subject: [PATCH] Bug 30579: (follow-up) Update layout to make hold any vs.
> hold item clearer
>
>This patch alters the structure of the hold for a little more in order
>to make it clear that "Hold next available" and "Hold specific item" are
>two mutually-exclusive options.
>
>To test, apply the patch and rebuild the staff interface CSS
>(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client).
>
>Perform the same tests as before:
>
>- Confirm you can successfully place single or multiple items on hold
>- Confirm that when placing a single hold the next available options
>  are grouped
>- Confirm pickup location, specific item type, and number of holds all
>  work for next available
>- Confirm item level holds save pickup location correctly
>---
> .../prog/css/src/staff-global.scss            |   7 +
> .../prog/en/modules/reserve/request.tt        | 191 +++++++++---------
> 2 files changed, 104 insertions(+), 94 deletions(-)
>
>diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
>index 2f84696730..960ae39bc3 100644
>--- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
>+++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
>@@ -880,6 +880,13 @@ fieldset {
>             font-size: 110%;
>             font-weight: bold;
>             margin-left: 1em;
>+
>+            label {
>+                display: inline-block;
>+                float: none;
>+                margin: .2em;
>+                width: auto;
>+            }
>         }
> 
>         li {
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>index c0acbf807b..b5304f562e 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
>@@ -435,9 +435,9 @@
>                 </div>
> 
>                 [% UNLESS ( multi_hold ) %]
>-                    <fieldset class="rows">
>-                        <legend>Hold details</legend>
>-                        <form action="placerequest.pl" method="post" name="form" id="hold-request-form">
>+                    <form action="placerequest.pl" method="post" name="form" id="hold-request-form">
>+                        <fieldset class="rows">
>+                            <legend>Hold details</legend>
> 
>                             <input type="hidden" name="borrowernumber" value="[% patron.borrowernumber | html %]" />
>                             <input type="hidden" name="type" value="str8" />
>@@ -461,10 +461,6 @@
>                                 <span class="label">Estimated priority:</span>
>                                 <strong>[% fixedRank | html %]</strong>
>                             </li>
>-                            <li>
>-                                <label for="holdnotes">Notes:</label>
>-                                <textarea id="holdnotes" name="notes" cols="30" rows="1"></textarea>
>-                            </li>
>                             [% IF ( reserve_in_future ) %]
>                                 <li>
>                                     <label for="from">Hold starts on date:</label>
>@@ -482,52 +478,57 @@
>                                 <input name="non_priority" id="non_priority" type="checkbox" />
>                                 <span class="hint">A non priority hold doesn't prevent a current checkout from renewing</span>
>                             </li>
>-                            <fieldset>
>-                                <legend>
>-                                    <label for="requestany">
>-                                        Hold next available item
>-                                        [% IF force_hold_level == 'item' %]
>-                                            <input type="checkbox" id="requestany" name="request" disabled="true" />
>-                                        [% ELSIF force_hold_level == 'record' %]
>-                                            <input type="checkbox" id="requestany" checked="checked" value="Any" disabled="true"/>
>-                                            <input type="hidden" name="request" value="Any"/>
>-                                        [% ELSE %]
>-                                            <input type="checkbox" id="requestany" name="request" checked="checked" value="Any" />
>-                                        [% END %]
>-                                        <input type="hidden" name="alreadyreserved" value="[% alreadyreserved | html %]" />
>-                                    </label>
>-                                </legend>
>-                                <ol>
>-                                    <li>
>-                                        <label for="pickup">Pickup at:</label>
>-                                        <select name="pickup" id="pickup"
>-                                                data-biblio-id="[% biblio.biblionumber | html %]"
>-                                                data-patron-id="[% patron.borrowernumber | html %]"
>-                                                data-pickup-location-source="biblio">
>-                                            [% PROCESS options_for_libraries libraries => Branches.pickup_locations({ search_params => { biblio => biblionumber, patron => patron }, selected => pickup }) %]
>-                                        </select>
>-                                    </li>
>-                                    [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
>-                                        <li>
>-                                            <label for="itemtype">Request specific item type:</label>
>-                                            <select name="itemtype" id="itemtype">
>-                                                <option value="">Any item type</option>
>-                                                [%- FOREACH itemtype IN available_itemtypes %]
>-                                                    <option value="[% itemtype | html %]">[% ItemTypes.GetDescription( itemtype ) | html %]</option>
>-                                                [%- END %]
>-                                            </select>
>-                                        </li>
>-                                    [% END %]
>-                                    [% IF remaining_holds_for_record > 1 %]
>-                                        <li>
>-                                            <label for="holds_to_place_count">Holds to place (count)</label>
>-                                            <input type="text" inputmode="numeric" pattern="[0-9]*" id="holds_to_place_count" name="holds_to_place_count" maxlength="[% remaining_holds_for_record | html %]" value="1" />
>-                                        </li>
>-                                    [% ELSE %]
>-                                        <input type="hidden" name="holds_to_place_count" value="1" />
>-                                    [% END %]
>-                                </ol>
>-                            </fieldset>
>+                            <li>
>+                                <label for="holdnotes">Notes:</label>
>+                                <textarea id="holdnotes" name="notes" cols="30" rows="1"></textarea>
>+                            </li>
>+                        </ol>
>+                    </fieldset>
>+
>+                    <fieldset class="rows">
>+                        <legend>
>+                            <label for="requestany">
>+                                Hold next available item
>+                                [% IF force_hold_level == 'item' %]
>+                                    <input type="radio" class="requestany" id="requestany" name="request" disabled="true" />
>+                                [% ELSIF force_hold_level == 'record' %]
>+                                    <input type="radio" class="requestany" id="requestany" checked="checked" value="Any" disabled="true"/>
>+                                    <input type="hidden" name="request" value="Any"/>
>+                                [% ELSE %]
>+                                    <input type="radio" class="requestany" id="requestany" name="request" checked="checked" value="Any" />
>+                                [% END %]
>+                                <input type="hidden" name="alreadyreserved" value="[% alreadyreserved | html %]" />
>+                            </label>
>+                        </legend>
>+                        <ol>
>+                            <li>
>+                                <label for="pickup">Pickup at:</label>
>+                                <select name="pickup" id="pickup"
>+                                        data-biblio-id="[% biblio.biblionumber | html %]"
>+                                        data-patron-id="[% patron.borrowernumber | html %]"
>+                                        data-pickup-location-source="biblio">
>+                                    [% PROCESS options_for_libraries libraries => Branches.pickup_locations({ search_params => { biblio => biblionumber, patron => patron }, selected => pickup }) %]
>+                                </select>
>+                            </li>
>+                            [% IF Koha.Preference('AllowHoldItemTypeSelection') %]
>+                                <li>
>+                                    <label for="itemtype">Request specific item type:</label>
>+                                    <select name="itemtype" id="itemtype">
>+                                        <option value="">Any item type</option>
>+                                        [%- FOREACH itemtype IN available_itemtypes %]
>+                                            <option value="[% itemtype | html %]">[% ItemTypes.GetDescription( itemtype ) | html %]</option>
>+                                        [%- END %]
>+                                    </select>
>+                                </li>
>+                            [% END %]
>+                            [% IF remaining_holds_for_record > 1 %]
>+                                <li>
>+                                    <label for="holds_to_place_count">Holds to place (count)</label>
>+                                    <input type="text" inputmode="numeric" pattern="[0-9]*" id="holds_to_place_count" name="holds_to_place_count" maxlength="[% remaining_holds_for_record | html %]" value="1" />
>+                                </li>
>+                            [% ELSE %]
>+                                <input type="hidden" name="holds_to_place_count" value="1" />
>+                            [% END %]
>                         </ol>
>                         <fieldset class="action">
>                             [% IF ( patron.borrowernumber ) %]
>@@ -540,6 +541,20 @@
>                                 [% END %]
>                             [% END %]
>                         </fieldset>
>+                    </fieldset>
>+
>+                    <hr />
>+
>+                    <fieldset class="rows">
>+                        <legend>
>+                            <label>
>+                                Place a hold on a specific item
>+                                <input type="radio" name="request" class="requestany" />
>+                            </label>
>+                            [% IF force_hold_level == 'item' %]
>+                                <span class="error"><em>Required</em></span>
>+                            [% END %]
>+                        </legend>
> 
>                         [% biblio = biblioloop.0 %]
> 
>@@ -559,13 +574,6 @@
>                             [% END %]
>                         </ol>
> 
>-                        <h2 style="padding: 0 1em;">
>-                            Place a hold on a specific item
>-                            [% IF force_hold_level == 'item' %]
>-                                 <span class="error"><em>(Required)</em></span>
>-                            [% END %]
>-                        </h2>
>-
>                         <table id="requestspecific">
>                             <thead>
>                                 <tr>
>@@ -600,9 +608,9 @@
>                                                         Hold must be record level
>                                                     </span>
>                                                 [% ELSIF ( itemloo.available ) %]
>-                                                    <input type="radio" name="checkitem" value="[% itemloo.itemnumber | html %]" />
>+                                                    <input type="radio" name="checkitem" class="requestspecific" value="[% itemloo.itemnumber | html %]" />
>                                                 [% ELSIF ( itemloo.override ) %]
>-                                                    <input type="radio" name="checkitem" class="needsoverride" value="[% itemloo.itemnumber | html %]" />
>+                                                    <input type="radio" name="checkitem" class="needsoverride requestspecific" value="[% itemloo.itemnumber | html %]" />
>                                                     <i class="fa fa-exclamation-triangle fa-lg" style="color:gold" title="Requires override of hold policy"/></i>
>                                                 [% ELSE %]
>                                                     <span class="error">
>@@ -645,7 +653,7 @@
>                                             </td>
>                                             <td>
>                                             [% IF (itemloo.pickup_locations_count > 0) %]
>-                                                <select name="item_pickup_[% itemloo.itemnumber | html %]" class="pickup_locations" style="width:100%;"
>+                                                <select name="item_pickup_[% itemloo.itemnumber | html %]" class="pickup_locations requestspecific" style="width:100%;"
>                                                         data-item-id="[% itemloo.itemnumber | html %]"
>                                                         data-patron-id="[% patron.borrowernumber | html %]"
>                                                         data-pickup-location-source="item">
>@@ -751,11 +759,9 @@
>                         </table> 
> 
>                         [% IF hiddencount %]
>-                            <form>
>-                                <p class="hiddencount">
>-                                    <a href="request.pl?biblionumber=[% biblio.biblionumber | uri %]&amp;borrowernumber=[% borrowernumber | uri %]&amp;showallitems=1">Show all items ([% biblio.hiddencount | html %] hidden)</a>
>-                                </p>
>-                            </form>
>+                            <p class="hiddencount">
>+                                <a href="request.pl?biblionumber=[% biblio.biblionumber | uri %]&amp;borrowernumber=[% borrowernumber | uri %]&amp;showallitems=1">Show all items ([% biblio.hiddencount | html %] hidden)</a>
>+                            </p>
>                         [% END # /IF hiddencount %]
> 
>                             <fieldset class="action">
>@@ -769,8 +775,8 @@
>                                     [% END %]
>                                 [% END # /IF patron %]
>                             </fieldset> 
>-                        </form> 
>-                    </fieldset> 
>+                        </fieldset> 
>+                    </form> 
>                 [% ELSE # Multi-hold %]
>                     <fieldset class="rows">
>                         <legend>Hold details</legend>
>@@ -801,10 +807,6 @@
>                                 [% END %]
>                             </li>
> 
>-                            <li>
>-                                <label for="holdnotes">Notes:</label>
>-                                <textarea id="holdnotes" name="notes" cols="30" rows="1"></textarea>
>-                            </li>
>                             <li>
>                                 <label for="pickup">Pickup at:</label>
>                                 <select name="pickup" id="pickup_multi" data-patron-id="[% patron.borrowernumber | html %]">
>@@ -815,7 +817,6 @@
>                                 </select>
>                             </li>
> 
>-
>                             [% IF ( reserve_in_future ) %]
>                                 <li>
>                                     <label for="from">Hold starts on date:</label>
>@@ -834,6 +835,11 @@
>                                 <input name="non_priority" id="non_priority" type="checkbox" />
>                                 <span class="hint">A non priority hold doesn't prevent a current checkout from renewing</span>
>                             </li>
>+
>+                            <li>
>+                                <label for="holdnotes">Notes:</label>
>+                                <textarea id="holdnotes" name="notes" cols="30" rows="1"></textarea>
>+                            </li>
>                         </ol>
> 
> 
>@@ -941,10 +947,10 @@
>                                 [% END %]
>                             [% END # /IF patron %]
>                         </fieldset> 
>-                    </form> 
>-                </fieldset> 
>-                [% END %]
>-                [% END %]
>+                    </fieldset> 
>+                </form> 
>+                [% END # /UNLESS ( multi_hold ) %]
>+            [% END # /UNLESS club OR patron... %]
> 
>             [% UNLESS ( patron ) %]
>                 [% UNLESS borrowers %]
>@@ -1250,15 +1256,9 @@
>                     $(this).find("input.focus").focus();
>                 }
>             });
>-            function ToggleHoldsToPlace() {
>-                if ( $("#requestany").prop('checked') ) {
>-                    $("#holds_to_place_count,#pickup,#itemtype").prop('disabled', false);
>-                } else {
>-                    $("#holds_to_place_count,#pickup,#itemtype").prop('disabled', true);
>-                }
>-            }
>+
>             ToggleHoldsToPlace();
>-            $("#requestany").on('change', function(){
>+            $(".requestany").on('change', function(){
>                 ToggleHoldsToPlace();
>             });
> 
>@@ -1277,13 +1277,6 @@
>                 "sDom": '<"top pager"ilf>t',
>             }));
> 
>-            //Override fieldset styling for dataTables search box
>-            $("div.top.pager").css("margin-left","1em");
>-            $(".dataTables_filter label").css({
>-                "width":"auto",
>-                "margin-right":"0em"
>-            });
>-
>             $("#club-request-form").on("submit", function() {
>                 let $t = $(this);
>                 $('.clubalert, .holdalert').addClass('hide');
>@@ -1382,6 +1375,16 @@
>             });
>         });
> 
>+        function ToggleHoldsToPlace() {
>+            if ( $("#requestany").prop('checked') ) {
>+                $("#holds_to_place_count, #pickup, #itemtype").prop('disabled', false);
>+                $(".requestspecific").prop("disabled", true );
>+            } else {
>+                $("#holds_to_place_count, #pickup, #itemtype").prop('disabled', true);
>+                $(".requestspecific").prop("disabled", false );
>+            }
>+        }
>+
>         function check( table ) {
> 
>             var msg = "";
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 30579: 133549 | 133550 | 133574 | 133575 | 133689 | 133690 | 134270 | 134468 | 134469 | 134491 | 134492 | 134493 | 134893 | 134894 | 140951 | 140952 | 140953 | 140954 | 148835 | 148898 | 149271 | 150767 | 150786 | 151483 | 151493 | 151558 | 154881 | 154882 | 154883 | 154886 | 157787 | 157788 | 157789 | 157790 | 157791 | 157792 | 157937 | 157938 | 157939 | 157940 | 157941 | 157942 | 157943 | 163260 | 163261 | 163262 | 163263 | 163264 | 163265 | 163266 | 163267 | 166012 | 166013 | 166014 | 166015 | 166016 | 166017 | 166018 | 166019 | 166034 | 166035 | 166036 | 166037 | 166038 | 166039 | 166040 | 166041 | 166074 | 166075 | 166076 | 166077 | 166078 | 166079 | 166080 | 166081 | 166082 | 166264 | 166265 | 166266 | 166267 | 166268 | 166269 | 166270 | 166271 | 166272 | 166273 | 166274 | 166275