Bug 33000 - Use template wrapper for breadcrumbs: Acquisitions part 1
Summary: Use template wrapper for breadcrumbs: Acquisitions part 1
Status: CLOSED 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: 32507
Blocks: 33007
  Show dependency treegraph
 
Reported: 2023-02-17 17:43 UTC by Owen Leonard
Modified: 2023-12-28 20:42 UTC (History)
3 users (show)

See Also:
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


Attachments
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1 (19.35 KB, patch)
2023-02-17 17:57 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1 (19.38 KB, patch)
2023-02-21 20:48 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33000: (follow-up) Add spans for translatability (10.60 KB, patch)
2023-02-23 22:32 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 33000: (follow-up) Add spans for translatability (10.63 KB, patch)
2023-03-02 13:32 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 33000: Use template wrapper for breadcrumbs: Acquisitions part 1 (19.44 KB, patch)
2023-03-04 12:44 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33000: (follow-up) Add spans for translatability (10.69 KB, patch)
2023-03-04 12:44 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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