From 4059d60e143993003120f3670a0e4c4a814ab326 Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Wed, 28 Jul 2021 19:27:34 +0000
Subject: [PATCH] Bug 14783: (follow-up) Move pickup library form into modal

This patch attemps to simplify the list of holds by moving the form for
changing a hold's pickup library into a modal dialog.

This patch also makes minor tweaks to the CSS for the "X" close button
shown in modal in windows. Instances where modal markup used "x" have
been updated to use the Bootstrap 4 standard "&times;"

To test, apply the patch and rebuild the OPAC
SCSS
(https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client)

- Turn on the OPACAllowUserToChangeBranch system preference.
- Log in to the OPAC as a user with holds.
- In the view of your holds, confirm that each hold which can be
  modified shows a "Change" link.
- Clicking the link should show a modal window with the title of the
  item on hold shown in the header.
- Confirm that changing the pickup location works correctly.
---
 .../opac-tmpl/bootstrap/css/src/_common.scss  | 28 ++++++------
 .../bootstrap/en/includes/holds-table.inc     | 43 ++++++++++++++-----
 .../en/includes/overdrive-checkout.inc        |  2 +-
 .../bootstrap/en/modules/opac-user.tt         |  6 +++
 koha-tmpl/opac-tmpl/bootstrap/js/global.js    |  2 +-
 5 files changed, 55 insertions(+), 26 deletions(-)

