Bug 32507 - Use template wrapper to build breadcrumb navigation
Summary: Use template wrapper to build breadcrumb navigation
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 32973 33000 33001 33005 33006 33068 33071 33111 33127 33129 33130 33131 33147 33148 33149 33372 33373 33382 33383 33384 33385 33386 33387 33388 33389 33409 33410 33429 33434 33436 33437 33438 33439 33555 33558 33559 33564 33565 33566 33571 33572 33582 33598 33599 33600 33601
  Show dependency treegraph
 
Reported: 2022-12-20 16:19 UTC by Owen Leonard
Modified: 2023-09-20 14:28 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Architectural enhancement in preparation for bootstrap 5 upgrade. This patch adds the foundations for abstracting the breadcrumb component of the staff client.
Version(s) released in:
23.05.00,22.11.04


Attachments
Bug 32507: Use template wrapper to build breadcrumb navigation (4.95 KB, patch)
2022-12-20 16:51 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32507: Use template wrapper to build breadcrumb navigation (5.00 KB, patch)
2022-12-21 01:02 UTC, David Nind
Details | Diff | Splinter Review
Bug 32507: (follow-up) Change parameter name to avoid collisions (2.30 KB, patch)
2023-01-05 18:51 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32507: Use template wrapper to build breadcrumb navigation (5.07 KB, patch)
2023-01-09 14:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32507: (follow-up) Change parameter name to avoid collisions (2.37 KB, patch)
2023-01-09 14:53 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32507: Use template wrapper to build breadcrumb navigation (6.06 KB, patch)
2023-02-08 16:48 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 32507: (follow-up) Change parameter name to avoid collisions (3.20 KB, patch)
2023-02-08 16:48 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Owen Leonard 2022-12-20 16:19:43 UTC
In preparation for an upgrade to Bootstrap 5 I propose that we try to abstract the way we build some components. This will help reduce the number of template changes required during the update.

Using Template::Toolkit WRAPPER we can create the main structure for breadcrumbs, currently:

    <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">
        <ol class="breadcrumb">
            ...
        </ol>
    </nav>

And also a structure for each navigation item:

    <li class="breadcrumb-item">
        ...
    </li>

This can be done gradually before the Bootstrap upgrade without causing any disruption.
Comment 1 Owen Leonard 2022-12-20 16:51:51 UTC Comment hidden (obsolete)
Comment 2 David Nind 2022-12-21 01:02:39 UTC
Created attachment 144754 [details] [review]
Bug 32507: Use template wrapper to build breadcrumb navigation

This patch lays the groundwork for updating the way we build breadcrumb
navigation on each page, and implements it on the cities administration
page as a proof of concept.

The page creates two blocks, 'breadcrumbs' and 'breadcrumb_item'

'breadcrumbs' builds the outer structure of the menu, currently:

    <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">
        <ol class="breadcrumb">

It automatically adds the "Home" link which appears on all pages.

The 'breadcrumb_item' block builds the list item which contains the
individual link.

To test, apply the patch and go to Administration -> Cities & towns. On
each variation of the page, check the breadcrumbs menu:

 - On the initial summary view it should be:
   Home -> Administration -> Cities
 - Click "New city" and the menu should be:
   Home -> Administration -> Cities -> New city
 - When you edit a city:
   Home -> Administration -> Cities -> Modify city
 - When you delete a city:
   Home -> Administration -> Cities -> Confirm deletion of city

Signed-off-by: David Nind <david@davidnind.com>
Comment 3 Owen Leonard 2023-01-05 18:51:56 UTC
Created attachment 145070 [details] [review]
Bug 32507: (follow-up) Change parameter name to avoid collisions

The variable name "active" is very likely to already be set for other
reasons, so changing the parameter name to "bc_active" to avoid
problems.
Comment 4 Martin Renvoize 2023-01-09 14:53:45 UTC
Created attachment 145147 [details] [review]
Bug 32507: Use template wrapper to build breadcrumb navigation

This patch lays the groundwork for updating the way we build breadcrumb
navigation on each page, and implements it on the cities administration
page as a proof of concept.

The page creates two blocks, 'breadcrumbs' and 'breadcrumb_item'

'breadcrumbs' builds the outer structure of the menu, currently:

    <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">
        <ol class="breadcrumb">

It automatically adds the "Home" link which appears on all pages.

The 'breadcrumb_item' block builds the list item which contains the
individual link.

To test, apply the patch and go to Administration -> Cities & towns. On
each variation of the page, check the breadcrumbs menu:

 - On the initial summary view it should be:
   Home -> Administration -> Cities
 - Click "New city" and the menu should be:
   Home -> Administration -> Cities -> New city
 - When you edit a city:
   Home -> Administration -> Cities -> Modify city
 - When you delete a city:
   Home -> Administration -> Cities -> Confirm deletion of city

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2023-01-09 14:53:49 UTC
Created attachment 145148 [details] [review]
Bug 32507: (follow-up) Change parameter name to avoid collisions

The variable name "active" is very likely to already be set for other
reasons, so changing the parameter name to "bc_active" to avoid
problems.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2023-01-09 14:54:50 UTC
Works for me, saves a few lines and is a good example of using the TT WRAPPER directive.

Passing QA
Comment 7 Tomás Cohen Arazi 2023-02-07 18:38:14 UTC
Conflict, please rebase.
Comment 8 Owen Leonard 2023-02-08 16:48:18 UTC
Created attachment 146400 [details] [review]
Bug 32507: Use template wrapper to build breadcrumb navigation

This patch lays the groundwork for updating the way we build breadcrumb
navigation on each page, and implements it on the cities administration
page as a proof of concept.

The page creates two blocks, 'breadcrumbs' and 'breadcrumb_item'

'breadcrumbs' builds the outer structure of the menu, currently:

    <nav id="breadcrumbs" aria-label="Breadcrumb" class="breadcrumb">
        <ol class="breadcrumb">

It automatically adds the "Home" link which appears on all pages.

The 'breadcrumb_item' block builds the list item which contains the
individual link.

To test, apply the patch and go to Administration -> Cities & towns. On
each variation of the page, check the breadcrumbs menu:

 - On the initial summary view it should be:
   Home -> Administration -> Cities
 - Click "New city" and the menu should be:
   Home -> Administration -> Cities -> New city
 - When you edit a city:
   Home -> Administration -> Cities -> Modify city
 - When you delete a city:
   Home -> Administration -> Cities -> Confirm deletion of city

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Owen Leonard 2023-02-08 16:48:20 UTC
Created attachment 146401 [details] [review]
Bug 32507: (follow-up) Change parameter name to avoid collisions

The variable name "active" is very likely to already be set for other
reasons, so changing the parameter name to "bc_active" to avoid
problems.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Tomás Cohen Arazi 2023-02-15 12:34:34 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 11 Pedro Amorim 2023-02-17 14:08:37 UTC
This will not be pushed to 22.11.x for now.
Comment 12 Martin Renvoize 2023-03-02 12:04:39 UTC
Many hands makes light work, thankyou everyone!

Pushed to 22.11.x for the next release
Comment 13 Martin Renvoize 2023-03-02 12:06:02 UTC
Backported to support future maintanence.. and we pushed bug 32973 which depends upon it so needed to push this too.
Comment 14 Lucas Gass 2023-03-10 14:47:36 UTC
Will not be backported to 22.05.x