It would be a very small but nice usability enhancement if when uploading a .po file on admin/config/regional/translate/import it could auto-select 'Russian' if I was uploading a file named token-7.x-1.2.ru.po.

CommentFileSizeAuthor
#19 AutoLanguage.png134.44 KBgábor hojtsy
#16 core-locale-import-autoselect-1744228-15.patch3.14 KBnod_
#11 interdiff-10-11.txt668 bytespenyaskito
#11 core-locale-import-autoselect-1744228-11.patch3.08 KBpenyaskito
#10 core-locale-import-autoselect-1744228-10.patch3.08 KBnod_
#6 1744228-po-import-autoselect-language-code.patch2.85 KBdave reid
#5 1744228-po-import-autoselect-language-code.patch2.81 KBdave reid
#4 1744228-po-import-autoselect-language-code.patch2.81 KBdave reid
#1 1744228-po-import-autoselect-language-code.patch2.87 KBdave reid

Comments

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Active » Needs review
StatusFileSize
new 1744228-po-import-autoselect-language-code.patch2.87 KB

Status: Needs review » Needs work

The last submitted patch, 1744228-po-import-autoselect-language-code.patch, failed testing.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +sprint, +language-ui

Good find, thanks will review later when I have time.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review
StatusFileSize
new 1744228-po-import-autoselect-language-code.patch2.81 KB

Ok, I shouldn't have tried to change the file upload field to a managed_file field.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 1744228-po-import-autoselect-language-code.patch2.81 KB

Ok this one should work.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
StatusFileSize
new 1744228-po-import-autoselect-language-code.patch2.85 KB

One more time. :)

bforchhammer’s picture

bforchhammer commented
Assigned: Unassigned » bforchhammer

I'll have a look at this...

bforchhammer’s picture

bforchhammer commented
Assigned: bforchhammer » Unassigned
Status: Needs review » Reviewed & tested by the community

Patch works as advertised. I tested it with a german translation file, and "German" is automatically selected in the language dropdown.

The patch also improves the help message for file-validation criteria by switching to theme_file_upload_help and adds the "maximum file size". Nice :)

Code looks clean and should be ready as far as I can tell.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

Awesome UX improvement!

I found this comment confusing:

"+ * This only works if the file is named something.locale-code.po."

I think it should be more like:

"+ * This only works if the file name ends with "LANGCODE.po."

And then it occurred to me that we don't have a thing called "localeCode" and so this should be changed to "langCode" throughout to be consistent.

I asked nod_ & seutje to take a look at the new JS and they said:

seutje: if that behavior is executed again, it would have 2 identical change handlers, won't it?
nod_: yep
nod_: webchick: well, it's the right Drupal.stuff at least, code could use some work to make it prettier

Asked them to follow-up here with more details, they did emphasize it doesn't necessarily have to hold up the patch but if we can get it all in one go, then woot.

nod_’s picture

nod_
Primary language French
Location Lille
commented
Issue tags: +JavaScript
StatusFileSize
new core-locale-import-autoselect-1744228-10.patch3.08 KB

For the future, If there is some js touched or added by a patch please add the JavaScript tag to the issue :)

Here is a reroll with kinda better JS. I added classes to the inputs as well, easier js selectors.

penyaskito’s picture

penyaskito
he/him
Primary language Spanish
Location 🧑🏽‍🌾 Seville 💃 Andalusia, UTC+2 🇪🇺
commented
Status: Needs work » Needs review
StatusFileSize
new core-locale-import-autoselect-1744228-11.patch3.08 KB
new interdiff-10-11.txt668 bytes

Changed localeCode for LANGCODE in js documentation.

Switched behavior name from importLocaleCodeSelector to importLanguageCodeSelector.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Reviewed & tested by the community » Needs work

The JS here seemed to regress because if you upload a file with a language code that does not match the any of the available options, it will select the first option in the list. The expected result is that it should *not* change the option that is selected.

nod_’s picture

nod_
Primary language French
Location Lille
commented

which browser?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Chrome 21 and Firefox. We should not call .val() on an element which does not actually have that value. Why was that removed?

nod_’s picture

nod_
Primary language French
Location Lille
commented
Status: Needs work » Needs review
StatusFileSize
new core-locale-import-autoselect-1744228-15.patch3.14 KB

nevermind, fixed, sorry about that. That'll teach me to try to be too smart :p

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Looks good again! :)

dries’s picture

dries commented

A quick screenshot would be nice! :)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
StatusFileSize
new AutoLanguage.png134.44 KB

Screenshot:
AutoLanguage.png

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Thanks. That helps. This looks handy, and code looks good so committed to 8.x. Thanks!

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: -sprint

Removing from sprint. I don't think this should get a changelog entry or change notice, it is such a tiny improvement. Thanks all though!

Automatically closed -- issue fixed for 2 weeks with no activity.