Added MOVE button translation key; improved italian translations #5180
Conversation
|
I added "active_admin.filters.predicates.eq" also to en.yml because it's not translated in my language (italian). In the first commit I added also the keys: Then I removed them because they are not defined in en.yml (for Travis) - shouldn't be added? |
config/locales/en.yml
Outdated
| filters: | ||
| buttons: | ||
| filter: "Filter" | ||
| clear: "Clear Filters" | ||
| predicates: | ||
| contains: "Contains" | ||
| eq: "equals" |
There was a problem hiding this comment.
Not sure but I think this a "native" ransack predicate so that's why we don't provide translation. It should be translated in ransack? @Fivell?
There was a problem hiding this comment.
You are right, "eq" is in Ransack translations ransack_en. I could make a PR to Ransack for italian lang
config/locales/en.yml
Outdated
| @@ -21,12 +21,14 @@ en: | |||
| has_many_new: "Add New %{model}" | |||
| has_many_delete: "Delete" | |||
| has_many_remove: "Remove" | |||
| move: "MOVE" | |||
There was a problem hiding this comment.
I guess we should keep the case consistent. I'm up for making this lowercase.
There was a problem hiding this comment.
I agree, I added it uppercase because in the source is uppercase
There was a problem hiding this comment.
This will change in #3862 but I agree we should have localized. That way I can use it in #3862 as alt text. Agree with @deivid-rodriguez on keeping case consistent. The text in the source will be removed anyway. Thanks!
lib/active_admin/form_builder.rb
Outdated
| @@ -99,8 +99,8 @@ def has_many_actions(has_many_form, builder_options, contents) | |||
| if builder_options[:sortable] | |||
| has_many_form.input builder_options[:sortable], as: :hidden | |||
|
|
|||
| contents << template.content_tag(:li, class: 'handle') do | |||
| "MOVE" | |||
| contents << template.content_tag(:li, class: 'handle sortable') do | |||
There was a problem hiding this comment.
Adding the "sortable" seems unrelated to this PR, but I think it makes sense to me.
There was a problem hiding this comment.
Don't add this class so just remove the change please. It has been changed in #3862.
There was a problem hiding this comment.
Yes :) I added it just to make easier to style it
There was a problem hiding this comment.
ook thanks :)
removed in the next commit
config/locales/it.yml
Outdated
| @@ -20,6 +20,7 @@ it: | |||
| has_many_new: "Aggiungi nuovo/a %{model}" | |||
| has_many_delete: "Rimuovi" | |||
| has_many_remove: "Rimuovi" | |||
| move: "sposta" | |||
There was a problem hiding this comment.
You want this to start uppercase I guess?
|
It's ready! Ideally you'd squash all commits into two, one for each sentence in the PR title:
I'd do it like this: git reset HEAD~7 # undo all commits and get the whole diff in your working tree
git add -p # and add interactively the changes for the first commit
git commit # commit the first set of changes
git commit -a # add and commit the rest of the changes
git push --force # force-push to the PR once you're sure everything is fine |
|
Understood. But I made only a commit. Hope it's ok anyway |
No description provided.