Fix admin language selector with more than 4 locales by sergei-krylov · Pull Request #9519 · decidim/decidim

sergei-krylov · 2022-07-06T09:39:42Z

🎩 What? Why?

A few issues in admin panel when using more than 4 languages with questionnaires (decidim-forms):

language selector doesn't work for newly created fields.
the field disappears when there are errors in the form and you select a different language for this field.
some translations of language with error are not correct

First issue is that when you add

📌 Related Issues

Link your PR to an issue

Related to #?
Fixes Issues with the translations of forms #9105

Testing

Prerequisites:

in the /config/initializers/decidim.rb, the config.available_locales should contain more than 4 languages, so language selector will change from "tabs" style to "select" style.
login as admin
choose the site language that is different from default one for organization
create a Survey component for any assembly/process or whatever
try to add a new question to that survey and fill in all languages - here is issue 1
try to save, page will refresh with error (because you didn't fill the default language for a question)
try to change the language for any field for new question, the field disappears - here is issue 2
you can also notice that there is a wrong label for language with error for some languages (they mention "English" instead of the language itself).

📷 Screenshots

Video explaining all 3 issues
On that video the default language of the organization is :fr!

The field disappears when there are errors in the form and select a different language for this field.

ahukkanen

Thanks for the fix @sergei-krylov.

I have left few comments regarding some architectural issues this change would introduce as-is. Could you please fix the described issue?

Also, please revert all changes you have done in the locale files (*.yml) as you should only change the en.yml files when adding new strings, removing strings or modifying the source locale strings as explained at:
https://docs.decidim.org/en/contribute/translations.html

I went ahead and applied all the changes from this PR to Crowdin, so they should be fixed once we merge the Crowdin pull requests the next time (before the next releases).

decidim-admin/app/packs/src/decidim/admin/choose_language.js

decidim-forms/app/packs/src/decidim/forms/admin/forms.js

ahukkanen

One more thing I realized during the testing regarding this method. Could you see my comments below?

We are also trying to migrate slowly away from jQuery so we kindly ask for any new code to be implemented without jQuery and I think this would be rather simple to migrate. Could you take care of that too?

And with this I only mean the choose_language.js, no need to refactor the forms related JS, as that would be much more work (and would not be related to the bug being fixed, so it should be handled separately from this).

decidim-admin/app/packs/src/decidim/admin/choose_language.js

decidim-forms/app/packs/src/decidim/forms/admin/forms.js

This reverts commit cda01ac

sergei-krylov · 2022-07-11T09:46:22Z

Thank you very much for the review @ahukkanen !
And sorry for missing the translations thing, thanks for updating the Crowdin.

ahukkanen

Thanks for the refactoring @sergei-krylov, looking really good now!

I just left few more ideas regarding code cleanup and after these it's good to go for me!

decidim-forms/app/packs/src/decidim/forms/admin/forms.js

decidim-admin/app/packs/src/decidim/admin/choose_language.js

sergei-krylov · 2022-07-12T11:05:38Z

@ahukkanen , thanks for the tips!

ahukkanen · 2022-07-12T11:19:13Z

Thanks! There's still some ESLint issues after the latest changes, could you take a look at those? Suggesting also adding ESLint plugin to your code editor to see these while you write code.

decidim-admin/app/packs/src/decidim/admin/choose_language.js

Typo. Change back "click" to "change" Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

…ging * feature/redesign-main-footer: Reorder elements in main links of footer and define links and texts Define a cell for static_pages and topics configured to appear in footer Fix translation call Set fixed links in redesigned_main_legal partial Add FooterMenuPresenter to display menu items in footer Fix budgets seeds on non development apps (#9585) Return 404 when there isn't a valid component in program (#9576) Add missing queue close_meeting_reminder to sidekiq configuration (#9568) Make the HERE Map display in the currently selected language (#9552) Add help text for proposals' 'publish answers immediately' setting (#9549) Fix admin language selector with more than 4 locales (#9519) Fix publish event on official proposals (#9421) Prevent missing ActionLog entries to break the application (#9502) Add boilerplate structure to CHANGELOG (#9501) Add step-by-step instructions of the Crowdin releases process (#9555) Fix translated attributes field type change (#9547) Add `modifyList` option to the autocomplete element (#9548) Admin log filters (#9460) Improve the default gitignore files created by the generators (#9507)

* Fix language selector doesn't work for newly created fields. The field disappears when there are errors in the form and select a different language for this field. * Fix translations of language with errors for language selector. * Disable require-jsdoc * Revert "Fix translations of language with errors for language selector." This reverts commit cda01ac * Refactoring of onchange function calls, reimplement without jQuery. * Refactor select change event * Fix linters * Update decidim-admin/app/packs/src/decidim/admin/choose_language.js Typo. Change back "click" to "change" Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi> Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

sergei-krylov added 3 commits July 6, 2022 12:18

Fix language selector doesn't work for newly created fields.

8639695

The field disappears when there are errors in the form and select a different language for this field.

Fix translations of language with errors for language selector.

cda01ac

Disable require-jsdoc

e70bc5b

ahukkanen added module: admin module: forms type: fix PRs that implement a fix for a bug labels Jul 8, 2022

ahukkanen requested changes Jul 8, 2022

View reviewed changes

decidim-admin/app/packs/src/decidim/admin/choose_language.js Outdated Show resolved Hide resolved

decidim-forms/app/packs/src/decidim/forms/admin/forms.js Show resolved Hide resolved

ahukkanen changed the title ~~Fix/language selector~~ Fix admin language selector with more than 4 locales Jul 8, 2022

ahukkanen requested changes Jul 8, 2022

View reviewed changes

decidim-admin/app/packs/src/decidim/admin/choose_language.js Outdated Show resolved Hide resolved

decidim-forms/app/packs/src/decidim/forms/admin/forms.js Outdated Show resolved Hide resolved

sergei-krylov added 2 commits July 11, 2022 10:51

Revert "Fix translations of language with errors for language selector."

272caf2

This reverts commit cda01ac

Refactoring of onchange function calls, reimplement without jQuery.

10a4772

sergei-krylov requested a review from ahukkanen July 11, 2022 10:01

ahukkanen requested changes Jul 12, 2022

View reviewed changes

Refactor select change event

3b1ce03

sergei-krylov requested a review from ahukkanen July 12, 2022 11:05

Fix linters

4b4888d

ahukkanen reviewed Jul 12, 2022

View reviewed changes

decidim-admin/app/packs/src/decidim/admin/choose_language.js Outdated Show resolved Hide resolved

Update decidim-admin/app/packs/src/decidim/admin/choose_language.js

0c51c8a

Typo. Change back "click" to "change" Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

sergei-krylov requested a review from ahukkanen July 13, 2022 16:50

ahukkanen approved these changes Jul 13, 2022

View reviewed changes

ahukkanen merged commit 81a60f8 into decidim:develop Jul 13, 2022

This was referenced Aug 15, 2022

Backport 'Fix admin language selector with more than 4 locales' to v0.27 #9709

Merged

Backport 'Fix admin language selector with more than 4 locales' to v0.26 #9710

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix admin language selector with more than 4 locales#9519

Fix admin language selector with more than 4 locales#9519
ahukkanen merged 8 commits intodecidim:developfrom
belighted:fix/language-selector

sergei-krylov commented Jul 6, 2022

Uh oh!

ahukkanen left a comment

Uh oh!

Uh oh!

Uh oh!

ahukkanen left a comment

Uh oh!

Uh oh!

Uh oh!

sergei-krylov commented Jul 11, 2022

Uh oh!

ahukkanen left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergei-krylov commented Jul 12, 2022

Uh oh!

ahukkanen commented Jul 12, 2022

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

sergei-krylov commented Jul 6, 2022

🎩 What? Why?

📌 Related Issues

Testing

📷 Screenshots

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sergei-krylov commented Jul 11, 2022

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sergei-krylov commented Jul 12, 2022

Uh oh!

ahukkanen commented Jul 12, 2022

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants