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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sort menus in the backend menu alphabetically #5683

Merged
merged 5 commits into from Jan 14, 2015

Conversation

@Hackwar
Copy link
Member

@Hackwar Hackwar commented Jan 12, 2015

This PR sorts the menutypes in the backend in alphabetical order and not in order how they were first created some ancient time ago. 馃槈

How to test

  1. Go into the backend, open the "Menus" menu and see that it is not ordered alphabetically.
  2. Apply change.
  3. See that the menus are now ordered alphabetically.
@@ -139,7 +139,8 @@
$menu->addSeparator();

// Menu Types
foreach (ModMenuHelper::getMenus() as $menuType)
$menuTypes = JArrayHelper::sortObjects(ModMenuHelper::getMenus(), 'title');
foreach ($menuTypes as $menuType)

This comment has been minimized.

@zero-24

zero-24 Jan 12, 2015
Member

@Hackwar can you add a line bevor the foreach?

This comment has been minimized.

@Hackwar

Hackwar Jan 12, 2015
Author Member

Done.

@richard67
Copy link
Member

@richard67 richard67 commented Jan 12, 2015

@test Success.
screen shot 2015-01-12 at 14 14 00


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Jan 12, 2015

I get the following error

Strict Standards: Only variables should be passed by reference in /Applications/MAMP/htdocs/joomla-cms-staging/administrator/modules/mod_menu/tmpl/default_enabled.php on line 142


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.
@richard67
Copy link
Member

@richard67 richard67 commented Jan 12, 2015

Ooops: I have the same, haven't seen it when testing before (blush).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.

@Hackwar
Copy link
Member Author

@Hackwar Hackwar commented Jan 12, 2015

Sorry, fixed that one. I even looked at the source of JArrayHelper::sortObjects() and saw the reference, but missed it...

@richard67
Copy link
Member

@richard67 richard67 commented Jan 12, 2015

@test Success. Works now without PHP warnings after correction.
What would be perfect of course wold be to have an ordering column for the menus, as we have for the menu items, and show them in this order in the menu (which can be modified by drag and drop in the menu manager).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Jan 12, 2015

@test the Strict standards issue has now been fixed.

I now see that this change isnt applied in Hathor. Not sure if that uses an override ?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.

@richard67
Copy link
Member

@richard67 richard67 commented Jan 12, 2015

@brianteeman Yes, I just see Hathor has an override mod_menu/default_enabled.php.
@test I've forgotten Hathor (blush).


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.

@infograf768
Copy link
Member

@infograf768 infograf768 commented Jan 13, 2015

Not sure I like this one. In my multilingual test site, the menus which were created one language after the other are now disordered.

Also, the sorting done in this PR is different in mod_menu vs the dropdown in "Menu Manager: Menu items"
In mod_menu all menus starting with a Capital letter are grouped first, not in the dropdown where it it is not case sensitive, i.e.

mod_menu
Mainmenu
Top menu
[...]
my menu
toys menu

Dropdown
Mainmenu
my menu
[...]
Top menu
toys menu

If it was decided to alpha sort the menus, the second solution is much better imho

can be used instead, to solve at least the case sensitive issue (IF we sort alpha)
$menuTypes = JArrayHelper::sortObjects($menuTypes, 'title', $direction = 1, $caseSensitive = false, $locale = true);
or, shorter
$menuTypes = JArrayHelper::sortObjects($menuTypes, 'title', 1, false, true);

we would get

screen shot 2015-01-13 at 09 04 27

instead of

screen shot 2015-01-13 at 09 14 18

changing locale to false has apparently no effect here.

@Hackwar
Copy link
Member Author

@Hackwar Hackwar commented Jan 13, 2015

I've changed the sorting to be case-insensitive and added the change to Hathor, too. I also first thought I enabled the local-aware sorting, but then noticed that that feature is actually implemented pretty stupidly... I would have to hand in the locale by which to sort and not simply true or false and quite frankly, the amount of code that would have to be written to do this properly is simply not worth the gain we maybe would get. As far as I can see, it would only make sure that letters like o and 枚 would be grouped next to each other in German for example. However, I think that it already orders via UTF8 and I guess that that takes care of this already anyway... Mind you, that is not based on hard facts, but a hunch. 馃槈

In any case, I guess this is now ready for final testing/merging, depending on if this is actually a wanted feature. However, if we don't do this, we should remove the ordering in the menu manager filter dropdown to keep this consistent.

@infograf768
Copy link
Member

@infograf768 infograf768 commented Jan 13, 2015

I agree it should be consistent, thus why I proposed the code.

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Jan 13, 2015

@test all works well for me - thanks for this @Hackwar

However, if we don't do this, we should remove the ordering in the menu manager filter dropdown to keep this consistent.

I dont see this filter?


This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/5683.

@Hackwar
Copy link
Member Author

@Hackwar Hackwar commented Jan 13, 2015

if you go into the list view of a menu in the menu manager, you can switch between the menus that are displayed. That is the dropdown that I'm talking about.

@brianteeman
Copy link
Member

@brianteeman brianteeman commented Jan 13, 2015

Ah gotcha
On 13 Jan 2015 10:18, "Hannes Papenberg" notifications@github.com wrote:

if you go into the list view of a menu in the menu manager, you can switch
between the menus that are displayed. That is the dropdown that I'm talking
about.


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

@brianteeman brianteeman mentioned this pull request Jan 13, 2015
@infograf768
Copy link
Member

@infograf768 infograf768 commented Jan 14, 2015

Fine here. Merging. Thanks.

infograf768 added a commit that referenced this pull request Jan 14, 2015
Sort menus in the backend menu alphabetically
@infograf768 infograf768 merged commit b545d3d into joomla:staging Jan 14, 2015
1 check passed
1 check passed
continuous-integration/travis-ci The Travis CI build passed
Details
@infograf768 infograf768 added this to the Joomla! 3.4.0 milestone Jan 14, 2015
@Hackwar Hackwar deleted the Hackwar:backendmenuordering branch Apr 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants