Bug 37309

Summary: Improve 'delete' and 'modify' links for items on the bibliographic detail page
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Staff interfaceAssignee: Jonathan Druart <jonathan.druart>
Status: RESOLVED FIXED QA Contact: Pedro Amorim <pedro.amorim>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, kebliss, lucas, martin.renvoize, oleonard, pedro.amorim, phil
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37728
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
This enhancement improves the way batch operation controls are dynamically generated when the user checks one or more checkboxes under the holdings tab on the bibliographic detail page. The markup has also been updated in order to improve consistency in the way the controls are styled.
Version(s) released in:
24.11.00
Circulation function:
Bug Depends on:    
Bug Blocks: 20411    
Attachments: Bug 37309: Improve Delete and Modify items links on the biblio detail page
Bug 37309: Improve Delete and Modify items links on the biblio detail page
Bug 37309: (follow-up) Adjust appearance of controls
Bug 37309: Fix behaviour when SeparateHoldings is in use
Bug 37309: Clear itemnumbers array before 'select all'
Bug 37309: Clear itemnumbers array before 'select all'
Bug 37309: Improve Delete and Modify items links on the biblio detail page
Bug 37309: (follow-up) Adjust appearance of controls
Bug 37309: Fix behaviour when SeparateHoldings is in use
Bug 37309: Clear itemnumbers array before 'select all'
Bug 37309: Clear itemnumbers array before 'select all'
Bug 37309: Adjustment for 33526
Bug 37309: Adjust test

Description Jonathan Druart 2024-07-10 15:12:36 UTC
The way the "Delete selected items" and "Modify selected items" links are build on the bibliographic detail page is not nice. We could do better, for instance using a proper form element instead of building an URL.
Comment 1 Jonathan Druart 2024-07-11 08:05:59 UTC
Created attachment 168777 [details] [review]
Bug 37309: Improve Delete and Modify items links on the biblio detail page

The way the "Delete selected items" and "Modify selected items" links are build
on the bibliographic detail page is not nice. We could do better, for instance
using a proper form element instead of building an URL.

Test plan:
Go to the detail page of a bibliographic record which has several items.
In the items table check some checkboxes and play with "select all",
"clear all" and the action buttons.
Notice that everything works as before.

Minor UI regression that must be fixed: there are CSS rules that cause
problem. When the action links are displayed or hidden the table is
moving up and down.
I think the problematic rules are:
  .btn {
    padding: 6px 12px;
    font-size: 12px;
  }
It makes the UI better if I remove them but still get the bumping
issue.
Comment 2 Jonathan Druart 2024-07-11 08:06:36 UTC
Owen, can I get some help to fix the UI glitches? See the commit message.
Comment 3 Owen Leonard 2024-07-11 12:20:38 UTC
Created attachment 168797 [details] [review]
Bug 37309: Improve Delete and Modify items links on the biblio detail page

The way the "Delete selected items" and "Modify selected items" links
are build on the bibliographic detail page is not nice. We could do
better, for instance using a proper form element instead of building an
URL.

Test plan:
Go to the detail page of a bibliographic record which has several items.
In the items table check some checkboxes and play with "select all",
"clear all" and the action buttons.
Notice that everything works as before.

Minor UI regression that must be fixed: there are CSS rules that cause
problem. When the action links are displayed or hidden the table is
moving up and down.
I think the problematic rules are:
  .btn {
    padding: 6px 12px;
    font-size: 12px;
  }
It makes the UI better if I remove them but still get the bumping
issue.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Owen Leonard 2024-07-11 12:20:41 UTC
Created attachment 168798 [details] [review]
Bug 37309: (follow-up) Adjust appearance of controls

This patch makes an adjustment to the style of .btn-link controls
associated with the holdings table. I've also updated the markup of the
other controls so that they match the updated batch controls.
Comment 5 Jonathan Druart 2024-07-11 12:38:00 UTC
Thanks a lot, Owen. I found some bugs retesting this however.
Comment 6 Jonathan Druart 2024-07-11 12:42:07 UTC
Created attachment 168799 [details] [review]
Bug 37309: Fix behaviour when SeparateHoldings is in use
Comment 7 Jonathan Druart 2024-07-11 12:48:18 UTC
Created attachment 168802 [details] [review]
Bug 37309: Clear itemnumbers array before 'select all'

