Support translations for ActiveRecord models #298

c-lliope · 2015-12-06T17:38:11Z

Problem:

Developers are unable to display their ActiveRecord classes in the UI by
any other name than the titleized class name.

For example, if a team is building an admin dashboard that will be used in
Russian, they may have ActiveRecord models with English names, that need to
be translated before they're displayed in the UI.

Solution:

Rails has support for translating ActiveRecord Models, which relies on
defining I18n translations under the activerecord.models namespace.

Rails expects translations in the form:

en:
 activerecord:
   models:
     user:
       one: Customer
       other: Customers

The separate one and other keys allow Rails to display the correct
plural or singular form of the word, depending on the context.

We've used Rails' built-in ActiveModel::Name#human method to correctly pull
the translations out of I18n, with sane fallbacks if the translation is not
defined.

houndci-bot · 2015-12-06T17:38:18Z

spec/helpers/administrate/application_helper_spec.rb

+              customer: {
+                one: "User",
+                other: "Users",
+              }


Put a comma after the last item of a multiline hash.

c-lliope · 2015-12-06T17:47:44Z

This was continued from the work in #180.

mgrachev · 2015-12-06T20:17:35Z

👍

danbee · 2015-12-07T08:38:34Z

app/helpers/administrate/application_helper.rb

@@ -4,5 +4,17 @@ def render_field(field, locals = {})
      locals.merge!(field: field)
      render locals: locals, partial: field.to_partial_path
    end
+
+    def display_resource_name(resource_name)
+      resource_name.


Should this just be resource?

I would think that "resource" would refer to the whole model object, but that's just me.

Yeah, I'm siding with @mcmire on this. I've been using resource to refer to instances of the ActiveRecord models. Here, we're referring to the class names of the ActiveRecord models.

I think if we end up going with @mcmire's suggestion in #298 (comment), then we should rename this to resource_class or something.

danbee · 2015-12-07T08:39:30Z

LGTM! 👍

JoelQ · 2015-12-07T15:17:16Z

app/helpers/administrate/application_helper.rb

+        constantize.
+        model_name.
+        human(
+          count: 0,


Is this always 0? how do we handle plurals?

@JoelQ this dictates how the resource names show up in the sidebar, and in the header on the index page. I think in both of those cases, we always want the displayed name to be in the plural form.

I started to code out an argument to this function that would let you specify a custom count, but I think YAGNI applies to that. If we need to display the singular form in the future, we can adjust this method accordingly.

mcmire · 2015-12-07T17:05:39Z

I had one comment, and I would also agree with Joël about why count: 0 is being used above. Other than these two things I'm good with this.

JoelQ · 2015-12-07T18:42:42Z

spec/features/sidebar_spec.rb

+    with_translations(:en, translations) do
+      visit admin_customers_path
+
+      sidebar = find(".sidebar__list")


Any reason this has double underscores?

We're using BEM syntax for our CSS. In this case, the CSS class would read as: "The list element inside the sidebar block", or something like that.

CSS selector formats are something that we could have a whole other discussion on - I'm still not sure what the best solution is for Administrate. I chose BEM because many designers at thoughtbot have had pleasant experiences working with it, and it makes it a bit easier to write maintainable (S)CSS.

JoelQ · 2015-12-07T18:43:54Z

One comment on a CSS class name, otherwise LGTM 🚀

Problem: Developers are unable to display their ActiveRecord classes in the UI by any other name than the titleized class name. For example, if a team is building an admin dashboard that will be used in Russian, they may have ActiveRecord models with English names, that need to be translated before they're displayed in the UI. Solution: Rails has support for [translating ActiveRecord Models][1], which relies on defining I18n translations under the `activerecord.models` namespace. Rails expects translations in the form: ```yaml en: activerecord: models: user: one: Customer other: Customers ``` The separate `one` and `other` keys allow Rails to display the correct plural or singular form of the word, depending on the context. We've used Rails' built-in `Foo.model_name.human` method to correctly pull the translations out of I18n, with sane fallbacks if the translation is not defined. [1]: http://guides.rubyonrails.org/i18n.html#translations-for-active-record-models

mgrachev · 2015-12-07T19:39:46Z

🎉

Changes: ``` * [#251] [FEATURE] Raise a helpful error when an attribute is missing from `ATTRIBUTE_TYPES` * [#298] [FEATURE] Support ActiveRecord model I18n translations * [#312] [FEATURE] Add a `nil` option to `belongs_to` form fields * [#231] [UI] Fix layout issue on show page where a long label next to an empty value would cause following fields on the page to be mis-aligned. * [#309] [UI] Fix layout issue in datetime pickers where months and years would not wrap correctly. * [#306] [UI] Wrap long text lines (on word breaks) on show pages * [#214] [UI] Improve header layout when there is a long page title * [#198] [UI] Improve spacing around bottom link in sidebar * [#206] [UI] Left-align checkboxes in boolean form fields * [#315] [UI] Remove the `IDS` suffix for `HasMany` form field labels * [#259] [BUGFIX] Make installation generator more robust by ignoring dynamically generated, unnamed models * [#243] [BUGFIX] Fix up a "Show" button on the edit page that was not using the `display_resource` method. * [#248] [BUGFIX] Improve polymorphic relationship's dashboard class detection. * [#247] [BUGFIX] Populate `has_many` and `belongs_to` select boxes with the current value of the relationship. * [#217] [I18n] Dutch * [#263] [I18n] Swedish * [#272] [I18n] Danish * [#270] [I18n] Don't apologize about missing relationship support. * [#237] [I18n] Fix broken paths for several I18n files (de, es, fr, pt-BR, vi). * [#266] [OPTIM] Save a few database queries by using cached counts ```

c-lliope added the In Review label Dec 6, 2015

houndci-bot reviewed Dec 6, 2015
View reviewed changes

c-lliope mentioned this pull request Dec 6, 2015

Add translations for ActiveRecord models #180

Closed

danbee reviewed Dec 7, 2015
View reviewed changes

JoelQ reviewed Dec 7, 2015
View reviewed changes

c-lliope force-pushed the gw-activerecord-translations branch from 14702eb to c75b572 Compare December 7, 2015 18:57

c-lliope merged commit c75b572 into master Dec 7, 2015

c-lliope deleted the gw-activerecord-translations branch December 7, 2015 18:58

c-lliope removed the In Review label Dec 7, 2015

c-lliope added this to the v0.1.2 milestone Dec 7, 2015

This was referenced Dec 9, 2015

Release Administrate v 0.1.2 #316

Merged

Translations of the resources used in Administrate #319

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support translations for ActiveRecord models #298

Support translations for ActiveRecord models #298

c-lliope commented Dec 6, 2015

houndci-bot Dec 6, 2015

c-lliope commented Dec 6, 2015

mgrachev commented Dec 6, 2015

danbee Dec 7, 2015

mcmire Dec 7, 2015

c-lliope Dec 7, 2015

danbee commented Dec 7, 2015

JoelQ Dec 7, 2015

c-lliope Dec 7, 2015

mcmire commented Dec 7, 2015

JoelQ Dec 7, 2015

c-lliope Dec 7, 2015

JoelQ Dec 7, 2015

JoelQ commented Dec 7, 2015

mgrachev commented Dec 7, 2015

Support translations for ActiveRecord models #298

Support translations for ActiveRecord models #298

Conversation

c-lliope commented Dec 6, 2015

Problem:

Solution:

Choose a reason for hiding this comment

c-lliope commented Dec 6, 2015

mgrachev commented Dec 6, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danbee commented Dec 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcmire commented Dec 7, 2015

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoelQ commented Dec 7, 2015

mgrachev commented Dec 7, 2015