Adding dropdown option to locales list#6462
Conversation
0e6e2b2 to
430356d
Compare
430356d to
fc506d1
Compare
|
Code looks good to me, I'm deploying it to a staging app to check it out! |
|
Hi @decidim/product! We've discussed this issue via chat previously, but just in case here's a reminder of the problem: A client wants an installation with a lot of languages (>10 languages). For translatable fields, we currently use tabs, but this solution does not scale for a lot of languages. With this PR we propose useing a dropdown instead of tabs to select the correct language. Tabs will appear if an installation has 4 or less languages. If an installation has 5 or more languages, dropdowns will appear:
Note that the screenshot has multiple English locales. This is due to a translation problem, some languages don't have the correct name set up and they show "English" instead of the correct one. We've setup a staging application for you to try this feature out: https://decidim-staging-pr-158.herokuapp.com/admin/participatory_processes/dolores-dolorum/edit If you approve its behavior, then we'll ask for tech reviews! |
|
@mrcasals looks good! Since you are with it, could you correct the error in the language names as well? 🙏 |
|
@carolromero I suspect it's not an actual error, but keys not being (yet) translated. When a key is not translated in a given language, the default one (the one in English) appears. That's why we get "English" here. Apart from that, that'd be something to be done on Crowdin 😄 @decidim/core can you review this, please? 😄 |
|
@tramuntanal could you please take a look at this? |
|
Hi @decidim/product I don't know if this change is usable when the form has errors in some language. How will it be rendered? I think that it will be very difficult for the user to note that there's a mandatory translation missing for example.. @anaghavl how does this PR solves this situation? |
ea0b3dc to
4fbe87e
Compare
@tramuntanal missed this, will update the PR for the same. |
|
@mrcasals can you take a look at the latest commit? |
| content_tag(:select, id: tabs_id, class: "language-change") do | ||
| locales.each_with_index.inject("".html_safe) do |string, (locale, index)| | ||
| title = I18n.with_locale(locale) { I18n.t("name", scope: "locale") } | ||
| title += " (error!)" if error?(name_with_locale(name, locale)) |
There was a problem hiding this comment.
If wee go with this solution, the (error!) part should be using the I18n system! 😄
There was a problem hiding this comment.
Something like
title = if error?(name_with_locale(name, locale))
I18n.with_locale(locale) { I18n.t("name_with_error", scope: "locale") }
else
I18n.with_locale(locale) { I18n.t("name", scope: "locale") }
end|
Looking good, @anaghavl! Can you add some screenshots with an error to showcase it? |
|
Ah, the screenshot is in the description. @tramuntanal can you check this out, please? |
* adding dropdown option to locales list * fixing linter error * adding test cases * fixing PerceivedComplexity and CyclomaticComplexity * fixing js lint errors * disable no-invalid-this error * Simplify code * Simplify code * Adding error to identify locale with error * moving error to I18n Co-authored-by: Marc Riera Casals <mrc2407@gmail.com>
Adding an option to display locales in dropdown if the total number of locales are more than 4.
When the default locale isn't filled out,
