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.
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!