Otherwise the same itemnumber can appear several times in the array.
It could be moved to its own bug but this is only code cleaning it
seems, there is no impact on the UX
Comment 8 Pedro Amorim 2024-07-11 16:43:00 UTC
Created attachment 168844 [details] [review]
Bug 37309: Clear itemnumbers array before 'select all'

Otherwise the same itemnumber can appear several times in the array.
It could be moved to its own bug but this is only code cleaning it
seems, there is no impact on the UX

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 9 Pedro Amorim 2024-07-11 16:43:16 UTC
Created attachment 168845 [details] [review]
Bug 37309: Improve Delete and Modify items links on the biblio detail page

The way the "Delete selected items" and "Modify selected items" links
are build on the bibliographic detail page is not nice. We could do
better, for instance using a proper form element instead of building an
URL.

Test plan:
Go to the detail page of a bibliographic record which has several items.
In the items table check some checkboxes and play with "select all",
"clear all" and the action buttons.
Notice that everything works as before.

Minor UI regression that must be fixed: there are CSS rules that cause
problem. When the action links are displayed or hidden the table is
moving up and down.
I think the problematic rules are:
  .btn {
    padding: 6px 12px;
    font-size: 12px;
  }
It makes the UI better if I remove them but still get the bumping
issue.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 10 Pedro Amorim 2024-07-11 16:43:19 UTC
Created attachment 168846 [details] [review]
Bug 37309: (follow-up) Adjust appearance of controls

This patch makes an adjustment to the style of .btn-link controls
associated with the holdings table. I've also updated the markup of the
other controls so that they match the updated batch controls.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 11 Pedro Amorim 2024-07-11 16:43:22 UTC
Created attachment 168847 [details] [review]
Bug 37309: Fix behaviour when SeparateHoldings is in use

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 12 Pedro Amorim 2024-07-11 16:43:24 UTC
Created attachment 168848 [details] [review]
Bug 37309: Clear itemnumbers array before 'select all'

Otherwise the same itemnumber can appear several times in the array.
It could be moved to its own bug but this is only code cleaning it
seems, there is no impact on the UX

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 13 Jonathan Druart 2024-08-14 08:28:56 UTC
Created attachment 170307 [details] [review]
Bug 37309: Clear itemnumbers array before 'select all'

Otherwise the same itemnumber can appear several times in the array.
It could be moved to its own bug but this is only code cleaning it
seems, there is no impact on the UX

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 14 Jonathan Druart 2024-08-14 08:29:06 UTC
Fixed conflict.
Comment 15 Jonathan Druart 2024-08-14 09:13:06 UTC
Created attachment 170309 [details] [review]
Bug 37309: Adjustment for 33526
Comment 16 Jonathan Druart 2024-08-14 09:22:36 UTC
Passing QA, we have Owen and Pedro stamps already. Also it will prevent further conflicts with bug 20411.
Comment 17 Katrin Fischer 2024-08-26 08:23:40 UTC
It looks like this makes the tests fail, I am not sure why yet, but can you please check:

prove -v xt/find-missing-op-in-forms.t
xt/find-missing-op-in-forms.t .. 
1..1
not ok 1 - The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478)

#   Failed test 'The <form> in the following files are missing it's corresponding op parameter, or op does not start with 'cud-' (see bug 34478)'
#   at xt/find-missing-op-in-forms.t line 43.
#          got: '1'
#     expected: '0'
# $VAR1 = 'koha-tmpl/intranet-tmpl/prog/en/includes/html_helpers/tables/items/catalogue_detail.inc:59,69';
# Looks like you failed 1 test of 1.
Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/1 subtests
Comment 18 Jonathan Druart 2024-08-26 08:59:36 UTC
Created attachment 170700 [details] [review]
Bug 37309: Adjust test

The test to catch wrong "op" in posted form was not supporting single
quotes.
Comment 19 Katrin Fischer 2024-08-26 15:43:48 UTC
Pushed for 24.11!

Well done everyone, thank you!