The current form that generates the list of languages is quite a mess.
You would think that it's a form of '#type' => 'radios', but in fact each language is its own form of '#type' => 'radio'. The form callback then uses '#return_value' to get everything working.
The attached patch
1. changes the form to build a normal $options array and simply use a '#type' => 'radios',
2. sticks a asort()
in the middle to make the list properly sorted. This is especially nice, because asort() is case-sensitive, so when using l10n_install, all those languages who don't have a proper name (xxx-lolspeak, etc...) show up last.
This is technically an API change, but in practice there's only one project that this could theoretically affect (l10n_install), but that doesn't currently alter the form.
Also Gábor Hojtsy approved this, over in #944390: Improve usability of language selector (l10n_install) issue.
Comment | File | Size | Author |
---|---|---|---|
#16 | sort_list_language-966818-16.patch | 1.46 KB | valthebald |
#11 | sort_list_language-966818-11.patch | 1.46 KB | idflood |
#11 | sort_list_language-966818-11-d8.patch | 1.46 KB | idflood |
#7 | 966818-install-locale-form-7.patch | 1.44 KB | idflood |
#1 | install-form.png | 55.49 KB | tstoeckler |
Comments
Comment #1
tstoecklerAnd a screenshot showing that the usage of '#type' => 'radios' removes a lot of useless whitespace.
Comment #2
Gábor HojtsyOh, looks like a much better default! Why is it needs work and not needs review?
Comment #3
tstoecklerThat, I don't know :)
Comment #4
Bojhan CreditAttribution: Bojhan commentedSorry, but did you check the implications of this on the usability of all other forms? I mean, we can easily create this more usable, I agree - but not if it negatively impacts the readability of other forms.
Comment #5
Gábor HojtsyThis is a PHP code change that only affect this single form. No other forms are affected in the installer or elsewhere. The patch merely changes from including the elements as 'radio' buttons one by one to include them as a group of 'radios' which required less PHP code and apparently displays tighter. See screenshot.
Comment #6
Bojhan CreditAttribution: Bojhan commentedStrange, well RTBC from visual perspective then.
Comment #7
idflood CreditAttribution: idflood commentedI applied the patch and at first it looked exactly to what I was expecting.
But the "value" attribute for the inputs are wrong with this patch ( output "1", "2", ... instead of "en", "fr",... ). The problem comes from the "sort($options)". It replace the array keys, leading to wrong values. Using asort($options) fixes the issue.
The new patch only replace the sort function with an asort.
Comment #8
Gábor HojtsyJust marked #1121400: Sorting the Language list will dramatically improve usability a duplicate.
Comment #10
tstoecklerAssigning this to me, so I don't forget. This simply needs a reroll, though, so if you're faster than me, feel free.
Comment #11
idflood CreditAttribution: idflood commentedHere is a reroll of the patch in #7. I also created a d8 patch only to see it's exactly the same as d7.
Comment #12
tstoecklerLooks good, will try it out later.
Comment #13
Gábor HojtsyOk, unfortunately needs to go to 8.x first. Since its the same, it should not be scary to anybody, right? :)
Comment #14
tstoecklerNot strictly related to this patch, but is this form of string concatenation compatible with RTL-languages?
Powered by Dreditor.
Comment #15
aschiwi CreditAttribution: aschiwi commentedDefinitely looks much nicer! The patch in #11 works for me (Drupal 8) and I have no problems installing in another language either.
Comment #16
valthebaldI'm not quite sure how to submit to retest patch that was ignored on the first run, so just re-upload patch sort_list_language-966818-11-d8.patch from #11
Comment #18
Gábor Hojtsy@valthebald: well, that will not apply to Drupal 8 anymore. Does this bug still relevant for Drupal 8? We have been reworking the language list in the installer various times in Drupal 8 :)
Comment #19
valthebald@Gábor Hojtsy: I guess no, just wanted to ensure
Comment #20
Gábor Hojtsy@valthebald: all I said is the patch does not apply to Drupal 8 not that all problems are not applicable to Drupal 8. I'm not sure Drupal 8 would apply any ordering different to the language code base alphabetical list, looking at the code. Also even if none apply to Drupal 8, Drupal 7 could still use a fix. So I'd not go as fast as closing this down outright.
Comment #21
tstoecklerThis has since been fixed.
Comment #22
tstoecklerOops, didn't read #20 correctly. I guess this could go into Drupal 7.