Skip to content

Translation for validate messages #7593

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

Closed
wants to merge 3 commits into from
Closed

Translation for validate messages #7593

wants to merge 3 commits into from

Conversation

redelschaap
Copy link
Contributor

Added phrase functions to validate messages.

Added phrase function to validator message.
Added phrase function to validate messages.
@vasilii-b
Copy link

This is a must-to-merge pull request!

@avoelkl
Copy link
Contributor

avoelkl commented Mar 15, 2017

Hi @redelschaap, Thanks for your PR!
I am currently looking into the update to jquery.validate.js as it is an external library that was modified and how we deal with it.
I'd love to merge the update to form.phtml as soon as possible though.

@avoelkl
Copy link
Contributor

avoelkl commented Mar 15, 2017

Hi @redelschaap,
I was talking to the Magento team about this this and changes to an external library as it is the case with jquery.validate.js should not be allowed.
If you look at the history of the file, the translation was added and removed again (see for example https://github.com/redelschaap/magento2/commits/47678008e826612ff00946354e457ec168fe5280/lib/web/jquery/jquery.validate.js).
So we need to work around this.

If you want you can create a seperate PR with the form.phtml so that this can be merged already until we find a solution for the jquery.validate.js.
Do you have an idea how this could be resolved?

@redelschaap
Copy link
Contributor Author

@avoelkl I noticed the jquery.validate project has a localization folder which isn't included in Magento. See https://github.com/jquery-validation/jquery-validation/tree/master/src/localization. The solution would be to include all language files and pick the right file when deploying the theme for a specific language.

But I guess that sort of behaviour isn't yet included in Magento 2? And at this moment I have no idea how that process works, so I can't provide any help here. Until that time, can my PR be a temporary soluation, optionally with a todo mention?

@Igloczek
Copy link
Contributor

I almost fixed this issue, but still struggle with extending objects (variables scope stuff probably).
Commit - Igloczek@2fdb9ba

@redelschaap
Copy link
Contributor Author

@Igloczek Looks good, does it work when you extend the $.validator.messages object directly? Like this:

$.extend($.validator.messages, {
    ....
});

@Igloczek
Copy link
Contributor

Igloczek commented Mar 17, 2017

I tried different options and as long as I'm not extending $ directly, changes are not visible outside module with translations. Unfortunately $.validator isn't just an object, it's more like a class, so I can't simply extend it this way, b/c it will remove constructor, methods etc.

@redelschaap
Copy link
Contributor Author

When looking at the lib/web/jquery folder, I see two forms of adding functionality to jQuery:

lib/web/jquery/jquery.validate.js:

(function (factory) {
    if (typeof define === 'function' && define.amd) {
        define([
            "jquery",
            "jquery/jquery.metadata"
        ], factory);
    } else {
        factory(jQuery);
    }
}(function (jQuery) {
    (function ($) {
        $.extend(
            ....
        );
    }(jQuery));
}));

and lib/web/jquery/jquery-ui.js:

(function( $, undefined ) {
    $.ui = $.ui || {};

    $.extend( $.ui, {
        ...
    });
})( jQuery );

I guess the second example can be usefull here when you say you have to use the actual variable instead of getting jQuery from define.

@Igloczek
Copy link
Contributor

Igloczek commented Mar 17, 2017

It's against AMD rules, I can't assume that something is already loaded somewhere.
It works, b/c M probably added this lib to the shim part of require.js config, but that's not a proper way of writing modules.

But at all, I tried it too now and it doesn't work.

There is also one more issue - how to get data from translations pack.
To use $.mage.__() we have to register translations manually, though $.mage.translate.add(), which means we need some link between backend and frontend.

@redelschaap
Copy link
Contributor Author

I did not know that, I'm not that into AMD.

I use the i18n:collect and i18n:pack commands on the command line to create translation files, which scans all files for translatable strings, including strings within $.mage.__().

@Igloczek
Copy link
Contributor

There is also related issue #5278.

@Igloczek
Copy link
Contributor

Igloczek commented Mar 19, 2017

I find out that this issue is already fixed and merged to develop branch (#5725 -
https://github.com/redelschaap/magento2/commit/3109c8e96bcd08041f558be65a2a4e3d213d22a4), so probably we just have to wait for 2.2 release and can easily changes in JS from this PR.

@okorshenko
Copy link
Contributor

Thank you @avoelkl , @Igloczek and @redelschaap for collaboration.
We are closing this PR taking into account latest comment from @Igloczek

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.

6 participants