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

Improve JS i18n #1730

Merged
merged 4 commits into from
Feb 23, 2017
Merged

Improve JS i18n #1730

merged 4 commits into from
Feb 23, 2017

Conversation

jhawthorn
Copy link
Contributor

@jhawthorn jhawthorn commented Feb 22, 2017

A few improvements to our (very basic) JS i18n

In c871eea @mtomov made the Handlebars t helper understand the dot-separated paths. 👍

This extracts that functionality to a Spree.t, helper, so that JS doesn't need to dig through the Spree.translations object manually.

Spree.t("thing.one.two")

This also adds a scope option similar to the one ruby's 18n.t accepts. This is especially useful from handlebars templates, which can't themselves concatenate the string keys.

Spree.t(order.state, {scope: "order_state"})
{{ t order.state scope="order_state" }}

Finally, this adds Spree.human_attribute_name, as well as ships the activerecord attribute translations to our JS.

Spree.human_attribute_name("spree/line_item", "description")

Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

This little code for a huge improvement 🍾

I have one little nit regarding logging

if (translation) {
return translation;
} else {
console.error("No translation found for " + key + ".");
Copy link
Member

Choose a reason for hiding this comment

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

I would consider to use a console.warn instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Done.

jhawthorn and others added 4 commits February 22, 2017 09:23
Previously Martin Tomov updated the Handlebars "t" helper to behave more
like the standard rails helper. Awesome!

This commit extracts that behaviour so it is available to all
javascripts via Spree.t
This allows us to conveniently use the activerecord attribute name
translations, which is our preferred convention for storing
translations, from JS.
This adds a scope option to our JS Spree.t similar to the one ruby's
`I18n.t` accepts. This is especially useful from handlebars templates,
which can't themselves concatenate the string keys.
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

Wow. 👍

@jhawthorn jhawthorn merged commit bc1e8e5 into solidusio:master Feb 23, 2017
@jhawthorn jhawthorn deleted the js-i18n branch February 23, 2017 18:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants