Bug 32633

Summary: Add 'page-section' to cataloging and authority pages
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Martin Renvoize <martin.renvoize>
Severity: minor    
Priority: P5 - low CC: lucas, martin.renvoize, testopia
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:
23.05.00,22.11.03
Circulation function:
Bug Depends on: 30952    
Bug Blocks: 31757    
Attachments: Bug 32633: Add 'page-section' to cataloging and authority pages
Bug 32633: Add 'page-section' to cataloging and authority pages
Bug 32633: Add 'page-section' to cataloging and authority pages
Bug 32633: (QA follow-up) Move page-section nesting
Bug 32633: (QA follow-up) Add handling for authorities merge

Description Owen Leonard 2023-01-13 17:50:38 UTC
A few cataloging and authority templates need to have main content wrapped in a "page-section" container.

- includes/merge-record.inc
- authorities/searchresultlist.tt
- cataloguing/addbooks.tt
- cataloguing/additem.tt
Comment 1 Owen Leonard 2023-01-13 18:05:25 UTC
Created attachment 145302 [details] [review]
Bug 32633: Add 'page-section' to cataloging and authority pages

This patch adds a "page-section" container div around the main section
of cataloging and authority pages which lack it.

This patch contains indentation changes, so please ignore whitespace
when looking at the diff.

Apply the patch and view the following pages to confirm that the main
content is contained in a white box:

 - Cataloging -> Cataloging search results
 - Cataloging -> Edit items: The table of items and the item edit form
   should each be contained in a page-section.
 - Authorities -> Search results
 - Authorites -> Merge authorities: From the authorities search results
   list, click "Actions -> Merge" for one result, and then "Actions ->
   Merge" for another.
 - Click "Next" after selecting a merge reference.
   - The "Destination record" information should be contained in a
     page-section.
Comment 2 Lucas Gass 2023-01-13 20:48:28 UTC
Owen,

Should the page section on cataloging search include the pagination links? I know it is not a dataTable but I am comparing it to item search. 

I am still a bit confused on what the standard should be.
Comment 3 Katrin Fischer 2023-01-16 19:45:41 UTC
Created attachment 145340 [details] [review]
Bug 32633: Add 'page-section' to cataloging and authority pages

This patch adds a "page-section" container div around the main section
of cataloging and authority pages which lack it.

This patch contains indentation changes, so please ignore whitespace
when looking at the diff.

Apply the patch and view the following pages to confirm that the main
content is contained in a white box:

 - Cataloging -> Cataloging search results
 - Cataloging -> Edit items: The table of items and the item edit form
   should each be contained in a page-section.
 - Authorities -> Search results
 - Authorites -> Merge authorities: From the authorities search results
   list, click "Actions -> Merge" for one result, and then "Actions ->
   Merge" for another.
 - Click "Next" after selecting a merge reference.
   - The "Destination record" information should be contained in a
     page-section.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Katrin Fischer 2023-01-30 22:57:19 UTC
Just realized.. I SO here. Can someone else take it for QA please?
Comment 5 Martin Renvoize 2023-01-31 14:19:04 UTC
Created attachment 145872 [details] [review]
Bug 32633: Add 'page-section' to cataloging and authority pages

This patch adds a "page-section" container div around the main section
of cataloging and authority pages which lack it.

This patch contains indentation changes, so please ignore whitespace
when looking at the diff.

Apply the patch and view the following pages to confirm that the main
content is contained in a white box:

 - Cataloging -> Cataloging search results
 - Cataloging -> Edit items: The table of items and the item edit form
   should each be contained in a page-section.
 - Authorities -> Search results
 - Authorites -> Merge authorities: From the authorities search results
   list, click "Actions -> Merge" for one result, and then "Actions ->
   Merge" for another.
 - Click "Next" after selecting a merge reference.
   - The "Destination record" information should be contained in a
     page-section.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2023-01-31 14:19:09 UTC
Created attachment 145873 [details] [review]
Bug 32633: (QA follow-up) Move page-section nesting

This follow-up makes a small change to the page-section nesting on the
Cataloging and Authorities search results pages.  We move the 'h2'
outside of the page section but include the paging and number of results
inside with the results tables themselves.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 7 Martin Renvoize 2023-01-31 14:19:14 UTC
Created attachment 145874 [details] [review]
Bug 32633: (QA follow-up) Add handling for authorities merge

When you first click 'actions -> merge' from a result in the authories
search page you will see a new 'Merging with authority: ' text appear at
the top of the search results.

Prior to this patch, the text appears outside of the white page-section
and is almost un-noticable.

After this patch, we add a page-section with bg-info (blue backround)
around the merge message which makes it stand up more clearly.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 8 Martin Renvoize 2023-01-31 14:20:44 UTC
I agreed with Lucas on the pageing so did that as a QA follow-up.

It also highlighted to me that the merge message wasn't entirely clear sitting outside the page-section but without any background of it's own.. So I added a further QA follow-up to clarify that too.

No regressions found and code all passes QA tests...

Passing QA
Comment 9 Tomás Cohen Arazi 2023-02-02 15:02:11 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 10 Jacob O'Mara 2023-02-06 15:58:29 UTC
Nice work, thanks everyone!

Pushed to 22.11.x for the next release.
Comment 11 Lucas Gass 2023-02-14 21:29:29 UTC
Missing dependencies for 22.05.x, no backport.