Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Update in LocalizationExample #8828

Merged
merged 7 commits into from
Aug 30, 2014
Merged

Update in LocalizationExample #8828

merged 7 commits into from
Aug 30, 2014

Conversation

Denisov21
Copy link
Contributor

No description provided.

@Denisov21 Denisov21 changed the title Create package.json Update in LocalizationExample Aug 21, 2014
},
"i18n": [
"en",
"fr"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: these should be indented one more level, like the "engines" section above

@Denisov21
Copy link
Contributor Author

@peterflynn Done, you can merge!

@@ -48,6 +48,8 @@ Move this plugin to the extensions\user\ folder to run the plugin. It will add a

* main.js – loads the Strings module for the plugin and uses mustache to localize html content

* package.json - add the translation languages as in the example: `"i18n: ["en", "fr" ]`

* strings.js – uses i18n to load a strings.js file in the nls folder

* htmlContent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was an error from before. But since you are editing this file, would you mind fixing the indentation here? This and the next line should be indented 4 more spaces. Thanks

@Denisov21
Copy link
Contributor Author

@TomMalbran Done!

@ingorichter
Copy link
Contributor

@TomMalbran Can we merge this?

* nls
* strings.js – configures i18n by specifying the root folder and listing the locales supported by the plugin
* root
* htmlContent
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still not quite right. It should be:

    * htmlContent
        * htmlfragment.html – html template to be localized by mustache
    * nls
        * strings.js – configures i18n by specifying the root folder  and listing the locales supported by the plugin
        * root
            * strings.js – contains the English strings
        * fr
            * strings.js – contains the French strings
        * etc. for each locale

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, then we'll take it for Release 44. Thanks.

@TomMalbran
Copy link
Contributor

@ingorichter The folder list is still not quite right. That should be fixed. We can still merge it and fix it if you want it for the next release.

@@ -48,17 +48,19 @@ Move this plugin to the extensions\user\ folder to run the plugin. It will add a

* main.js – loads the Strings module for the plugin and uses mustache to localize html content

* package.json - add the translation languages as in the example: `"i18n: ["en", "fr" ]`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may point out that this step is optional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything here is optional. But if you are adding translations, is better if you add this.

@Denisov21
Copy link
Contributor Author

Hi @ingorichter, I made the changes suggested by @marcelgerber

@marcelgerber
Copy link
Contributor

The README is still not quite right (see the parsed version). Just copy and paste the markdown:

* htmlContent
    * htmlfragment.html – html template to be localized by mustache
* nls
    * strings.js – configures i18n by specifying the  root folder and listing the locales supported by the plugin
    * root
        * strings.js – contains the English strings
    * fr
        * strings.js – contains the French strings
    * etc. for each locale

@Denisov21
Copy link
Contributor Author

Hi @marcelgerber, Ready for a final check! Sorry for the error.

@marcelgerber
Copy link
Contributor

Looks good. (But I can't merge, so let's wait for @TomMalbran)

@TomMalbran
Copy link
Contributor

It looks good to me too. Lets merge it :)

TomMalbran added a commit that referenced this pull request Aug 30, 2014
Update in LocalizationExample
@TomMalbran TomMalbran merged commit 771c3e3 into adobe:master Aug 30, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants