Bug 30707

Summary: Move container's closing div tag into template from intranet-bottom.inc
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: Pushed to main --- QA Contact: Jonathan Druart <jonathan.druart>
Severity: normal    
Priority: P5 - low CC: david, dcook, lucas, phil
Version: MainKeywords: RM_priority
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
For templates which use the intranet-bottom include file, the markup in that template lacks the main container's closing </div>, expecting it to be closed in intranet-bottom.inc. This moves that closing </div> into each template. This is intended to make it clearer to anyone trying to maintain the correct structure of the page, and hopefully to make it possible to automatically tidy template files.
Version(s) released in:
25.05.00
Circulation function:
Bug Depends on:    
Bug Blocks: 38713    
Attachments: Bug 30707: Move container's closing div tag into template from intranet-bottom.inc
Bug 30707: Move container's closing div tag into template from intranet-bottom.inc
Bug 30707: Fix acqui/z3950_search
Bug 30707: Fix cataloguing/z3950_search.tt
Bug 30707: Fix acqui/z3950_search
Bug 30707: Fix cataloguing/z3950_search.tt
Bug 30707: (follow-up) Many more instances
Bug 30707: Fix catalogue/MARCdetail.tt and catalogue/labeledMARCdetail.tt
Bug 30707: Fix members/moremember-print.tt
Bug 30707: Move container's closing div tag into template from intranet-bottom.inc
Bug 30707: Fix acqui/z3950_search
Bug 30707: Fix cataloguing/z3950_search.tt
Bug 30707: (follow-up) Many more instances
Bug 30707: Fix members/moremember-print.tt
Bug 30707: Move container's closing div tag into template from intranet-bottom.inc
Bug 30707: Fix acqui/z3950_search
Bug 30707: Fix cataloguing/z3950_search.tt
Bug 30707: (follow-up) Many more instances
Bug 30707: Fix members/moremember-print.tt

Description Jonathan Druart 2022-05-06 08:50:07 UTC
There is a closing div in intranet-bottom.inc, the related opening div is usually added by the different template.
Since it's at the top of the include file it does not make much sense to have it here, better to not add confusion to the templates and let them close it from there.
Comment 1 Jonathan Druart 2022-05-06 11:43:03 UTC
From IRC

oleonard> Joubu: Re: Bug 30707, I know it was my decision ages ago but I don't remember the reason. It may not have even been a good one at the time!
Comment 2 Owen Leonard 2024-12-13 16:04:45 UTC Comment hidden (obsolete)
Comment 3 David Nind 2024-12-15 18:13:32 UTC
Created attachment 175478 [details] [review]
Bug 30707: Move container's closing div tag into template from intranet-bottom.inc

For templates which use the intranet-bottom include file, the markup in
that template lacks the main container's closing </div>, expecting it to
be closed in intranet-bottom.inc.

This patch moves that closing </div> into each template. This is
intended to make it clearer to anyone trying to maintain the correct
structure of the page and hopefully to make it possible to automatically
tidy template files.

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-12-15 18:17:36 UTC
Testing notes (using KTD):

1. I wasn't sure how to test this, so I did these things:
   - Checked the patch - it makes logical sense to me
   - Used an HTML validator on a sample of pages (such as About Koha, main page, Cataloguing page, and various others) - no errors because of missing divs