diff --git a/koha-tmpl/opac-tmpl/bootstrap/css/src/_common.scss b/koha-tmpl/opac-tmpl/bootstrap/css/src/_common.scss
index 1d0c0a9419..285ad13248 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/css/src/_common.scss
+++ b/koha-tmpl/opac-tmpl/bootstrap/css/src/_common.scss
@@ -941,35 +941,35 @@ ul {
 /* Redefine a new style for Bootstrap's class "close" since we use that already */
 /* Use <a class="closebtn" href="#">&times;</a> */
 
-.modal-header .closebtn {
-    margin-top: 2px;
-}
-
 .closebtn {
+    background: transparent;
+    border: 0;
     color: #000000;
+    cursor: pointer;
     float: right;
-    font-size: 20px;
+    font-size: 1.5rem;
     font-weight: bold;
-    line-height: 20px;
-    opacity: .2;
+    line-height: 1;
+    opacity: .5;
+    padding: 1rem 1rem;
+    margin: -1rem -1rem -1rem auto;
     text-shadow: 0 1px 0 #FFFFFF;
 
     &:hover {
         color: #000000;
         cursor: pointer;
-        opacity: .4;
+        opacity: .7;
         text-decoration: none;
     }
 }
 
-button {
-    &.closebtn {
-        background: transparent;
-        border: 0;
-        cursor: pointer;
-        padding: 0;
+.ui-widget {
+    .closebtn {
+        font-size: 1.5rem;
     }
+}
 
+button {
     &.remove {
         &:hover {
             color: #900;
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
index 3ff4b50b77..5694461b46 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/holds-table.inc
@@ -84,16 +84,39 @@
                         [% UNLESS( singleBranchMode) %]
                             <td class="branch">
                                 <span class="tdlabel">Pick up location:</span>
+                                [% HOLD.branch.branchname | html %]
                                 [% IF ( HOLD.can_change_branch_opac ) %]
-                                    <form class="change_branch" action="/cgi-bin/koha/opac-modrequest.pl" method="post">
-                                        <select name="new_branch" >
-                                            [% PROCESS options_for_libraries libraries = Branches.pickup_locations({    search_params => { biblio => HOLD.biblionumber, patron => HOLD.borrower }, selected => HOLD.branchcode }) %]
-                                       </select>
-                                       <input type="hidden" name="new_branch_reserveid" value="[% HOLD.reserve_id | html %]" />
-                                       <button class="btn btn-primary" type="submit" name="change_branch" value="1">Change</button>
-                                    </form>
-                                [% ELSE %]
-                                    [% HOLD.branch.branchname | html %]
+                                    <button type="button" class="btn btn-sm btn-link" data-toggle="modal" data-target="#changePickup[% HOLD.reserve_id | html %]">
+                                        <i class="fa fa-pencil" aria-hidden="true"></i> Change
+                                    </button>
+                                    <!-- Change pickup location modal -->
+                                    <div class="modal" id="changePickup[% HOLD.reserve_id | html %]" tabindex="-1" aria-labelledby="changePickup[% HOLD.reserve_id | html %]Label" aria-hidden="true">
+                                        <div class="modal-dialog">
+                                            <form id="change_branch_form[% HOLD.reserve_id | html %]" action="/cgi-bin/koha/opac-modrequest.pl" method="post">
+                                                <div class="modal-content">
+                                                    <div class="modal-header">
+                                                        <h5 class="modal-title" id="changePickup[% HOLD.reserve_id | html %]Label">Change pickup location for <em>[% INCLUDE 'biblio-title.inc' biblio=HOLD.biblio %]</em></h5>
+                                                        <button type="button" class="closebtn" data-dismiss="modal" aria-label="Close">
+                                                            <span aria-hidden="true">&times;</span>
+                                                        </button>
+                                                    </div>
+                                                    <div class="modal-body">
+                                                        <div class="form-group">
+                                                        <label for="new_branch[% HOLD.reserve_id | html %]">New pickup location:</label>
+                                                        <select name="new_branch" id="new_branch[% HOLD.reserve_id | html %]" class="form-control">
+                                                            [% PROCESS options_for_libraries libraries = Branches.pickup_locations({ search_params => { biblio => HOLD.biblionumber, patron => HOLD.borrower }, selected => HOLD.branchcode }) %]
+                                                        </select>
+                                                        </div>
+                                                        <input type="hidden" name="new_branch_reserveid" value="[% HOLD.reserve_id | html %]" />
+                                                    </div>
+                                                    <div class="modal-footer">
+                                                        <button type="submit" name="change_branch" value="1" class="btn btn-primary"><i class="fa fa-check" aria-hidden="true"></i> Save</button>
+                                                        <button type="button" class="btn btn-secondary" data-dismiss="modal"><i class="fa fa-remove" aria-hidden="true"></i> Cancel</button>
+                                                    </div>
+                                                </div> <!-- /.modal-content -->
+                                            </form>
+                                        </div> <!-- /.modal-dialog -->
+                                    </div> <!-- /.modal -->
                                 [% END  %]
                             </td>
                         [% END %]
@@ -161,7 +184,7 @@
                                                                     <h3 class="modal-title" id="suspendModal[% HOLD.reserve_id | html %]Label">Suspend your hold on <em>[% HOLD.biblio.title | html %]</em></h3>
                                                                 [% END %]
                                                                 <button type="button" class="closebtn" data-dismiss="modal" aria-label="Close">
-                                                                    <span aria-hidden="true">x</span>
+                                                                    <span aria-hidden="true">&times;</span>
                                                                 </button>
                                                             </div>
                                                             <div class="modal-body">
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/includes/overdrive-checkout.inc b/koha-tmpl/opac-tmpl/bootstrap/en/includes/overdrive-checkout.inc
index fc5766998c..5350b42a33 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/includes/overdrive-checkout.inc
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/includes/overdrive-checkout.inc
@@ -1,6 +1,6 @@
 <div id="overdrive-checkout" class="modal" tabindex="-1" role="dialog" aria-labelledby="overdrive-checkout-label" aria-hidden="true">
     <div class="modal-header">
-        <button type="button" class="closebtn" data-dismiss="modal" aria-hidden="true">×</button>
+        <button type="button" class="closebtn" data-dismiss="modal" aria-hidden="true">&times;</button>
         <h3 id="overdrive-checkout-label">Checkout</h3>
     </div>
     <form action="#" method="post" id="overdrive-checkout-form">
diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
index 7c92ba2f9e..82d0b1205d 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
@@ -1051,6 +1051,12 @@
                 minDate: 1
             });
 
+            $(".change_pickup").on("click", function(){
+                var hold_id = $(this).data("hold-id");
+                $(this).hide();
+                $("#change-pickup-location" + hold_id ).show();
+            });
+
             if ( $('#opac-user-clubs').length ) {
                 $('#opac-user-clubs-tab-link').on('click', function() {
                     $('#opac-user-clubs').text(_("Loading..."));
diff --git a/koha-tmpl/opac-tmpl/bootstrap/js/global.js b/koha-tmpl/opac-tmpl/bootstrap/js/global.js
index 27f2874502..6602f13364 100644
--- a/koha-tmpl/opac-tmpl/bootstrap/js/global.js
+++ b/koha-tmpl/opac-tmpl/bootstrap/js/global.js
@@ -84,7 +84,7 @@ function confirmModal(message, title, yes_label, no_label, callback) {
                     <div class="modal-header" style="min-height:40px;">\
                         <h4 class="modal-title"></h4>\
                         <button type="button" class="closebtn" data-dismiss="modal" aria-label="Close">\
-                        <span aria-hidden="true">×</span>\
+                        <span aria-hidden="true">&times;</span>\
                     </button>\
                     </div>\
                     <div class="modal-body"><p></p></div>\
-- 
2.20.1