Skip to content
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 issue #4679: Sorting Joomla! available languages #4690

Merged
merged 3 commits into from
Oct 16, 2014
Merged

Fix issue #4679: Sorting Joomla! available languages #4690

merged 3 commits into from
Oct 16, 2014

Conversation

joomdonation
Copy link
Contributor

This PR fix the issue #4679 reported by @dkanchev. Basically, if your site has multiple languages installed, when you are on login page to login to administrator area of your site, the languages in languages dropdown might be displayed in random order (see #4679 for more detail).

I just added one line of code to make sure the languages are sorted based on locale code instead of random order before (although this is difficult to replicate this issue).

How to test

  1. Make sure your site has atleast two languages installed.
  2. Access to administrator area of your site, makes sure the language dropdown is sorted based on locale code instead of in a random order as explained in the issue.

@infograf768
Copy link
Member

I can't reproduce the issue here.
Languages are alpha ordered by language code:
below
de-DE
en-GB
es-ES
fr-FR
it-IT
mk-MK
ta-IN
screen shot 2014-10-16 at 08 47 33

@brianteeman
Copy link
Contributor

@Infograf I can't reproduce it either but as Daniel said it is specific to
certain storage . I have however seen it happen that is why I confirmed it
On 16 Oct 2014 07:50, "infograf768" notifications@github.com wrote:

I can't reproduce the issue here.
Languages are alpha ordered by language code:
below
de-DE
en-GB
es-ES
fr-FR
it-IT
mk-MK
ta-IN
[image: screen shot 2014-10-16 at 08 47 33]
https://cloud.githubusercontent.com/assets/869724/4658611/af7f67e2-5500-11e4-9b02-de6bbd7a8114.png


Reply to this email directly or view it on GitHub
#4690 (comment).

@infograf768
Copy link
Member

@test
adding ksort() has no undesired effect here.
Please correct code style to let us merge this PR.

@javigomez
Copy link
Contributor

I haven't yet check the changes but Travis complains for code style issues in this pull:

FILE: ...avis/build/joomla/joomla-cms/administrator/modules/mod_login/helper.php
--------------------------------------------------------------------------------
FOUND 2 ERROR(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 35 | ERROR | Opening parenthesis of a multi-line function call must be the
    |       | last content on the line
 38 | ERROR | Closing parenthesis of a multi-line function call must be on a
    |       | line by itself
--------------------------------------------------------------------------------

@joomdonation
Copy link
Contributor Author

@ALL : Sorry, the code of my first commit is clearly wrong. It has no affect to the $languages array and doesn't solve the issue.

@dkanchev: Your code should work as well. However, I think the code of my second commit is shorter and easier to read:

usort(
    $languages,
    function ($a, $b)
    {
        return strcmp($a["value"], $b["value"]);
    }
);

However, it still has problem with code style. Anyone here know how to correct it and can help me with this codestyle issue ?

@joomdonation
Copy link
Contributor Author

OK. I was able to fix the code style issue. Please help testing it again :).

@dkanchev
Copy link

The proposed patch resolves the issue. Tested and works as expected :)

@brianteeman
Copy link
Contributor

Awesome

On 16 October 2014 13:44, dkanchev notifications@github.com wrote:

The proposed patch resolves the issue. Tested and works as expected :)


Reply to this email directly or view it on GitHub
#4690 (comment).

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

@joomdonation
Copy link
Contributor Author

I think this PR can be merged now. Thanks All :).

@infograf768
Copy link
Member

I wish I could first test this here locally.
let me first test if no issue on my settings before merging

@joomdonation
Copy link
Contributor Author

Thanks @infograf768.

@infograf768
Copy link
Member

No unwanted effect here. Languages are still presented alpha by lang tag

@infograf768 infograf768 added this to the Joomla! 3.3.7 milestone Oct 16, 2014
infograf768 added a commit that referenced this pull request Oct 16, 2014
Fix issue #4679: Sorting Joomla! available languages
@infograf768 infograf768 merged commit 0d13914 into joomla:staging Oct 16, 2014
@mbabker mbabker modified the milestones: Joomla! 3.3.7, Joomla! 3.4.0 Nov 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants