Currently (and for years) the sorting of items in the admin menu (/admin) has been incorrect if an item started with multiple capital letters. For example, SMTP comes before "Save", "Search", and "Site". I know it seems trivial, but the total number of minutes I've spent going to enable modules I thought were missing actually adds up to something over the years.
Our users expect that when something appears to be sorted, it will be sorted correctly.
I have attached two screenshots and a patch for this. There's a "before" screenshot, where you see the menu entry "SZZZ*" come before "Site Information". There's an "after patch" screenshot, showing it sort correctly, and there's a patch.
The patch just sorts the items by title before outputting them.
Comment | File | Size | Author |
---|---|---|---|
#11 | 528204_admin_page_sorting_d6_01.patch | 630 bytes | rfay |
#8 | admin_menu_image_toolkit_change.png | 8.47 KB | rfay |
#6 | Picture 1.png | 124.88 KB | drewish |
#3 | admin_page_sorting_528204_02.patch | 657 bytes | rfay |
system_module_admin_menu_01.patch | 1.16 KB | rfay | |
Comments
Comment #1
drewish CreditAttribution: drewish commentedSubscribing. As the maintainer of the iTunes module I've been annoyed by this for quite a while, my module always ends up at the bottom of the settings list.
Though I think we can use PHP's natcasesort() and avoid the helper function.
Comment #2
chx CreditAttribution: chx commentedpatch is without -p, i can't really review it. i would think you want to fiddle in menu.inc with
$new_tree[(50000 + $item['weight']) . ' ' . $item['title'] . ' ' . $item['mlid']] = $tree[$key];
which then goes to be ksorted but then that will affect every menu tree if you lowercase the title in there.Comment #3
rfayThank you for the reviews and the suggestions. I found a far better way to do this (suggested by chx, but in a different place). Now it's a one-line patch, and it affects only the presentation of the admin menu. I'd appreciate it if you could take another look.
The new patch does nothing but drupal_strtolower() the artificial array key that's being used for menu generation.
Comment #4
drewish CreditAttribution: drewish commentedhumm.... that looks really good. i haven't had a chance to apply it and test it but lets update the status and get the testbot feedback.
Comment #5
Dries CreditAttribution: Dries commentedLooks good to me too. drewish, do you have a change to test this out?
Comment #6
drewish CreditAttribution: drewish commentedmaybe i was testing it wrong but i just went into the menu settings and renamed the "Image toolkit" menu item to "image toolkit" but the sorting was still wrong:
Comment #7
rfay@drewish: The admin menu is cached. I assume you cleared your cache?
Comment #8
rfay@drewish: The test scenario you tried is an excellent one (just go to admin/structure/menu-customize/management and change the name of "Image Toolkit" to "image toolkit". Without the patch, it drops to the bottom of the menu.) But I just did it (clean D7, applied patch from #3) and got the expected results. See screenshot. Could you just verify your setup? Thanks!
Comment #9
drewish CreditAttribution: drewish commenteddoh. i had applied the patch to the wrong site. this works beautifully.
Comment #10
webchickThat's a nice little improvement! Committed to HEAD.
Moving back to 6.x for consideration. Patch doesn't apply for me.
Comment #11
rfayHere's the D6 version. A no-brainer.
Comment #12
rfayping... @drewish, perhaps you can look at this and we can get it RTBC'd so we can get rid of this annoyance in D6.
Comment #13
cweagansLooks good, patch applies, code works.
Comment #14
Gábor HojtsyCommitted to Drupal 6, thanks!