-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix custom translations with options #3959
Conversation
8eb4b0e
to
ff64afe
Compare
@@ -12,7 +12,15 @@ def t(key, options = {}) | |||
i18_content = I18nContent.find_by(key: key) | |||
translation = I18nContentTranslation.find_by(i18n_content_id: i18_content&.id, | |||
locale: current_locale)&.value | |||
translation.presence || translate(key, options) | |||
if translation.present? | |||
if options.present? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this condition is necessary 🤔. What do you think about:
if translation.present?
translation % options
else
translate(key, options)
end
We could also write translation.presence&.%(options) || translate(key, options)
but IMHO it would be hard to understand it 😅.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used the first option that seems simpler. The second one is also very hard to understand for me. 😂
ff64afe
to
af1e118
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 😄.
References
This PR resolves #3676.
Objectives
There was a bug when an admin user tries to change a text on the "Custom information text" section
/admin/site_customization/information_texts
with the keys that include some%{variable}
in the text.Screenshot
BEFORE
AFTER