The staff side wrapper template for tool plugins needs an update to create breadcrumbs using the template wrapper for breadcrumbs.
Created attachment 153623 [details] [review] Bug 34307: Update plugin template to match master conventions This change uses WRAPPER to build the breadcrumbs and updates the plugin template to follow some other master conventions like differentiating between Administration and Tools for plugins. Test plan: 0. Apply patch 1. Upload koha-plugin-test-wrapper 2. Enable the plugin 3. Click "Actions" and click "Run tool" 4. Note how the plugin page looks like a perfect Koha Tools page 5. Note that the plugin only contains 6 lines of template code to achieve this effect
Created attachment 153624 [details] test plugin
Created attachment 154324 [details] [review] Bug 34307: Update plugin template to match master conventions This change uses WRAPPER to build the breadcrumbs and updates the plugin template to follow some other master conventions like differentiating between Administration and Tools for plugins. Test plan: 0. Apply patch 1. Upload koha-plugin-test-wrapper 2. Enable the plugin 3. Click "Actions" and click "Run tool" 4. Note how the plugin page looks like a perfect Koha Tools page 5. Note that the plugin only contains 6 lines of template code to achieve this effect Signed-off-by: David Nind <david@davidnind.com>
> 4. Note how the plugin page looks like a perfect Koha Tools page Is there anything to do or check about the left menu? Or is that normal that plugins can't insert themselves there? Or maybe they can and it has nothing to do with wrapper-staff-tool-plugin.inc ?
Created attachment 154673 [details] [review] Bug 34307: Update plugin template to match master conventions This change uses WRAPPER to build the breadcrumbs and updates the plugin template to follow some other master conventions like differentiating between Administration and Tools for plugins. Test plan: 0. Apply patch 1. Upload koha-plugin-test-wrapper 2. Enable the plugin 3. Click "Actions" and click "Run tool" 4. Note how the plugin page looks like a perfect Koha Tools page 5. Note that the plugin only contains 6 lines of template code to achieve this effect Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 154674 [details] [review] Bug 34307: (QA follow-up) add <span> to have cleaner strings for i18n Cleaner strings are extracted this way.
It works! So second signoff. I've reached for help about reviewing the code. follow-up to improve i18n But there is something weird, there is a part I couldn't change to improve the extracted strings. I did this: ``` <title> [% IF ( plugin_title ) %][% plugin_title | html %] › [% END %] <span>Plugins</span> › [% IF ( method == 'tool' ) %] <span>Tools</span> [% ELSE %] <span>Administration</span> [% END %] › <span>Koha</span> </title> ``` and got this as a diff of the translation files ``` misc/translator/Koha-staff-prog.pot +msgid "%s%s › %s <span>Plugins</span> › %s <span>Tools</span> %s <span>Administration</span> %s › <span>Koha</span> " misc/translator/po/fr-FR-staff-prog.po +"%s%s › %s <span>Plugins</span> › %s <span>Tools</span> %s " +"<span>Administration</span> %s › <span>Koha</span> " +msgstr "Plugins › Outils › Koha " ``` But I see in the diff that I have individual occurrences of Plugins, Tools, Administration, Koha linked with wrapper-staff-tool-plugin.inc:7 So it's liked it worked, and didn't work at the same time. I checked and the bad occurences aren't in the commented out section of the .po. And they contain the <span> so it's not the remains of the previous version. Any hint?
(In reply to Victor Grousset/tuxayo from comment #4) > > 4. Note how the plugin page looks like a perfect Koha Tools page > > Is there anything to do or check about the left menu? Or is that normal that > plugins can't insert themselves there? Or maybe they can and it has nothing > to do with wrapper-staff-tool-plugin.inc ? Doesn't really have anything to do with the wrapper.
(In reply to Victor Grousset/tuxayo from comment #7) > But I see in the diff that I have individual occurrences of Plugins, Tools, > Administration, Koha linked with wrapper-staff-tool-plugin.inc:7 > > So it's liked it worked, and didn't work at the same time. I checked and the > bad occurences aren't in the commented out section of the .po. > And they contain the <span> so it's not the remains of the previous version. > > Any hint? I have no idea about translations. I just followed the pattern that I saw.
Created attachment 154678 [details] [review] Bug 34307: (QA follow-up) add <span> to have cleaner strings for i18n Cleaner strings are extracted this way. Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 154679 [details] [review] Bug 34307: (follow-up) Use template wrapper for title tag This patch updates the title tag to use a wrapper too, since Bug 33906 is in the process of getting fixed. The patch also removes an extra <span> tag: Only the translatable string needs the span, not the entire line.
Created attachment 154683 [details] [review] Bug 34307: (follow-up) Use template wrapper for title tag This patch updates the title tag to use a wrapper too, since Bug 33906 is in the process of getting fixed. The patch also removes an extra <span> tag: Only the translatable string needs the span, not the entire line. Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
i18n is nice now, thanks Owen for the follow-up :D And thanks for confirming that the code is ok and the adjustments. So we are good to go: it works, makes sense, QA script happy, code looks good, passing QA :)
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.04
Dependency missing in 22.11.x - not backporting Nice work everyone!