Bug 33000

Summary: Use template wrapper for breadcrumbs: Acquisitions part 1
Product: Koha Reporter: Owen Leonard <oleonard>
Component: TemplatesAssignee: Owen Leonard <oleonard>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, lucas, testopia
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.05.00,22.11.04
Bug Depends on: 32507    
Bug Blocks: 33007    
Attachments: Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1
Bug 33000: (follow-up) Add spans for translatability
Bug 33000: (follow-up) Add spans for translatability
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1
Bug 33000: (follow-up) Add spans for translatability

Description Owen Leonard 2023-02-17 17:43:41 UTC
To continue the process started by Bug 32507, this patch will update some templates in acquisitions so that they use the new template wrapper for breadcrumbs.

- acqui-home.tt
- addorderiso2709.tt
- basket.tt
- basketgroup.tt
- basketheader.tt
- booksellers.tt
- cancelorder.tt
- duplicate_orders.tt
- edi_ean.tt
- edifactmsgs.tt
- edimsg.tt
Comment 1 Owen Leonard 2023-02-17 17:57:28 UTC Comment hidden (obsolete)
Comment 2 Lucas Gass 2023-02-21 20:48:50 UTC
Created attachment 147094 [details] [review]
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1

This patch updates several acquisitions-related templates so that they
use the new WRAPPER for displaying breadcrumbs.

To test, apply the patch and test each page and its variations.
Breadcrumbs should look correct, and each link should be correct.

- Acquisitions home
  - Vendor search results
  - Basket details
    - Cancel order
    - Create EDIFACT order
    - Basket edit
    - Basket deletion confirmation
    - Add to basket
      - From a staged MARC file
      - From existing orders (copy)
  - Basket groups (create, edit)
  - EDIFACT messages

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 3 Jonathan Druart 2023-02-23 15:43:09 UTC
I think I am too late to the party, but I would like to raise a couple of things:

1. We should surround the strings with span, or the translation script will add awful strings with lot of %s.
Example:
. %1$s:  END.
#. %2$s:  WRAPPER breadcrumb_item bc_active=1.
#. %3$s:  END.
#. %4$s:  END.
#. %5$s:  IF ( else ).
#. %6$s:  WRAPPER breadcrumb_item bc_active=1.
#. %7$s:  END.
#. %8$s:  END.
#. %9$s:  END #/ WRAPPER breadcrumbs.
#. %10$s:  END #/ WRAPPER sub-header.inc.
#: koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt:69
msgid "%s %s Contract deleted %s %s %s %s Contracts %s %s %s %s "

2. In the same template (aqcontract), I see:

        [% IF ( add_validate ) %]
            [% WRAPPER breadcrumb_item %]
                <a href="/cgi-bin/koha/admin/aqcontract.pl?booksellerid=[% booksellerid | uri %]">Contracts</a>
            [% END %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Data recorded
            [% END %]
        [% END %]

        [% IF ( delete_confirm ) %]
            [% WRAPPER breadcrumb_item %]
                <a href="/cgi-bin/koha/admin/aqcontract.pl?booksellerid=[% booksellerid | uri %]">Contracts</a>
            [% END %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Confirm deletion of contract [% contractnumber | html %]
            [% END %]
        [% END %]

        [% IF ( delete_confirmed ) %]
            [% WRAPPER breadcrumb_item %]
                <a href="/cgi-bin/koha/admin/aqcontract.pl?booksellerid=[% booksellerid | uri %]">Contracts</a>
            [% END %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Contract deleted
            [% END %]
        [% END %]

        [% IF ( else ) %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Contracts
            [% END %]
        [% END %]

I can we should improve that we avoid repetition.
We should only need something like:

        [% WRAPPER breadcrumb_item remove_link=else%]
            <a href="/cgi-bin/koha/admin/aqcontract.pl?booksellerid=[% booksellerid | uri %]">Contracts</a>
        [% END %]
        [% IF ( add_validate ) %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Data recorded
            [% END %]
        [% END %]

        [% IF ( delete_confirm ) %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Confirm deletion of contract [% contractnumber | html %]
            [% END %]
        [% END %]

        [% IF ( delete_confirmed ) %]
            [% WRAPPER breadcrumb_item bc_active=1 %]
                Contract deleted
            [% END %]
        [% END %]


I don't know how hard it would be to implement 2, but 1 looks blocker to me.
Comment 4 Owen Leonard 2023-02-23 22:32:08 UTC
Created attachment 147275 [details] [review]
Bug 33000: (follow-up) Add spans for translatability

This patch finds places in the updated breadcrumbs markup where a
translatable string is isolated in a way that makes it hard for the
translation script to find it, and wraps these strings with <span>

This follow-up includes a change to admin/cities.tt as well.
Comment 5 Lucas Gass 2023-03-02 13:32:18 UTC
Created attachment 147615 [details] [review]
Bug 33000: (follow-up) Add spans for translatability

This patch finds places in the updated breadcrumbs markup where a
translatable string is isolated in a way that makes it hard for the
translation script to find it, and wraps these strings with <span>

This follow-up includes a change to admin/cities.tt as well.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 6 Katrin Fischer 2023-03-04 12:44:04 UTC
Created attachment 147746 [details] [review]
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1

This patch updates several acquisitions-related templates so that they
use the new WRAPPER for displaying breadcrumbs.

To test, apply the patch and test each page and its variations.
Breadcrumbs should look correct, and each link should be correct.

- Acquisitions home
  - Vendor search results
  - Basket details
    - Cancel order
    - Create EDIFACT order
    - Basket edit
    - Basket deletion confirmation
    - Add to basket
      - From a staged MARC file
      - From existing orders (copy)
  - Basket groups (create, edit)
  - EDIFACT messages

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2023-03-04 12:44:07 UTC
Created attachment 147747 [details] [review]
Bug 33000: (follow-up) Add spans for translatability

This patch finds places in the updated breadcrumbs markup where a
translatable string is isolated in a way that makes it hard for the
translation script to find it, and wraps these strings with <span>

This follow-up includes a change to admin/cities.tt as well.

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Tomás Cohen Arazi 2023-03-06 17:47:12 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 9 Matt Blenkinsop 2023-03-09 09:34:16 UTC
Nice work everyone!

Pushed to stable for 22.11.x
Comment 10 Lucas Gass 2023-03-14 20:13:37 UTC
Enhancement will not be backported to 22.05.x