Comment 5 Jonathan Druart 2024-12-16 10:19:08 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2024-12-16 15:58:14 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2024-12-17 11:07:33 UTC
Adjusting status as there might be other problems here.
Comment 8 Owen Leonard 2024-12-17 13:41:25 UTC
Created attachment 175611 [details] [review]
Bug 30707: Fix acqui/z3950_search

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 9 Owen Leonard 2024-12-17 13:41:27 UTC
Created attachment 175612 [details] [review]
Bug 30707: Fix cataloguing/z3950_search.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 10 Owen Leonard 2024-12-17 13:41:30 UTC
Created attachment 175613 [details] [review]
Bug 30707: (follow-up) Many more instances
Comment 11 Phil Ringnalda 2024-12-17 14:05:18 UTC
Created attachment 175617 [details] [review]
Bug 30707: Fix catalogue/MARCdetail.tt and catalogue/labeledMARCdetail.tt
Comment 12 Jonathan Druart 2024-12-17 14:15:53 UTC
(In reply to Phil Ringnalda from comment #11)
> Created attachment 175617 [details] [review] [review]
> Bug 30707: Fix catalogue/MARCdetail.tt and catalogue/labeledMARCdetail.tt

This is not correct. If you have an invalid record you don't want to see the aside.
Comment 13 Phil Ringnalda 2024-12-17 18:57:04 UTC
And I guess since invalid bib numbers are going to be broken anyway by the STOP in includes/blocking_errors.inc, it doesn't actually matter whether they also are in MARCdetail.
Comment 14 Jonathan Druart 2024-12-19 14:10:44 UTC
Created attachment 175766 [details] [review]
Bug 30707: Fix members/moremember-print.tt
Comment 15 Jonathan Druart 2025-01-21 08:48:16 UTC
Ready for testing, but the patches can also be tested with the parent bugs applied.

If the patches do not apply retrieve them from the branch of bug 38664.
Comment 16 Katrin Fischer 2025-01-24 14:10:23 UTC
Can you please fix the conflict in searchresultlist-auth.tt?
Comment 17 Lucas Gass (lukeg) 2025-01-24 14:31:39 UTC
I pulled current main this AM and there is 1 file in this patchset not applying cleanly: 

authorities/searchresultlist-auth.tt
Comment 18 Katrin Fischer 2025-01-24 14:33:22 UTC
(In reply to Lucas Gass (lukeg) from comment #17)
> I pulled current main this AM and there is 1 file in this patchset not
> applying cleanly: 
> 
> authorities/searchresultlist-auth.tt

Yes, same for me.
Comment 19 Owen Leonard 2025-01-24 15:50:59 UTC
Created attachment 177120 [details] [review]
Bug 30707: Move container's closing div tag into template from intranet-bottom.inc

For templates which use the intranet-bottom include file, the markup in
that template lacks the main container's closing </div>, expecting it to
be closed in intranet-bottom.inc.

This patch moves that closing </div> into each template. This is
intended to make it clearer to anyone trying to maintain the correct
structure of the page and hopefully to make it possible to automatically
tidy template files.

Signed-off-by: David Nind <david@davidnind.com>
Comment 20 Owen Leonard 2025-01-24 15:51:02 UTC
Created attachment 177121 [details] [review]
Bug 30707: Fix acqui/z3950_search

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 21 Owen Leonard 2025-01-24 15:51:04 UTC
Created attachment 177122 [details] [review]
Bug 30707: Fix cataloguing/z3950_search.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 22 Owen Leonard 2025-01-24 15:51:07 UTC
Created attachment 177123 [details] [review]
Bug 30707: (follow-up) Many more instances
Comment 23 Owen Leonard 2025-01-24 15:51:10 UTC
Created attachment 177124 [details] [review]
Bug 30707: Fix members/moremember-print.tt
Comment 24 David Nind 2025-01-25 09:21:28 UTC
Created attachment 177137 [details] [review]
Bug 30707: Move container's closing div tag into template from intranet-bottom.inc

For templates which use the intranet-bottom include file, the markup in
that template lacks the main container's closing </div>, expecting it to
be closed in intranet-bottom.inc.

This patch moves that closing </div> into each template. This is
intended to make it clearer to anyone trying to maintain the correct
structure of the page and hopefully to make it possible to automatically
tidy template files.

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 David Nind 2025-01-25 09:21:32 UTC
Created attachment 177138 [details] [review]
Bug 30707: Fix acqui/z3950_search

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 26 David Nind 2025-01-25 09:21:35 UTC
Created attachment 177139 [details] [review]
Bug 30707: Fix cataloguing/z3950_search.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 27 David Nind 2025-01-25 09:21:39 UTC
Created attachment 177140 [details] [review]
Bug 30707: (follow-up) Many more instances

Signed-off-by: David Nind <david@davidnind.com>
Comment 28 David Nind 2025-01-25 09:21:42 UTC
Created attachment 177141 [details] [review]
Bug 30707: Fix members/moremember-print.tt

Signed-off-by: David Nind <david@davidnind.com>
Comment 29 David Nind 2025-01-25 09:24:11 UTC
I wasn't sure exactly how to test - if what I've done isn't sufficient, please change the status back to needs sign off.

Testing notes (using KTD):
1. Check a sample of the change sin the patch - check that the templates have a closing div.
2. Check a sample of page sin the staff interface to make sure they continue to work as expected.
Comment 30 Jonathan Druart 2025-01-29 10:59:41 UTC
Passing QA.
Comment 31 Katrin Fischer 2025-01-30 17:07:08 UTC
Pushed for 25.05!

Well done everyone, thank you!