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 and improve I18n for application templates #1004

Merged
merged 3 commits into from
Dec 4, 2017
Merged

Add and improve I18n for application templates #1004

merged 3 commits into from
Dec 4, 2017

Conversation

buren
Copy link
Contributor

@buren buren commented Oct 16, 2017

I finally got around replacing/updating #785, which adds better I18n support for administrate.


  • Removes "hardcoded" placement of resource name for edit/new/show links (problematic on Arabic and other right-to-left languages since the positioning of the name will be flipped too).
  • Update config/locale/administrate.en.yml
  • Includes the latest commit from master
  • Use display_resource_name consistently when calling page.resource_name in view templates
  • Update all other locale files
    • Any comments/suggestions on how I best should go about this?
    • If I use i18n-tasks add-missing other updates (formatting etc) are made in the locale files
    • Should we try to re-use the existing strings or do we deem that dangerous and think its better that translations are re-done by someone who actually understands the language? 😅

⚠️ There is one expected test failure for rspec ./spec/i18n_spec.rb:9 # I18n does not have missing keys (see TODO)

@hanneskaeufler
Copy link
Contributor

I improved the german translation in a similar pr. See #1002. To preserve the existing behaviour for all other locales I reused the existing translation and tackes on the resource token. But surely this is not optimal for many more locales other then german as well.

@buren
Copy link
Contributor Author

buren commented Oct 23, 2017

tl;dr Updated locales, tests are passing 🎉

I've updated non-default locales now I ended up including most of the locale changes @hanneskaeufler did in #1002. Thanks for that! Saved me a lot of time my editor didn't play nice with the arabic strings :)

@hanneskaeufler I didn't include your German translation update (keys: search.label, search.clear). Thought that would be better to put in a separate PR.

@nickcharlton let me know if there are any changes you want and if I should squash all the commits together as one :)

@nickcharlton
Copy link
Member

Hi @buren!

This is looking good to me. I like the way the commits are split up, so I'm not going to squash them here.

What do you think about me merging #1002, then you rebasing on top of that? It'll solve the separate PR issue you mention (I think?).

@buren
Copy link
Contributor Author

buren commented Oct 31, 2017

@nickcharlton perhaps merging this and then just opening a new PR with just the 2-line German fix? If @hanneskaeufler is unavailable to do it, I can :) Then the I18n fix for German would be as clean as possibly can be.

@hanneskaeufler
Copy link
Contributor

Sure go ahead and merge this and I'll update/reopen something similar to #1002.

@buren
Copy link
Contributor Author

buren commented Nov 9, 2017

@nickcharlton something blocking this? :)

Can I do anything to help?

@nickcharlton
Copy link
Member

@buren Just my time to hit the magic button!

I'm going to merge this now. Thanks for all of your time on it!

@hanneskaeufler if you could go ahead and do the German language fix in a new PR, that'd be wonderful.

@nickcharlton nickcharlton merged commit 6661889 into thoughtbot:master Dec 4, 2017
@buren
Copy link
Contributor Author

buren commented Dec 4, 2017

@nickcharlton 🎉 Sweat!

Thank you!

@buren buren deleted the i18n branch December 4, 2017 21:32
nickcharlton added a commit that referenced this pull request Dec 21, 2017
In #1004 the handling of translation strings improved, but bought in a
change to the buttons for new resources. This started to pluralise them
when they shouldn't be.

So you'd have "New Line items", not "New Line item". This reverts that
change.
nickcharlton added a commit that referenced this pull request Jan 12, 2018
In #1004 the handling of translation strings improved, but bought in a
change to the buttons for new resources. This started to pluralise them
when they shouldn't be.

So you'd have "New Line items", not "New Line item". This reverts that
change.
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.

3 participants