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

Add translations for ActiveRecord models #180

Closed
wants to merge 1 commit into from
Closed

Add translations for ActiveRecord models #180

wants to merge 1 commit into from

Conversation

@mgrachev
Copy link
Contributor

@mgrachev mgrachev commented Nov 6, 2015

Hi. Added translation support for ActiveRecord models in the sidebar and on the index page (header).

File locale must be of such content:

en:
  activerecord:
    models:
      bet: Rates
@gracewashere
Copy link
Contributor

@gracewashere gracewashere commented Nov 21, 2015

Hey, @mgrachev! Sorry I haven't responded sooner.

Could you add some tests for this? Maybe something like this, in spec/features/sidebar_spec.rb?

it "honors model translations" do
  allow(I18n).to receive(:translate).with("customer").and_return("user")

  visit admin_customers_path

  sidebar = find(".sidebar__list")
  expect(sidebar).to have_link("Users")
  expect(page).to have_header("Users")
end
@@ -11,7 +11,7 @@ as defined by the DashboardManifest.
<% DashboardManifest::DASHBOARDS.each do |resource| %>
<li>
<%= link_to(
resource.to_s.titleize,
resource.to_s.classify.constantize.model_name.human(default: resource.to_s.titleize),

This comment has been minimized.

@gracewashere

gracewashere Nov 21, 2015
Contributor

Do we want to pluralize this?

This comment has been minimized.

@mgrachev

mgrachev Nov 23, 2015
Author Contributor

No. The user decides how to display the model name through a file locale. In default pluralization.

@@ -21,7 +21,9 @@ It renders the `_table` partial to display details about the resources.
[1]: http://www.rubydoc.info/gems/administrate/Administrate/Page/Table
%>

<% content_for(:title) { page.resource_name.pluralize.titleize } %>
<% content_for(:title) do
page.resource_name.classify.constantize.model_name.human(default: page.resource_name.pluralize.titleize)

This comment has been minimized.

@gracewashere

gracewashere Nov 21, 2015
Contributor

Maybe the pluralization should happen after the human(...) call? That way it would apply even if it didn't use the default.

This comment has been minimized.

@mgrachev

mgrachev Nov 23, 2015
Author Contributor

So better not to do. For example, for the Russian language the forced pluralization it looks like this:

# config/locales/ru.yml
ru:
  activerecord:
    models:
      bet: Ставки
[1] pry(main)> Bet.model_name.human(default: 'Bets').pluralize
=> Ставкиs
@@ -8,4 +8,14 @@

expect(active_link.text).to eq "Customers"
end

it "displays translated name of model" do
allow(Customer.model_name).to receive(:human).and_return('Users')

This comment has been minimized.

@houndci-bot

houndci-bot Nov 23, 2015

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.

@mgrachev
Copy link
Contributor Author

@mgrachev mgrachev commented Nov 23, 2015

Added test

@mgrachev
Copy link
Contributor Author

@mgrachev mgrachev commented Dec 2, 2015

Please, restart the build on CircleCI

@gracewashere
Copy link
Contributor

@gracewashere gracewashere commented Dec 6, 2015

@mgrachev The build is failing because of a Nokogiri security vulnerability, which I've fixed in #285. If you rebase on master, the build should pass and I'll be able to merge it in.

Thanks!

@gracewashere
Copy link
Contributor

@gracewashere gracewashere commented Dec 6, 2015

@mgrachev I've cleaned it up a bit and added more thorough tests in #298. Let's move discussion over there.

Thanks for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants