Bug 38718 - main container wrapper
Summary: main container wrapper
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 38714
Blocks: 38664 39357
  Show dependency treegraph
 
Reported: 2024-12-17 07:56 UTC by Jonathan Druart
Modified: 2025-03-17 12:26 UTC (History)
4 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 38718: (follow-up) Wrap both search form and search results in wrapper (2.03 KB, patch)
2025-01-31 19:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 38718: (follow-up) Use main-container.inc on ILL requests page (9.75 KB, patch)
2025-01-31 19:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 38718: (follow-up) Restore sidebar menu to point of sale page (849 bytes, patch)
2025-01-31 19:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 38718: (follow-up) Correct aside parameter (944 bytes, patch)
2025-01-31 19:08 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 38718: (follow-up) Use container wrapper on tags page (8.08 KB, patch)
2025-01-31 19:08 UTC, Owen Leonard
Details | Diff | Splinter Review
ILL form (71.20 KB, image/png)
2025-02-19 11:16 UTC, Pedro Amorim
Details
Previously (left) and now (right) (143.82 KB, image/jpeg)
2025-02-19 12:14 UTC, Owen Leonard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2024-12-17 07:56:21 UTC
On bug 38664 we are going to tidy the template file and so indent them properly.

Most of our templates have the following structure:

<div class="main container-fluid">
    <div class="row">
        <div class="col-md-10 order-md-2 order-sm-1">
            <main>
                [% INCLUDE 'messages.inc' %]
                <!-- The content -->
            </main>
        </div>
        <div class="col-md-2 order-sm-2 order-md-1">
            <aside>
            </aside>
        </div>
    </div>
</div>

It would be useful to have a WRAPPER to avoid:
* errors (sometimes we see the two divs with the same order: sm-2)
* inconsistencies
* 4 indentation blocks (will be replaced with a single one)
Comment 1 Jonathan Druart 2024-12-17 08:11:42 UTC Comment hidden (obsolete)
Comment 2 Jonathan Druart 2024-12-17 08:37:19 UTC
This is how it would look like: https://gitlab.com/joubu/Koha/-/commit/360a3811f3194b762028c3a927c81f14322d6f38
Comment 3 Jonathan Druart 2025-01-21 08:46:51 UTC
This is ready for testing, use the commits from the branch of bug 38664.
Comment 4 Owen Leonard 2025-01-31 19:08:29 UTC
Created attachment 177403 [details] [review]
Bug 38718: (follow-up) Wrap both search form and search results in wrapper

Also pass "wide_full" parameter to main-container.inc
Comment 5 Owen Leonard 2025-01-31 19:08:32 UTC
Created attachment 177404 [details] [review]
Bug 38718: (follow-up) Use main-container.inc on ILL requests page
Comment 6 Owen Leonard 2025-01-31 19:08:34 UTC
Created attachment 177405 [details] [review]
Bug 38718: (follow-up) Restore sidebar menu to point of sale page
Comment 7 Owen Leonard 2025-01-31 19:08:36 UTC
Created attachment 177406 [details] [review]
Bug 38718: (follow-up) Correct aside parameter
Comment 8 Owen Leonard 2025-01-31 19:08:38 UTC
Created attachment 177407 [details] [review]
Bug 38718: (follow-up) Use container wrapper on tags page
Comment 9 Jonathan Druart 2025-02-03 08:37:52 UTC
Patches cherry-pick on bug_38664
Comment 10 Jonathan Druart 2025-02-10 11:53:44 UTC
Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Added to the bug_38664 remote branch

