Bug 31806

Summary: Add 'page-section' to holds page
Product: Koha Reporter: David Nind <david>
Component: Staff interfaceAssignee: Martin Renvoize (ashimema) <martin.renvoize>
Status: CLOSED FIXED QA Contact: Katrin Fischer <katrin.fischer>
Severity: enhancement    
Priority: P5 - low CC: gmcharlt, lucas, martin.renvoize
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
22.11.00
Circulation function:
Bug Depends on: 30952    
Bug Blocks: 31757, 31905    
Attachments: Screenshot - Record details > Holds
Bug 31806: Add page-section to holds tables
Bug 31806: Add page-section container to existing holds
Bug 31806: Add page-section to holds tables
Bug 31806: Add page-section container to existing holds
Screenshots - options and display of holds page
Bug 31806: Add page-section to holds tables
Bug 31806: Add page-section container to existing holds

Description David Nind 2022-10-14 20:20:15 UTC
We need the page-section div when placing holds to ensure proper contrast for even/odd rows on the tables.

To replicate:
1. Place a hold or an item.
2. On the details page for a record select the holds section/tab.
Comment 1 Owen Leonard 2022-10-20 12:00:30 UTC
Are you referring to this page?

/cgi-bin/koha/reserve/request.pl?biblionumber=X&borrowernumber=X

I don't think I'm seeing the issue.
Comment 2 David Nind 2022-10-20 12:55:04 UTC
Created attachment 142242 [details]
Screenshot - Record details > Holds

On the Record details > Holds page: http://127.0.0.1:8081/cgi-bin/koha/reserve/request.pl?biblionumber=126
Comment 3 Martin Renvoize (ashimema) 2022-10-21 08:59:31 UTC
Created attachment 142309 [details] [review]
Bug 31806: Add page-section to holds tables

This patch adds the page-section class to each holds view on
reserve/request.pl

Test plan
1) Find an biblio to test on and add holds items of different types and
   homebranches.
2) Work through each setting of the HoldsSplitQueue preference and
   confirm the page-sections appear as expected
Comment 4 Martin Renvoize (ashimema) 2022-10-21 09:03:38 UTC
I wasn't entirely sure where to put the page-section on this one.. especially given the various options for HoldsSplitQueue.

At the moment, I've sectioned off each 'Split' as governed by HoldsSplitQueue.. but the argument could be made that the whole 'Existing holds' form could be a single page-section container?

Also.. do the buttons look a bit crowded together for anyone else.. probabably a different bug entirely, but I think the markup there might be wrong too.
Comment 5 Martin Renvoize (ashimema) 2022-10-21 09:11:22 UTC
Created attachment 142310 [details] [review]
Bug 31806: Add page-section container to existing holds

This patch adds a page-section container around the existing holds
content section
Comment 6 Martin Renvoize (ashimema) 2022-10-21 09:12:15 UTC
OK.. after a little more testing I decided a container around the whole existing holds section feels like it makes more sense.. second patch attached does this so you can test one after the other if you'd like to see the distinction.
Comment 7 David Nind 2022-10-23 22:50:15 UTC
Created attachment 142468 [details] [review]
Bug 31806: Add page-section to holds tables

This patch adds the page-section class to each holds view on
reserve/request.pl

Test plan
1) Find an biblio to test on and add holds items of different types and
   homebranches.
2) Work through each setting of the HoldsSplitQueue preference and
   confirm the page-sections appear as expected

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2022-10-23 22:50:21 UTC
Created attachment 142469 [details] [review]
Bug 31806: Add page-section container to existing holds

This patch adds a page-section container around the existing holds
content section

Signed-off-by: David Nind <david@davidnind.com>
Comment 9 David Nind 2022-10-23 22:56:51 UTC
Created attachment 142470 [details]
Screenshots - options and display of holds page

I've attached a PDF with screenshots of the options for HoldsSplitQueue.

Thanks Martin for logging separate bugs for the table heading row and button placement - I've added a comment about how the light grey background continues after the buttons.

Testing notes:
- For item types to display:
  . I added an item to the record I tested this on (books) and made it a Music item type
  . Placed record level and item level holds
Comment 10 Katrin Fischer 2022-10-24 15:46:17 UTC
Created attachment 142527 [details] [review]
Bug 31806: Add page-section to holds tables

This patch adds the page-section class to each holds view on
reserve/request.pl

Test plan
1) Find an biblio to test on and add holds items of different types and
   homebranches.
2) Work through each setting of the HoldsSplitQueue preference and
   confirm the page-sections appear as expected

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 11 Katrin Fischer 2022-10-24 15:46:22 UTC
Created attachment 142528 [details] [review]
Bug 31806: Add page-section container to existing holds

This patch adds a page-section container around the existing holds
content section

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 12 Tomás Cohen Arazi (tcohen) 2022-10-24 16:23:55 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!