Bug 32507

Summary: Use template wrapper to build breadcrumb navigation
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: RESOLVED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: dcook, lucas, martin.renvoize, pedro.amorim, tomascohen
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34849
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
Circulation function:
Bug Depends on:    
Bug 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, 36945    
Attachments: Bug 32507: Use template wrapper to build breadcrumb navigation
Bug 32507: Use template wrapper to build breadcrumb navigation
Bug 32507: (follow-up) Change parameter name to avoid collisions
Bug 32507: Use template wrapper to build breadcrumb navigation
Bug 32507: (follow-up) Change parameter name to avoid collisions
Bug 32507: Use template wrapper to build breadcrumb navigation
Bug 32507: (follow-up) Change parameter name to avoid collisions

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 (ashimema) 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 (ashimema) 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 (ashimema) 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 (tcohen) 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 (tcohen) 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 (ashimema) 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 (ashimema) 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 (lukeg) 2023-03-10 14:47:36 UTC
Will not be backported to 22.05.x