Bug 35068 - Split 'Renew or check in selected items' button in issues table into separate buttons
Summary: Split 'Renew or check in selected items' button in issues table into separate...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Lucas Gass
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks: 36060 35251
  Show dependency treegraph
 
Reported: 2023-10-16 22:19 UTC by Lucas Gass
Modified: 2024-02-28 22:54 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact: Caroline Cyr La Rose
Documentation submission: https://gitlab.com/koha-community/koha-manual/-/merge_requests/816
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons (5.82 KB, patch)
2023-10-16 22:30 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35068: (follow-up) Disable button after clicking Renew all (882 bytes, patch)
2023-10-17 14:52 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35068: (follow-up) Always disable buttons after action (1.43 KB, patch)
2023-10-18 20:05 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons (5.88 KB, patch)
2023-10-19 14:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35068: (follow-up) Disable button after clicking Renew all (930 bytes, patch)
2023-10-19 14:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35068: (follow-up) Always disable buttons after action (1.48 KB, patch)
2023-10-19 14:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35068: (follow-up) Update button order and icons (2.04 KB, patch)
2023-10-19 14:52 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons (5.93 KB, patch)
2023-10-20 14:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35068: (follow-up) Disable button after clicking Renew all (980 bytes, patch)
2023-10-20 14:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35068: (follow-up) Always disable buttons after action (1.53 KB, patch)
2023-10-20 14:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35068: (follow-up) Update button order and icons (2.09 KB, patch)
2023-10-20 14:34 UTC, ByWater Sandboxes
Details | Diff | Splinter Review
Bug 35068: (follow-up) Update button order and icons (2.14 KB, patch)
2023-10-20 15:10 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload (2.35 KB, patch)
2023-11-03 15:27 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons (5.99 KB, patch)
2023-11-05 19:30 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up) Disable button after clicking Renew all (1.01 KB, patch)
2023-11-05 19:30 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up) Always disable buttons after action (1.58 KB, patch)
2023-11-05 19:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up) Update button order and icons (2.20 KB, patch)
2023-11-05 19:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload (2.41 KB, patch)
2023-11-05 19:31 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons (6.06 KB, patch)
2023-11-05 19:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up) Disable button after clicking Renew all (1.01 KB, patch)
2023-11-05 19:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up) Always disable buttons after action (1.58 KB, patch)
2023-11-05 19:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up) Update button order and icons (2.20 KB, patch)
2023-11-05 19:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload (2.41 KB, patch)
2023-11-05 19:40 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons (6.15 KB, patch)
2023-11-10 08:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35068: (follow-up) Disable button after clicking Renew all (1.11 KB, patch)
2023-11-10 08:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35068: (follow-up) Always disable buttons after action (1.68 KB, patch)
2023-11-10 08:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35068: (follow-up) Update button order and icons (2.29 KB, patch)
2023-11-10 08:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload (2.50 KB, patch)
2023-11-10 08:56 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass 2023-10-16 22:19:05 UTC
Many libraries find 'Renew or check in selected items' within the issues/checkout table confusing. 

It makes sense to split these into 2 separate buttons.
Comment 1 Lucas Gass 2023-10-16 22:30:42 UTC
Created attachment 157218 [details] [review]
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin selected items' button.
5. Play around with each making sure that they work as excepted when checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items' button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew' and 'Checkin' columns. Make sure the 'Renew selected items' and 'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.
Comment 2 Owen Leonard 2023-10-17 11:18:07 UTC
I really like this and it works well except for one issue I've found: After clicking "Renew all" the "Renew selected" button becomes active even though there are no checked checkboxes.
Comment 3 Lucas Gass 2023-10-17 14:52:44 UTC
Created attachment 157254 [details] [review]
Bug 35068: (follow-up) Disable button after clicking Renew all
Comment 4 Lucas Gass 2023-10-17 14:53:14 UTC
(In reply to Owen Leonard from comment #2)
> I really like this and it works well except for one issue I've found: After
> clicking "Renew all" the "Renew selected" button becomes active even though
> there are no checked checkboxes.

Owen, thanks for testing! I have provided a small follow-up and reset to NSO.
Comment 5 Brendan Lawlor 2023-10-18 18:42:03 UTC
I tested this and it works well, except I found one edge case. If you make selections in both the renew and checkin columns and then click one of the buttons to renew or checkin the selected items, when circulation.pl reloads both buttons start as enabled, but they should be disabled like when circulation.pl initially loads.
Comment 6 Lucas Gass 2023-10-18 20:05:18 UTC
Created attachment 157357 [details] [review]
Bug 35068: (follow-up) Always disable buttons after action
Comment 7 Lucas Gass 2023-10-18 20:05:42 UTC
(In reply to Brendan Lawlor from comment #5)
> I tested this and it works well, except I found one edge case. If you make
> selections in both the renew and checkin columns and then click one of the
> buttons to renew or checkin the selected items, when circulation.pl reloads
> both buttons start as enabled, but they should be disabled like when
> circulation.pl initially loads.

Good catch! I've provided a follow-up.
Comment 8 Owen Leonard 2023-10-19 14:52:37 UTC Comment hidden (obsolete)
Comment 9 Owen Leonard 2023-10-19 14:52:39 UTC Comment hidden (obsolete)
Comment 10 Owen Leonard 2023-10-19 14:52:41 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2023-10-19 14:52:43 UTC Comment hidden (obsolete)
Comment 12 ByWater Sandboxes 2023-10-20 14:34:49 UTC
Created attachment 157523 [details] [review]
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 13 ByWater Sandboxes 2023-10-20 14:34:50 UTC
Created attachment 157524 [details] [review]
Bug 35068: (follow-up) Disable button after clicking Renew all

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 14 ByWater Sandboxes 2023-10-20 14:34:52 UTC
Created attachment 157525 [details] [review]
Bug 35068: (follow-up) Always disable buttons after action

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Comment 15 ByWater Sandboxes 2023-10-20 14:34:54 UTC Comment hidden (obsolete)
Comment 16 Brendan Lawlor 2023-10-20 15:10:21 UTC
Created attachment 157552 [details] [review]
Bug 35068: (follow-up) Update button order and icons

This patch updates the Renew and Check-in buttons so that they have
icons matching the ones used for the Renew and Check-in search tabs in
the header.

The patch also moves the Renew button so that it is next to the Renew
all button. I think this makes more logical sense. So the order is:

- Check in selected items
- Renew selected items
- Renew all

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 17 Lucas Gass 2023-10-23 20:27:35 UTC
Owen, thanks for the follow-up. Those icons look good!
Comment 18 Marcel de Rooy 2023-11-03 08:28:48 UTC
Looks good, but wondering about:

Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
The item is checked in. But the tab still lists two checkouts (wrong).
And the selection for renew is gone. Uncomfortable? Suppose I had a large selection?
Comment 19 Owen Leonard 2023-11-03 10:51:06 UTC
(In reply to Marcel de Rooy from comment #18)
> The item is checked in. But the tab still lists two checkouts (wrong).

This is not related to this patch. That's the existing behavior.

> And the selection for renew is gone. Uncomfortable? Suppose I had a large
> selection?

I agree that this is a problem
Comment 20 Lucas Gass 2023-11-03 15:23:32 UTC
> And the selection for renew is gone. Uncomfortable? Suppose I had a large
> selection?

Good catch, I think I have a solution.
Comment 21 Lucas Gass 2023-11-03 15:27:46 UTC
Created attachment 158343 [details] [review]
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload

To test:
1. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
2. When the table reloads the 'renew' selections should be retained
3. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
4. When the table reloads the 'check in' selections should be retained
Comment 22 Lucas Gass 2023-11-03 15:28:50 UTC
I think it is now prudent here to get another sign-off so I have reset to NSO.
Comment 23 Lucas Gass 2023-11-03 15:34:22 UTC
> The item is checked in. But the tab still lists two checkouts (wrong).

I have filed a separate bug for this issue since it happens with or without my patch.
Comment 24 Victor Grousset/tuxayo 2023-11-05 19:30:51 UTC Comment hidden (obsolete)
Comment 25 Victor Grousset/tuxayo 2023-11-05 19:30:57 UTC Comment hidden (obsolete)
Comment 26 Victor Grousset/tuxayo 2023-11-05 19:31:02 UTC Comment hidden (obsolete)
Comment 27 Victor Grousset/tuxayo 2023-11-05 19:31:07 UTC Comment hidden (obsolete)
Comment 28 Victor Grousset/tuxayo 2023-11-05 19:31:11 UTC Comment hidden (obsolete)
Comment 29 Victor Grousset/tuxayo 2023-11-05 19:40:42 UTC
Created attachment 158481 [details] [review]
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.
11. Do the test plan of 5th patch "Make sure renew/checkin selections"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 30 Victor Grousset/tuxayo 2023-11-05 19:40:45 UTC
Created attachment 158482 [details] [review]
Bug 35068: (follow-up) Disable button after clicking Renew all

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 31 Victor Grousset/tuxayo 2023-11-05 19:40:49 UTC
Created attachment 158483 [details] [review]
Bug 35068: (follow-up) Always disable buttons after action

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 32 Victor Grousset/tuxayo 2023-11-05 19:40:52 UTC
Created attachment 158484 [details] [review]
Bug 35068: (follow-up) Update button order and icons

This patch updates the Renew and Check-in buttons so that they have
icons matching the ones used for the Renew and Check-in search tabs in
the header.

The patch also moves the Renew button so that it is next to the Renew
all button. I think this makes more logical sense. So the order is:

- Check in selected items
- Renew selected items
- Renew all

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 33 Victor Grousset/tuxayo 2023-11-05 19:40:55 UTC
Created attachment 158485 [details] [review]
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload

To test:
1. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
2. When the table reloads the 'renew' selections should be retained
3. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
4. When the table reloads the 'check in' selections should be retained

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 34 Victor Grousset/tuxayo 2023-11-05 19:50:20 UTC
Amended test plan to mention the 2nd test plan. It's not reliable to have other test plans in follow-up without having the 1st test plan point to them. I hope I never missed such cases in the past >_<

Anyway, it works! :D

And the functional chance makes sense. I was always confused about that 2 in one button.
Comment 35 Martin Renvoize 2023-11-08 14:11:38 UTC
I'd be tempted to tie the buttons even more directly to the select columns they adhere to..  perhaps a second footer row for actions buttons that pertain to the column above them?
Comment 36 Marcel de Rooy 2023-11-10 08:36:37 UTC
Looking here
Comment 37 Marcel de Rooy 2023-11-10 08:55:21 UTC
(In reply to Martin Renvoize from comment #35)
> I'd be tempted to tie the buttons even more directly to the select columns
> they adhere to..  perhaps a second footer row for actions buttons that
> pertain to the column above them?

I dont think that we should block this now. But personally the buttons and checkboxes are a bit confusing. If I click one renew checkbox, I would expect Renew all to be disabled for instance.
Renew all is now a shortcut for Select all+Renew selected. Could we do without it? We dont have Checkin all which is just Select all+Checkin selected. Feels like an inconsistency?

What about (just brainstorming, sorry): Checkin = Checkin All when no checkbox is enabled. Same for Renew. As soon as you select a checkbox, the button becomes an action for selected lines.
Comment 38 Marcel de Rooy 2023-11-10 08:56:42 UTC
Created attachment 158788 [details] [review]
Bug 35068: Split RenewCheckinChecked button into seperate renew/checkin buttons

To test:
1. APPLY patch, clear browser's cache
2. Have more than 1 checkout issued to a borrower.
3. Load the checkout table on circ/circulation.pl
4. Notice that there is now 'Renew selected items' and 'Checkin
   selected items' button.
5. Play around with each making sure that they work as excepted when
   checkouts are checked in either the 'Check in' or 'Renew' columns.
6. Uncheck everything in the 'Renew column', the 'Renew selected items'
   button should be disabled.
7. Uncheck everything in the 'Checkin column', the 'Checkin selected
   items' button should be disabled.
8. Try unchecking some of these to make sure the Renew/Checkin button is
   properly disabled when no related checkboxes are checked.
9. Try using the select all/none options at the top of both the 'Renew'
   and 'Checkin' columns. Make sure the 'Renew selected items' and
   'Checkin selected items' become properly disabled/enabled.
10. Make sure the 'Renew all' button still works properly.
11. Do the test plan of 5th patch "Make sure renew/checkin selections"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 39 Marcel de Rooy 2023-11-10 08:56:45 UTC
Created attachment 158789 [details] [review]
Bug 35068: (follow-up) Disable button after clicking Renew all

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 40 Marcel de Rooy 2023-11-10 08:56:47 UTC
Created attachment 158790 [details] [review]
Bug 35068: (follow-up) Always disable buttons after action

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 41 Marcel de Rooy 2023-11-10 08:56:50 UTC
Created attachment 158791 [details] [review]
Bug 35068: (follow-up) Update button order and icons

This patch updates the Renew and Check-in buttons so that they have
icons matching the ones used for the Renew and Check-in search tabs in
the header.

The patch also moves the Renew button so that it is next to the Renew
all button. I think this makes more logical sense. So the order is:

- Check in selected items
- Renew selected items
- Renew all

Signed-off-by: Kelly <kelly@bywatersolutions.com>
Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 42 Marcel de Rooy 2023-11-10 08:56:53 UTC
Created attachment 158792 [details] [review]
Bug 35068: (follow-up): Make sure renew/checkin selections get retained on reload

To test:
1. Have two checkouts. Click one for Renew and one for Checkin. Click on Check in selected items.
2. When the table reloads the 'renew' selections should be retained
3. Have two checkouts. Click one for Renew and one for Checkin. Click on Renew selected items.
4. When the table reloads the 'check in' selections should be retained

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 43 Tomás Cohen Arazi 2023-11-10 14:02:29 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 44 Fridolin Somers 2023-11-11 07:50:03 UTC
Enhancement not pushed to 23.05.x
Comment 45 Martin Renvoize 2024-02-28 22:54:45 UTC
(In reply to Marcel de Rooy from comment #37)
> (In reply to Martin Renvoize from comment #35)
> > I'd be tempted to tie the buttons even more directly to the select columns
> > they adhere to..  perhaps a second footer row for actions buttons that
> > pertain to the column above them?
> 
> I dont think that we should block this now. But personally the buttons and
> checkboxes are a bit confusing. If I click one renew checkbox, I would
> expect Renew all to be disabled for instance.
> Renew all is now a shortcut for Select all+Renew selected. Could we do
> without it? We dont have Checkin all which is just Select all+Checkin
> selected. Feels like an inconsistency?
> 
> What about (just brainstorming, sorry): Checkin = Checkin All when no
> checkbox is enabled. Same for Renew. As soon as you select a checkbox, the
> button becomes an action for selected lines.

Did we open a new bug for this?