Description
Jonathan Druart
2022-05-06 08:50:07 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! Created attachment 175454 [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. 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> 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 Created attachment 175489 [details] [review] Bug 30707: Fix acqui/z3950_search Created attachment 175500 [details] [review] Bug 30707: Fix cataloguing/z3950_search.tt Adjusting status as there might be other problems here. Created attachment 175611 [details] [review] Bug 30707: Fix acqui/z3950_search Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 175612 [details] [review] Bug 30707: Fix cataloguing/z3950_search.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 175613 [details] [review] Bug 30707: (follow-up) Many more instances Created attachment 175617 [details] [review] Bug 30707: Fix catalogue/MARCdetail.tt and catalogue/labeledMARCdetail.tt (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. 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. Created attachment 175766 [details] [review] Bug 30707: Fix members/moremember-print.tt 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. Can you please fix the conflict in searchresultlist-auth.tt? I pulled current main this AM and there is 1 file in this patchset not applying cleanly: authorities/searchresultlist-auth.tt (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. 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> Created attachment 177121 [details] [review] Bug 30707: Fix acqui/z3950_search Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 177122 [details] [review] Bug 30707: Fix cataloguing/z3950_search.tt Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 177123 [details] [review] Bug 30707: (follow-up) Many more instances Created attachment 177124 [details] [review] Bug 30707: Fix members/moremember-print.tt 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> 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> 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> Created attachment 177140 [details] [review] Bug 30707: (follow-up) Many more instances Signed-off-by: David Nind <david@davidnind.com> Created attachment 177141 [details] [review] Bug 30707: Fix members/moremember-print.tt Signed-off-by: David Nind <david@davidnind.com> 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. Passing QA. Pushed for 25.05! Well done everyone, thank you! |