Fix i18n translation for Store link in admin header for fr
#2649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kristinalim
added
the
bug-s4
sauloperez
reviewed
Sep 6, 2018
| @@ -11,4 +11,4 @@ | |||
| = link_to t(:logout), spree.logout_path | |||
| %li{"data-hook" => "store-frontend-link"} | |||
| %i.icon-external-link | |||
| = link_to t(:store), spree.root_path, :target => '_blank' | |||
| = link_to t("admin.header.store"), spree.root_path, target: "_blank" | |||
There was a problem hiding this comment.
could you check if Rails' lazy lookup would work with t('.header.store') here? Using lazy lookup is one of the conventions we have for i18n. See https://github.com/openfoodfoundation/openfoodnetwork/wiki/Internationalisation-%28i18n%29#development
There was a problem hiding this comment.
Thanks, @sauloperez. I updated this, and it's now spree.layouts.admin.header.store (but referencing through .header.store). Right now, I'm questioning whether we should still have spree there, but I find it acceptable.
kristinalim
force-pushed
the
translations-fix_store_link_in_admin_header
branch
from
9124403 to
0d94b5e
Compare
spree_i18n translates the "store" key in different ways, sometimes like "shop" and sometimes like "save". This could be because of a clash in usage between Spree add-ons. To be more specific, the "Store" link in the admin section now uses the "admin.header.store" translation key.
kristinalim
force-pushed
the
translations-fix_store_link_in_admin_header
branch
from
0d94b5e to
d06bccb
Compare
sauloperez
approved these changes
Sep 6, 2018
luisramos0
approved these changes
Sep 6, 2018
|
Testing Notes
https://docs.google.com/document/d/1-oOc-NcwLtGnlGrrAfAy3vmQ6QvNAb-tP3KevnHEVCE/edit# |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Use more specific translation key for the "Store" link in the admin header
What? Why?
Closes #2632
We use the
spree_i18nfor base Spree translations.Its language translations for the "store" key is sometimes like "shop" and sometimes like "save". It's possible there is a clash in usage between Spree add-ons, so websites have been using it in different ways.
To make this more specific and to allow custom translation, I changed the "Store" link in the admin section to use the new translation key "admin.header.store".
What should we test?
Go to admin section and check the "Store" (
en) link in header.This should be properly translated in non-English instances, especially for France for which the correct translation will not match the one used before this PR.
Release notes
Changelog Category: Fixed