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

Remove translations provided by ActiveModel #1354

Merged
merged 1 commit into from
Feb 13, 2018
Merged

Remove translations provided by ActiveModel #1354

merged 1 commit into from
Feb 13, 2018

Conversation

pelargir
Copy link
Contributor

@pelargir pelargir commented Jan 25, 2018

Alchemy defines several translations that are already defined by ActiveModel. One of these, the "confirmation" translation, is actually incorrect.

As of Rails 5, ActiveModel includes an "%{attribute}" variable reference in the translation, whereas Alchemy's copy simply has the word "confirmation." This results in poorly worded confirmation validation error messages.

For example, with a model defined this way:

class User < ActiveRecord::Base
  validates :password, confirmation: true
end

If the user doesn't provide a matching password confirmation, the validation error is:

"Password Confirmation doesn't match confirmation"

Whereas using the Rails 5 translation with the embedded "%{attribute}" variable gives:

"Password Confirmation doesn't match Password"

Which makes a whole lot more sense.

I've opted to simply remove the Alchemy translations entirely since these are already defined/provided by ActiveModel and there is really no need to redefine them again in Alchemy's translation file.

@pelargir pelargir changed the title WIP: Remove translations provided by ActiveModel Remove translations provided by ActiveModel Jan 26, 2018
@tvdeyen
Copy link
Member

tvdeyen commented Feb 13, 2018

@pelargir Thanks Matt! Would you mind to remove them from the other locale files as well?

@pelargir
Copy link
Contributor Author

@tvdeyen I hesitate to remove the other translations since ActiveModel only provides the English translation file.

@tvdeyen
Copy link
Member

tvdeyen commented Feb 13, 2018

As discussed IRL we agreed to not remove the other translations.

@tvdeyen tvdeyen merged commit 4927a21 into AlchemyCMS:master Feb 13, 2018
@pelargir pelargir deleted the fix-translation branch October 24, 2019 19:12
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