|
Description
Nick Clemens (kidclamp)
2022-04-21 12:32:05 UTC
Created attachment 133549 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch simply groups the forms so that multi-hold and single hold are each in their own section While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Place a single bib and multi bib hold in staff client 2 - Apply this patch 3 - Repeat and confirm nothing changes with this patch Created attachment 133550 [details] [review] Bug 30579: Group next available hold options, and disable when item is chosen This patch rearranges options that only apply to next available holds and disables them when an item is checked. This is intended to make it more obvious which parts of the form apply to this selection To test: 1 - Apply patches 2 - Confirm you can successfully place single or multiple items on hold 3 - Confirm that when placing a single hold the next available options are grouped 4 - Confirm pickup location, specific item type, and number of holds all work for next available 5 - Confirm item level holds save pickup location correctly *** Bug 30564 has been marked as a duplicate of this bug. *** Created attachment 133574 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch simply groups the forms so that multi-hold and single hold are each in their own section While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Place a single bib and multi bib hold in staff client 2 - Apply this patch 3 - Repeat and confirm nothing changes with this patch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 133575 [details] [review] Bug 30579: Group next available hold options, and disable when item is chosen This patch rearranges options that only apply to next available holds and disables them when an item is checked. This is intended to make it more obvious which parts of the form apply to this selection To test: 1 - Apply patches 2 - Confirm you can successfully place single or multiple items on hold 3 - Confirm that when placing a single hold the next available options are grouped 4 - Confirm pickup location, specific item type, and number of holds all work for next available 5 - Confirm item level holds save pickup location correctly Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 133689 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch simply groups the forms so that multi-hold and single hold are each in their own section While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Place a single bib and multi bib hold in staff client 2 - Apply this patch 3 - Repeat and confirm nothing changes with this patch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Created attachment 133690 [details] [review] Bug 30579: Group next available hold options, and disable when item is chosen This patch rearranges options that only apply to next available holds and disables them when an item is checked. This is intended to make it more obvious which parts of the form apply to this selection To test: 1 - Apply patches 2 - Confirm you can successfully place single or multiple items on hold 3 - Confirm that when placing a single hold the next available options are grouped 4 - Confirm pickup location, specific item type, and number of holds all work for next available 5 - Confirm item level holds save pickup location correctly Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Created attachment 134270 [details]
Screenshot of place hold page with patch applied
I am sorry, but I think this doesn't look quite right yet.
I am not sure using the label here is the right element for grouping. I think it would be a completely new pattern and it might be much nicer to stick with something we already use. - Although I have a hard time to come up with something other than having tabs for item specific and next available right now.
Also: for languages other than English that have longer translations this label will get really "high", I think some CSS change is at least in order.
In general I think it would be nice to make the label part of the page broader here as a lot of labels already have longer descriptions and break on multiple lines (unrelated... but wouldn't mind having it here)
Created attachment 134468 [details]
Suggested update
This is the update I'm trying out.
Created attachment 134469 [details] [review] Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer This patch alters the structure of the hold for a little more in order to make it clear that "Hold next available" and "Hold specific item" are two mutually-exclusive options. To test, apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Perform the same tests as before: - Confirm you can successfully place single or multiple items on hold - Confirm that when placing a single hold the next available options are grouped - Confirm pickup location, specific item type, and number of holds all work for next available - Confirm item level holds save pickup location correctly Created attachment 134491 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch simply groups the forms so that multi-hold and single hold are each in their own section While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Place a single bib and multi bib hold in staff client 2 - Apply this patch 3 - Repeat and confirm nothing changes with this patch Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Created attachment 134492 [details] [review] Bug 30579: Group next available hold options, and disable when item is chosen This patch rearranges options that only apply to next available holds and disables them when an item is checked. This is intended to make it more obvious which parts of the form apply to this selection To test: 1 - Apply patches 2 - Confirm you can successfully place single or multiple items on hold 3 - Confirm that when placing a single hold the next available options are grouped 4 - Confirm pickup location, specific item type, and number of holds all work for next available 5 - Confirm item level holds save pickup location correctly Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Rebecca Coert <rcoert@arlingtonva.us> Created attachment 134493 [details] [review] Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer This patch alters the structure of the hold for a little more in order to make it clear that "Hold next available" and "Hold specific item" are two mutually-exclusive options. To test, apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Perform the same tests as before: - Confirm you can successfully place single or multiple items on hold - Confirm that when placing a single hold the next available options are grouped - Confirm pickup location, specific item type, and number of holds all work for next available - Confirm item level holds save pickup location correctly Rebased, attempted to test Owen's patches. When I click "Hold next available item" or "Place a hold on a specific item" the options are not enabling/disabling correctly. The overall look is very clear though Created attachment 134893 [details] [review] Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer This patch alters the structure of the hold for a little more in order to make it clear that "Hold next available" and "Hold specific item" are two mutually-exclusive options. To test, apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Perform the same tests as before: - Confirm you can successfully place single or multiple items on hold - Confirm that when placing a single hold the next available options are grouped - Confirm pickup location, specific item type, and number of holds all work for next available - Confirm item level holds save pickup location correctly Created attachment 134894 [details] [review] Bug 30579: (follow-up) Expand range of controls which are disabled/enabled This patch updates the process of switching between "hold next" and "hold specific item" so that all relevant form fields are disabled. I really like the redesign - it's very clear and user friendly. It all works perfectly, except if the staff make a mistake and do the following: - Place a hold for a patron - Choose 'Place a hold on a specific item' - Fail to choose an item - Just press the 'Place hold' button beneath 'Place a hold on a specific item' (#hold_item_btn) The hold is now placed for pick up at a blank library. If you check an item in to fill the hold, Koha presents an error page, e.g. Can't call method "branchcode" on an undefined value at /kohadevbox/koha/Koha/Item.pm line 683 On the current version of Koha, if the staff press this button (#hold_item_btn) without selecting an item, the hold defaults to being a bib level hold where the pick up library is the logged in library (as if they'd pressed: #hold_any_btn) It'd be good if the 'Place hold' button for 'Place a hold on a specific item' could be blanked out until a specific item has been chosen. Alternatively, maybe the pick up location drop down could go back into the 'Hold details' section at the top of the page - so then it can still fall back to a bib level hold if it's pressed without selection, but it would still be given a location. Created attachment 140951 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch simply groups the forms so that multi-hold and single hold are each in their own section While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Place a single bib and multi bib hold in staff client 2 - Apply this patch 3 - Repeat and confirm nothing changes with this patch Created attachment 140952 [details] [review] Bug 30579: Whitespace Created attachment 140953 [details] [review] Bug 30579: Group next available hold options, and disable when item is chosen This patch rearranges options that only apply to next available holds and disables them when an item is checked. This is intended to make it more obvious which parts of the form apply to this selection To test: 1 - Apply patches 2 - Confirm you can successfully place single or multiple items on hold 3 - Confirm that when placing a single hold the next available options are grouped 4 - Confirm pickup location, specific item type, and number of holds all work for next available 5 - Confirm item level holds save pickup location correctly Created attachment 140954 [details] [review] Bug 30579: (follow-up) Update layout to make hold any vs. hold item clearer This patch alters the structure of the hold for a little more in order to make it clear that "Hold next available" and "Hold specific item" are two mutually-exclusive options. To test, apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Perform the same tests as before: - Confirm you can successfully place single or multiple items on hold - Confirm that when placing a single hold the next available options are grouped - Confirm pickup location, specific item type, and number of holds all work for next available - Confirm item level holds save pickup location correctly This will at least apply now - with the exception of Owen's last patch - I think it needs some cleanup though - could you take a look Owen? Created attachment 148835 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly Discussed at hackfest - next available item from an item group needs a pickup location as an error is being thrown Created attachment 148898 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly Hi Nick - I'm getting an alert asking me to select a pickup location when placing a hold on next available item, even when a location is selected. Could you check again at your end and see if you're seeing the same? Looks like the alert is being thrown by the else block in lines 1570 - 1575 Created attachment 149271 [details] [review] Bug 30579: (follow-up) Update form checker for each form Still getting an error, this time when placing a hold on next available item in an item group - I'm getting an alert saying "Please select an item group to place hold", even though an item group is selected I don't think this works as intended for me. 1. When I load the page, the radio button to the right of "Hold next available item" is active/selected. - Under "Hold next available item from an item group", the dropdown and the radio button is disabled. The dropdown is "greyed out" and nothing happens if I click on either of them. - But under "Place a hold on a specific item" the dropdown for "Allowed pickup locations" is not "greyed out" and I can interact with it. I can click on the radio buttons under "Hold", and if I do, the radio button to the right of "Hold next available item" is no longer active/selected. In fact, none of the three radio buttons to the right of the headings are active/selected. 2. If i click on the radio button for "Hold next available item from an item group" - I can interact with the "Hold" radio button and the "Allowed pickup locations" dropdown under "Place a hold on a specific item". - If i click on a radio button under "Hold", then the radio button to the right of "Hold next available item from an item group" remains active/selected (this is different from what happens under 1 above). PS. Tested on 112.0.5615.49 snap (64-bit) on Ubuntu Created attachment 150767 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly Created attachment 150786 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly Signed-off-by: Hinemoea Viault <hinemoea.viault@inlibro.com> Hi Hinemoea. As you have signed off this bug, could you update the bug status to Signed Off? That way, you will get the credit on the dashboard (https://dashboard.koha-community.org/). (It will also move in to the Needs QA queue, for the QA Team to do their thing!) David CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt Created attachment 151483 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly Signed-off-by: hinemoea <hinemoea@inlibro.com> Created attachment 151493 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Place a hold for a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Confirm you can successfully place all 3 types of holds 10 - Confirm if placing a second hold that the type is forced correctly Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Created attachment 151558 [details] [review] Bug 30579: Add missing for attribute on label That's better but we should do better in my opinion. First would be to have only 1 "place hold" button maybe? And hide the form when the related radio is not checked? Owen, what do you think? This is a very brave effort to get this template more clear. But as it seems, not without any discussion. There was no feedback on the previous comment. The number of adjustments .../prog/en/modules/reserve/request.tt | 587 ++++++++++-------- Does not make it easier. Holds is very bug prone. Not to mention recent additions like item groups etc. I am not sure if this (simple) test plan covers all edge cases. Hard to verify. Does this need some broader consensus/discussion? Should this be marked as an enhancment? I could see us using a Bootstrap Collapse to show the options, with the "next available" section displayed by default. I could also see us keeping this version with the addition of making sure that all form fields are disabled when the section isn't "active." We could even set the opacity of the inactive fieldsets to .5 or something like that in order to indicate that the controls are not available. I find that the pickup location dropdowns are not populating, and the form doesn't prevent me from placing the hold without selecting one. That must be resolved. I agree that a more detailed test plan would be helpful. Created attachment 154881 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Created attachment 154882 [details] [review] Bug 30579: Add missing for attribute on label Created attachment 154883 [details] [review] Bug 30579: Remove doubled notes field (In reply to Marcel de Rooy from comment #39) > I am not sure if this (simple) test plan covers all edge cases. Test plan updated > Should this be marked as an enhancment? Done (In reply to Owen Leonard from comment #40) > I could see us using a Bootstrap Collapse to show the options, with the > "next available" section displayed by default. > > I could also see us keeping this version with the addition of making sure > that all form fields are disabled when the section isn't "active." We could > even set the opacity of the inactive fieldsets to .5 or something like that > in order to indicate that the controls are not available. Follow-up patch will be attached > I find that the pickup location dropdowns are not populating, and the form > doesn't prevent me from placing the hold without selecting one. That must be > resolved. I cannot recreate, can you retest? Can you confirm API is loading content on other pages like http://localhost:8081/cgi-bin/koha/admin/branches.pl Created attachment 154886 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) I really like where this is going! I found a few more bugs in testing, but it seems to be nearly there. The only two that are definite blockers for me are some odd behavior with hold policy limits: - When trying to place a hold on an item group, I always got the prompt: "None of these items can normally be put on hold for this patron. Place hold?" although there should be nothing forbidding that hold. (with item group holds enabled and regardless of patron or AllowHoldPolicyOverride value) - The circ rule for maximum number of items allowed per record was not enforced for item-level holds. i.e. with a limit of 2, it was possible to place 3 item-level holds for the same patron even if overrides were not allowed. If overrides are allowed, there should be a warning at the top of the page, but the warning does not appear for a patron with the max number of item-level holds. Not necessarily blockers but would be nice to fix: - The select2's for pickup location are not actually disabled when the fieldset is disabled. (They are greyed out, and the data selected is not used, but they're still clickable.) Is this an oversight, or is it an unfortunate characteristic of select2's? (not necessarily a blocker for me because the visual cue and actual processing of data are correct) - When enforcing that a patron with an existing item-level hold may only place item-level holds, the heading for item-level holds says (Required) in bright red. Same thing for item group holds. However, the heading for record-level holds does not say (Required). (this is the case in master, but if it's easy to add in here for consistency, I'd say do it) Created attachment 157787 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Created attachment 157788 [details] [review] Bug 30579: Add missing for attribute on label Created attachment 157789 [details] [review] Bug 30579: Remove doubled notes field Created attachment 157790 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Created attachment 157791 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Created attachment 157792 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required (In reply to Emily Lamancusa from comment #46) > I really like where this is going! I found a few more bugs in testing, but > it seems to be nearly there. > > The only two that are definite blockers for me are some odd behavior with > hold policy limits: > > - When trying to place a hold on an item group, I always got the prompt: > "None of these items can normally be put on hold for this patron. Place > hold?" although there should be nothing forbidding that hold. (with item > group holds enabled and regardless of patron or AllowHoldPolicyOverride > value) Button updated to fix this > > - The circ rule for maximum number of items allowed per record was not > enforced for item-level holds. i.e. with a limit of 2, it was possible to > place 3 item-level holds for the same patron even if overrides were not > allowed. If overrides are allowed, there should be a warning at the top of > the page, but the warning does not appear for a patron with the max number > of item-level holds. I don't recreate this one > > > Not necessarily blockers but would be nice to fix: > > - The select2's for pickup location are not actually disabled when the > fieldset is disabled. I don't see a way to accomplish this, I think it's just select2 > - When enforcing that a patron with an existing item-level hold may only > place item-level holds, the heading for item-level holds says (Required) in > bright red. Added Created attachment 157937 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 157938 [details] [review] Bug 30579: Add missing for attribute on label Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 157939 [details] [review] Bug 30579: Remove doubled notes field Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 157940 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 157941 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 157942 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 157943 [details] [review] Bug 30579: (follow-up) fix button id Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Awesome, just tested the fixes and they all seem to be working!
Adding a tiny follow-up patch to fix what I assume is a copy/paste error, though.
In the "Use same conditions..." follow-up, shouldn't the button id still be "hold_grp_btn" though? (the patch changed it to "hold_any_btn")
> > - The circ rule for maximum number of items allowed per record was not
> > enforced for item-level holds. i.e. with a limit of 2, it was possible to
> > place 3 item-level holds for the same patron even if overrides were not
> > allowed. If overrides are allowed, there should be a warning at the top of
> > the page, but the warning does not appear for a patron with the max number
> > of item-level holds.
>
> I don't recreate this one
I'm not recreating it anymore either. Maybe it was just something weird on my end.
I don't follow Koha's template logic well enough yet to be confident QAing this one, but I added an additional sign-off since I confirm everything worked as expected in my testing.
(In reply to Emily Lamancusa from comment #61) > Awesome, just tested the fixes and they all seem to be working! > Adding a tiny follow-up patch to fix what I assume is a copy/paste error, > though. > > In the "Use same conditions..." follow-up, shouldn't the button id still be > "hold_grp_btn" though? (the patch changed it to "hold_any_btn") Yes, lazy copy/pasta error, thank you for follow-up :-) Created attachment 163260 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163261 [details] [review] Bug 30579: Add missing for attribute on label Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163262 [details] [review] Bug 30579: Remove doubled notes field Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163263 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163264 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163265 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163266 [details] [review] Bug 30579: (follow-up) fix button id Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 163267 [details] [review] Bug 30579: (follow-up) Remove unnecessary nested form elements This patch and bz31579 have different logics This patch completely changes the form, with this patch you can either use “Hold next available item” (containing the “Pickup at” field), or “Place to hold on a specific item” since selecting one disables the other. However, bz31579 needs the “Pickup at” and “Place a hold on a specific item” fields to all be active since we configure the “Place a hold on a specific item” fields according to the value chosen in the field “Pickup at” which means that it is impossible to use both patches at the same time, we will have to make a choice between the two. Poke me in mattermost or slack when this is rebased and I'll jump straight on it. Created attachment 166012 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166013 [details] [review] Bug 30579: Add missing for attribute on label Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166014 [details] [review] Bug 30579: Remove doubled notes field Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166015 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166016 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166017 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166018 [details] [review] Bug 30579: (follow-up) fix button id Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Created attachment 166019 [details] [review] Bug 30579: (follow-up) Remove unnecessary nested form elements Looks like there was an addition when using item groups to list the items and to list (disabled) groups with no items - so should define an item group with no items for testing Created attachment 166034 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166035 [details] [review] Bug 30579: Add missing for attribute on label Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166036 [details] [review] Bug 30579: Remove doubled notes field Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166037 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166038 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166039 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166040 [details] [review] Bug 30579: (follow-up) fix button id Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166041 [details] [review] Bug 30579: (follow-up) Remove unnecessary nested form elements Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> This is a great improvement. All working as described and no regressions found. Tests all passing, QA script happy Passing QA Created attachment 166074 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166075 [details] [review] Bug 30579: Add missing for attribute on label Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166076 [details] [review] Bug 30579: Remove doubled notes field Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166077 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166078 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166079 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166080 [details] [review] Bug 30579: (follow-up) fix button id Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166081 [details] [review] Bug 30579: (follow-up) Remove unnecessary nested form elements Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166082 [details] [review] Bug 30579: Fix rebase issue This one is for Nick's hair... Tiny bit torn about this one, it makes things clearer, but it adds a click... but yes, I can see why we are doing this. Still... sadly found some blockers that prevent me from pushing this right now. Please follow-up quickly! If not marked as blocker, it's also good for a later follow-up. 1) Standardize headings We have: Hold next available item Place a hold on a specific item We could do: Hold on next available item Hold on a specific item OR Place hold on next available item Place hold on a specific item Tend to the first, because it's a little shorter. 2) Usability I know that I can click on the text instead of into the radio box, but from the formatting that is not visually clear. Could we think about replacing the checkboxes for an element that makes for a bigger "click target" visually? 3) Multi-holds result in a CSRF error (blocker) Programming error - No CSRF token passed for POST http://localhost:8081/intranet/reserve/placerequest.pl (referer: http://localhost:8081/cgi-bin/koha/reserve/request.pl?biblionumber=262&biblionumber=5&biblionumber=13&biblionumber=146&biblionumber=191&biblionumber=11&biblionumber=126&biblionumber=139&biblionumber=165&biblionumber=255&biblionumber=115&multi_hold=1&borrowernumber=19) 4) Multi-holds missing pick-up location and other options (blocker) If you do a lot of holds at once, the pick-up location at the top allowed you to set them all at once. now this option and others, like "hold expires on" and "notes" are completely gone. (In reply to Katrin Fischer from comment #101) > 4) Multi-holds missing pick-up location and other options (blocker) > > If you do a lot of holds at once, the pick-up location at the top allowed > you to set them all at once. now this option and others, like "hold expires > on" and "notes" are completely gone. I can't recreate this. Or maybe it's more a question of clarity of interface? Anything I set in the top "Hold details" section of the page is applied to all holds created, regardless of choices lower in the page. When I entered notes and hold expiration date, those values were applied to all holds created via multi-hold, just as they were applied to individual bib- or item-level holds. (In reply to Andrew Fuerste-Henry from comment #102) > (In reply to Katrin Fischer from comment #101) > > 4) Multi-holds missing pick-up location and other options (blocker) > > > > If you do a lot of holds at once, the pick-up location at the top allowed > > you to set them all at once. now this option and others, like "hold expires > > on" and "notes" are completely gone. > > I can't recreate this. Or maybe it's more a question of clarity of > interface? Anything I set in the top "Hold details" section of the page is > applied to all holds created, regardless of choices lower in the page. When > I entered notes and hold expiration date, those values were applied to all > holds created via multi-hold, just as they were applied to individual bib- > or item-level holds. Maybe I should have been more clear: I am placing multiple holds on different records by selecting them in the staff interface result list. The hold details section is completely missing on top of the table. Can you check if that is the same thing you were looking at? (In reply to Katrin Fischer from comment #103) > Can you check if that is the same thing you were looking at? It is not! I thought you meant placing more than one hold at a time on the same record. I can confirm that the interface to place holds on multiple different records at once currently includes notes, a selection for pickup branch, expiration date, and non-priority toggle. With this patch, none of those things show. I know people are keen on this from the activity - it's still on the table, but we will need to hurry a little. Created attachment 166264 [details] [review] Bug 30579: Disentangle multi-hold and single bib forms This patch alters the structure of the hold request page to make it clear that "Hold next available", "Hold item group", and "Hold specific item" are mutually-exclusive options. While there is some duplication from this, it makes the sections easier to read and allows for more variation in the two forms To test: 1 - Find a bib with multiple items 2 - Enable item groups and item group holds in system preferences 3 - Load the records detail page 4 - Add an item group on the item groups tab 5 - Select two items and add to the group 6 - Click the 'Holds' tab and search for/select a patron 7 - Confirm the three levels of holds are clear 8 - Confirm checking the radio next to one disables the others 9 - Check 'Hold next available item from an item group' 10 - Do not select an item group 11 - Click 'Place hold' and confirm you are notified of need to select an item group NOTE: if you are overrirding you may also have an alert that the items cannot normally be put on hold 12 - Click 'Place hold on a specific item' - but do not select an item 13 - Click place hold and confirm there is an alert and you cannot continue 14 - Click 'Hold next available item' and place hold 15 - Hold is successfully placed 16 - Place another hold for the same patron 17 - Only the 'Hold next available item' form is enabled 18 - Confirm you cannot switch hold type 19 - Place hold 20 - Select a new patron and place an item group hold 21 - Select the same patron and place another hold - you are forced to place an item group hold 22 - Select a new patron and place a hold on a specific item 23 - Place a second hold, confirm you can only place it on a specific item Signed-off-by: hinemoea <hinemoea@inlibro.com> Signed-off-by: Sam Lau <samalau@gmail.com> Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166265 [details] [review] Bug 30579: Add missing for attribute on label Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166266 [details] [review] Bug 30579: Remove doubled notes field Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166267 [details] [review] Bug 30579: (follow-up) Add fieldsets and style disabled elements This patch surrounds each section with a fieldset which allow enabling/disabling all the elements in one go. CSS is added to make it clear which fields are disabled. Button for item specific is moved form the multi-hold section Javascript is simplified to use the new fieldsets To test: 1 - Repeate test plan on earlier commit 2 - Additionally, search and select multiple records and place hold 3 - Confirm multi-hold placing works :-) Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166268 [details] [review] Bug 30579: (follow-up) Use same conditions on item groups place hold button as on other options Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166269 [details] [review] Bug 30579: (follow-up) Add required label and warning when hold levels required Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166270 [details] [review] Bug 30579: (follow-up) fix button id Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166271 [details] [review] Bug 30579: (follow-up) Remove unnecessary nested form elements Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 166272 [details] [review] Bug 30579: Fix rebase issue Created attachment 166273 [details] [review] Bug 30579: (follow-up) Fix rebase of multi holds form and add CSRF This patch restores missing elemtns to multi-hold form and adds CSRF variables Created attachment 166274 [details] [review] Bug 30579: (follow-up) Standardize labels: This patch changes the heading for specific item holds, so the three options are now: Hold next available item Hold next available item from an item group Hold a specific item Created attachment 166275 [details] [review] Bug 30579: (follow-up) Style selections as buttons I am not persuaded by the current "design" of the radiobox buttons, but maybe we can get some help there if we ask really nicely as a follow-up? Pushed for 24.05! Well done everyone, thank you! (In reply to Katrin Fischer from comment #118) > I am not persuaded by the current "design" of the radiobox buttons, but > maybe we can get some help there if we ask really nicely as a follow-up? Agree. Also, inputs as children of a button creates invalid HTML. Not backported to 23.11.x I think the additional pieces were handled in follow-up bugs.. dropping the additional_work_needed flag |