f5f2eeff9aa Bug 38718: (follow-up) Use container wrapper on tags page
4aaf897e847 Bug 38718: (follow-up) Correct aside parameter
f659d3d995e Bug 38718: (follow-up) Restore sidebar menu to point of sale page
bfc1a61e260 Bug 38718: (follow-up) Use main-container.inc on ILL requests page
9bdcc80dc2c Bug 38718: (follow-up) Wrap both search form and search results in wrapper
34337e73de5 Bug 38718: (follow-up) Fix a few minor errors
dc7e4d057b9 Bug 38718: Preferences pages should use prefs-menu.inc, not admin-menu.inc
0a958696b40 Bug 38718: Main container - Virtual shelves
5a40cd03020 Bug 38718: Main container - Tools
2679ab4f1ae Bug 38718: Main container - Serials
fd208977cd5 Bug 38718: Main container - Rotating collections
8f279d434ba Bug 38718: Main container - Reviews
8f931abffa8 Bug 38718: Main container - Reserves
07ebed37b20 Bug 38718: Main container - Reports
c9c6a436531 Bug 38718: Main container - POS
6c359c8a85e Bug 38718: Main container - Patron lists
eb284a2634e Bug 38718: Main container - Recalls
47ee7dd0d25 Bug 38718: Main container - Patron cards
5e2a6b13fb0 Bug 38718: Main container - Offline circ
01dbe1da2d5 Bug 38718: Main container - Members
09e10442c7e Bug 38718: Main container - Labels
2b8c93a35ef Bug 38718: Main container - Errors
19a31cd554e Bug 38718: Main container - Course reserves
ca751c33770 Bug 38718: Main container - Clubs
f2c04e7e433 Bug 38718: Main container - Circ
1e91cfb0743 Bug 38718: Main container - Cataloguing
1ea2aedc50b Bug 38718: Main container - Catalogue
2c1c7a6184f Bug 38718: Main container - Bookings
1272d7d442a Bug 38718: Main container - Basket
be6ce4b32bf Bug 38718: Main container - Authorities
d6f23ebdf9c Bug 38718: Main container - Administration
d22684b16d0 Bug 38718: Main container - Acquisition
Comment 11 Katrin Fischer 2025-02-11 14:03:12 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 12 Pedro Amorim 2025-02-19 11:16:18 UTC
Created attachment 178320 [details]
ILL form

It appears this commit:

https://github.com/Koha-Community/Koha/commit/8bc6839#diff-ea700ef00f2961c96d937e78bbed33a241c0ff8a71faa58f89488fb6ede38a0c

Is restoring the bug fixed by bug 38359 (form too close to the left side of the page).
Comment 13 Owen Leonard 2025-02-19 12:14:29 UTC
Created attachment 178321 [details]
Previously (left) and now (right)

(In reply to Pedro Amorim from comment #12)
> Is restoring the bug fixed by bug 38359 (form too close to the left side of
> the page).

If I'm seeing the right before and after, the form is now centered on the page rather than aligned to the right. It's different but it looks correct to me.
Comment 14 Pedro Amorim 2025-02-19 12:29:26 UTC
(In reply to Owen Leonard from comment #13)
> Created attachment 178321 [details]
> Previously (left) and now (right)
> 
> (In reply to Pedro Amorim from comment #12)
> > Is restoring the bug fixed by bug 38359 (form too close to the left side of
> > the page).
> 
> If I'm seeing the right before and after, the form is now centered on the
> page rather than aligned to the right. It's different but it looks correct
> to me.

The before is how it was before the bootstrap 5 upgrade.
The bootstrap 5 upgrade unintentionally (I believe) changed it.
Bug 38359 then fixed what the bootstrap 5 unintentionally changed.
This is now causing it to become how it was after the unintentional bootstrap 5 change again.

The above is how I understand this but you are right, the whole form was aligned to the right before, and it now looks aligned correctly.

Problem is that the inputs were more centered before (because the whole form was aligned to the right) and they're too pushed to the left now. Most forms have a left sidebar but that doesn't happen here. So I think the "right alignment" of the form was there to compensate for that and display the form inputs a bit more "centered" as if a siderbar existed, compeare with ERM agreements form or the patron edit form for example to see what I mean.

Anyway, I don't know what the fix (if any) is here.
Comment 15 Owen Leonard 2025-02-19 12:37:17 UTC
(In reply to Pedro Amorim from comment #14)
> compeare with ERM
> agreements form or the patron edit form for example to see what I mean.

ERM agreements always show the sidebar, so I don't think that's a good comparison. Comparing with the patron edit form I think it matches how the ILL form currently looks, which is why I consider it to be correct.