Description
Matt Blenkinsop
2023-04-05 11:55:14 UTC
Created attachment 149193 [details] [review] Bug 33417: Create standard toolbar component This commit introduces a standard Toolbar component that can be used throughout ERM Test plan: 1) Apply patch 2) Navigate to Agreements and click the "Add agreement" button - this should work as normal 3) Navigate to Licenses and repeat 4) Navigate to Local packages and repeat 5) Navigate to local titles - there should be two buttons, one to add and one to import. These should work as normal Created attachment 149194 [details] [review] Bug 33417: Add toolbar imports and options This commit amends the required files to import the toolbar and add the correct options Test plan as above Created attachment 149195 [details] [review] Bug 33417: Remove old components This commit deletes the old toolbar components as they are no longer necessary Test plan as above Not ready for signoff yet as 32932 needs a rebase and some fixes + {{ $__(`${option.button_title}`) }}</router-link This won't work, it won't be picked by the translate tool. You need to explicitly write $__("Your string") in the source code. For a Vue module I am writing I need to show/hide button from the toolbar depending on other states. Wondering how easy/hard it will be with this new implementation. (In reply to Jonathan Druart from comment #6) > For a Vue module I am writing I need to show/hide button from the toolbar > depending on other states. Wondering how easy/hard it will be with this new > implementation. My first thoughts would be that filtering the toolbar_options array would show/hide buttons, perhaps a toolbar_options and a filtered_toolbar_options would be necessary Created attachment 149212 [details] [review] Bug 33417: Fix translation in toolbar This commit allows the string to be translated using String.prototype.format() Test plan as above (In reply to Matt Blenkinsop from comment #8) > Created attachment 149212 [details] [review] [review] > Bug 33417: Fix translation in toolbar > > This commit allows the string to be translated using > String.prototype.format() > > Test plan as above Have you tried it? I haven't :) But I don't think it will work. Try that: Replace "New agreement" with something that is NOT in the PO already, then run the translate script => Make sure it's in the PO I am pretty sure it does not work and that you need to explicitly have $__("New agreement") in the code base. Created attachment 149215 [details] [review] Bug 33417: Fix translation in toolbar This commit allows the string to be translated - tested using fr-FR and a dummy string in the .po file (see image attached) Test plan as above Created attachment 149217 [details]
Translation example
Tested, and it does not work. % git --no-pager diff **/*.vue diff --git a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsList.vue b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsList.vue index 5fee4862f17..a2bca463341 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsList.vue +++ b/koha-tmpl/intranet-tmpl/prog/js/vue/components/ERM/AgreementsList.vue @@ -88,10 +88,12 @@ export default { toolbar_options: [ { to: "AgreementsFormAdd", - button_title: "New agreement", + button_title: "Here is a test string", }, ], + another_string: this.$__("Here is ANOTHER string"), } }, beforeRouteEnter(to, from, next) { next(vm => { % cd misc/translator % perl translate update fr-FR % git grep 'Here is .* string' misc/translator/po/fr-FR-* misc/translator/po/fr-FR-messages-js.po:msgid "Here is ANOTHER string" Matt, you are not supposed to add the string in the po file manually. What I meant previously is that the 'update' part of the translate process is picking the strings from the source code (tt, inc, js and vue files). To make it aware of the strings (and so new strings) you MUST use explicitly the __() function ($__() in vue files) If you didn't add the string manually in the po file then tell me which black magic you used ;) Sorry, translations are new to me! I'll try again Created attachment 149229 [details] [review] Bug 33417: Fix translation in toolbar This commit allows the string to be translated - tested using fr-FR and a dummy string in the .po file Test plan as above Created attachment 149936 [details] [review] Bug 33417: Create standard toolbar component This commit introduces a standard Toolbar component that can be used throughout ERM Test plan: 1) Apply patch 2) Navigate to Agreements and click the "Add agreement" button - this should work as normal 3) Navigate to Licenses and repeat 4) Navigate to Local packages and repeat 5) Navigate to local titles - there should be two buttons, one to add and one to import. These should work as normal Created attachment 149937 [details] [review] Bug 33417: Add toolbar imports and options This commit amends the required files to import the toolbar and add the correct options Test plan as above Created attachment 149938 [details] [review] Bug 33417: Remove old components This commit deletes the old toolbar components as they are no longer necessary Test plan as above Created attachment 149939 [details] [review] Bug 33417: Fix translation in toolbar This commit allows the string to be translated - tested using fr-FR and a dummy string in the .po file Test plan as above Created attachment 149964 [details] [review] Bug 33417: Create standard toolbar component This commit introduces a standard Toolbar component that can be used throughout ERM Test plan: 1) Apply patch 2) Navigate to Agreements and click the "Add agreement" button - this should work as normal 3) Navigate to Licenses and repeat 4) Navigate to Local packages and repeat 5) Navigate to local titles - there should be two buttons, one to add and one to import. These should work as normal Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 149965 [details] [review] Bug 33417: Add toolbar imports and options This commit amends the required files to import the toolbar and add the correct options Test plan as above Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 149966 [details] [review] Bug 33417: Remove old components This commit deletes the old toolbar components as they are no longer necessary Test plan as above Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 149967 [details] [review] Bug 33417: Fix translation in toolbar This commit allows the string to be translated - tested using fr-FR and a dummy string in the .po file Test plan as above Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> All working as expected and the translations working nicely now.. well spotted Jonathan. I'm happy to signoff. With Jonathan highlighting the translation points here I think we can call his comments a signoff.. my testing confirms all is working and I'm not seeing any regressions.. so I'm tempted to convert this to PQA. I am not sure, see comment 6. I would like to give it a try, but I am lacking time at the moment... Created attachment 152244 [details] [review] Bug 33417: Create standard toolbar component This commit introduces a standard Toolbar component that can be used throughout ERM Test plan: 1) Apply patch 2) Navigate to Agreements and click the "Add agreement" button - this should work as normal 3) Navigate to Licenses and repeat 4) Navigate to Local packages and repeat 5) Navigate to local titles - there should be two buttons, one to add and one to import. These should work as normal Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 152245 [details] [review] Bug 33417: Add toolbar imports and options This commit amends the required files to import the toolbar and add the correct options Test plan as above Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 152246 [details] [review] Bug 33417: Remove old components This commit deletes the old toolbar components as they are no longer necessary Test plan as above Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 152247 [details] [review] Bug 33417: Fix translation in toolbar This commit allows the string to be translated - tested using fr-FR and a dummy string in the .po file Test plan as above Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Sorry for the delay! (In reply to Jonathan Druart from comment #30) > Sorry for the delay! Thanks for reviewing, Jonathan! Pushed to master for 23.11. Nice work everyone, thanks! This is adding the 'plus' icon without making it configurable. Please see bug 34214. This is not flexible enough for the needs I have on bug 30708. I have attached an alternative version on bug 34215. Thanks for all the hard work! Pushed to 23.05.x for the next release Nice work everyone! Pushed to 22.11.x for next release |