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.

Owen, can I get some help to fix the UI glitches? See the commit message.

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>

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.

Thanks a lot, Owen. I found some bugs retesting this however.

Created attachment 168799 [details] [review]
Bug 37309: Fix behaviour when SeparateHoldings is in use

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

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>

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>

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>

Created attachment 168847 [details] [review]
Bug 37309: Fix behaviour when SeparateHoldings is in use

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>

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>

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>

Fixed conflict.

Created attachment 170309 [details] [review]
Bug 37309: Adjustment for 33526

Passing QA, we have Owen and Pedro stamps already. Also it will prevent further conflicts with bug 20411.

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

Created attachment 170700 [details] [review]
Bug 37309: Adjust test

The test to catch wrong "op" in posted form was not supporting single
quotes.

Pushed for 24.11!

Well done everyone, thank you!