Bug 31806 - Add 'page-section' to holds page
Summary: Add 'page-section' to holds page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Martin Renvoize (ashimema)
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 30952
Blocks: 31757 31905
  Show dependency treegraph
 
Reported: 2022-10-14 20:20 UTC by David Nind
Modified: 2023-12-28 20:42 UTC (History)
3 users (show)

See Also:
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:


Attachments
Screenshot - Record details > Holds (99.94 KB, image/png)
2022-10-20 12:55 UTC, David Nind
Details
Bug 31806: Add page-section to holds tables (2.69 KB, patch)
2022-10-21 08:59 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 31806: Add page-section container to existing holds (3.31 KB, patch)
2022-10-21 09:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review
Bug 31806: Add page-section to holds tables (2.73 KB, patch)
2022-10-23 22:50 UTC, David Nind
Details | Diff | Splinter Review
Bug 31806: Add page-section container to existing holds (3.36 KB, patch)
2022-10-23 22:50 UTC, David Nind
Details | Diff | Splinter Review
Screenshots - options and display of holds page (581.09 KB, application/pdf)
2022-10-23 22:56 UTC, David Nind
Details
Bug 31806: Add page-section to holds tables (2.79 KB, patch)
2022-10-24 15:46 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 31806: Add page-section container to existing holds (3.42 KB, patch)
2022-10-24 15:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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!