Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JS translation search #10445

Merged
merged 5 commits into from
Aug 18, 2017
Merged

Fix JS translation search #10445

merged 5 commits into from
Aug 18, 2017

Conversation

AntonEvers
Copy link
Contributor

@AntonEvers AntonEvers commented Aug 6, 2017

Description

  • Add support for jQuery.mage.__('translate me') as well as $.mage.__('translate me')
  • Add support for $.mage.__('Don\'t break on escape characters before \' or "')
  • Add support for $.mage.__('Concatenating strings' + 'within a tranlation' + 'function')

Fixed Issues (if relevant)

  1. JS Translation Regex leads to unexpected results and untranslatable strings #7403: JS Translation Regex leads to unexpected results and untranslatable strings
  2. Translate messages on password strength #5509: Translate messages on password strength
  3. js validation messages translation not working in customer account #5820: js validation messages translation not working in customer account
  4. Some messages in Customer Account Create not translated #9967: Some messages in Customer Account Create not translated

Manual testing scenarios

  1. See mentioned tickets for instances that could not be translated so far and translate them
  2. Flush caches, clear browser storage, hard refresh and see translated strings

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@ishakhsuvarov ishakhsuvarov self-assigned this Aug 7, 2017
@ishakhsuvarov ishakhsuvarov added this to the August 2017 milestone Aug 7, 2017
@ishakhsuvarov
Copy link
Contributor

@ajpevers Would that still variables provided to the translation function? Would you like to implement some tests to verify these cases and keep those functional in the future?

@magento-team magento-team merged commit 5ae47df into magento:develop Aug 18, 2017
magento-team pushed a commit that referenced this pull request Aug 18, 2017
[EngCom] Public Pull Requests
 - MAGETWO-71659: Fix for url_rewrite on page delete via api #10568
 - MAGETWO-71617: Fixes Regression in 2.2 - menu.xml config ignored #10543
 - MAGETWO-71380: Fix JS translation search #10445
 - MAGETWO-71201: Improved calculating version hash for the js-translation.json file. #10378
@maaarghk
Copy link

maaarghk commented Sep 7, 2017

I get an error on the core file i18n.js before and after this is merged, see the following -
https://regex101.com/r/AluQPA/1

@AntonEvers
Copy link
Contributor Author

@maaarghk Good point. How on earth are we going to find out which strings will go into the original variable. It's hard to translate those properly at this moment...

@ishakhsuvarov
Copy link
Contributor

@ajpevers @maaarghk I assume we have to find a solution to this case or revert the original fix. Do you have any ideas?

@AntonEvers
Copy link
Contributor Author

AntonEvers commented Sep 22, 2017

@maaarghk If I read your comment right, the error happens before, as well as after this commit. So this PR has nothing to do with it, right?

@ishakhsuvarov As for finding JS translations in Javascript files using PHP, the way we currently look for translatable strings is not very robust. It only takes literal strings into account and the regular expressions used were (are?) not covering all instances of literal strings.

Apart from that, inline translations or any translation that is saved in the translation table is not taken into account with JS translations, because they are store bound and static file generation only knows languages and not store views.

In my opinion the only real solution is to revise the way js-translation.json is being compiled. Maybe a dynamic way that looks for translations once they are passed to the JS function and retrieves them from the php translation cache if they don't exist in js translation. This is just an idea, but optimising the current setup is not going to lead to a 100% working solution.

magento-team pushed a commit that referenced this pull request Oct 10, 2017
Fixed issues:
- MAGETWO-71552: Attribute values on store view level not searchable - for 2.2
- MAGETWO-72866: Redundant indexers invalidation - RIATCS-340
- MAGETWO-75458: [Backport] - Fix overwrite default value image/file with NULL #10253 - for 2.2
- MAGETWO-75460: [Backport] - LowestPriceOptionsProvider returns products without attributes which are used for price calculation (e.g. tax adjustment)
- MAGETWO-80193: [2.2.x] - Add cast to string for CUST_GROUP_ALL #10475
- MAGETWO-80204: [2.2.x] - Grammar fix for #9533 #10627
- MAGETWO-71549: Impossible to export Advanced Prices on a medium profile
- MAGETWO-80198: [2.2.x] - Fix issue #10565 #10575
- MAGETWO-80197: [2.2.x] - Fix JS translation search #10445
- MAGETWO-80195: [2.2.x] - Send different base currency in Google analytics #10508
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants