As we are having Drag'n'Drop now, paging the menu items in admin/build/menu-customize/... makes no sense.
Moreover it renders the Drag'n'Drop functionality unusable, as items can be moved only within the same page.
Additionally, the $sql_count query in menu_overview_form() leads to a wrong count, resulting in pages with less than the intended 200 items each. In fact, in a new installation only some 80 entries are shown on the first page. The reason for this is, that all menu items are counted, but in the end ony the ones are shown, 'where hidden > -1'.
If we completely remove paging, the latter bug doesn't matter. Still we should anyway include the where-clause in the $sql query, so menu_tree_data() doesn't has to sort out menu items that are anyway not being displayed. This should improve performance to some extent.
My enclosed patch:
- removes paging to show all items on the same page.
- adds 'where hidden <> -1' to the $sql query
Did some basic testing, but some more testing might be appropriate.
Comment | File | Size | Author |
---|---|---|---|
#11 | admin_menu-customize.patch | 1.08 KB | Pancho |
#8 | menu_count.patch | 866 bytes | catch |
#1 | remove_menu_paging.patch | 1.09 KB | Pancho |
Comments
Comment #1
PanchoHere's the patch.
Comment #2
JirkaRybka CreditAttribution: JirkaRybka commentedHm, personally I'm having trouble with the amount of items per page being already too big: My desktop is not exactly the fastest, so the d-n-d JavaScript takes *ages* to initialize after page load, even shows a browser-warning about the script possibly being dead, I must confirm that I want to wait further... So I'm afraid that unlimited page-size might kill usability of that page even in more snappy environments, if the menu is really big, leaving only just JavaScript manual disable as a workaround.
Comment #3
PanchoJirkaRybka: Please try the patch and report us, whether page initializing takes significantly longer or not. If yes, we might have to improve the JS script or to allow for turning it off (though this is an admin-only interface and admins usually have enough RAM on their desktops).
While unlimited page size is not exactly optimal, a broken Drag and Drop interface is simply unacceptable. IMHO, we have no other choice than to remove paging.
Comment #4
catchI'm sure this came up at the time - and the answer was to edit the menu item individually and change it's position there. Exactly the same issue applies for taxonomy, and exactly the same solution.
I'd support a larger number of visible items per page - 200, 500 whatever, but we could be looking at tens of thousands of items on some sites so the pager ought to be left in.
So I'm tempted to won't fix this, but open a feature request in drupal 7 for some kind of "persistent drag and drop drop box" block which acts like a clipboard between pages to avoid having to edit items individually (and entirely remove the concept of weight).
Will leave it open for more to comment though.
Comment #5
JirkaRybka CreditAttribution: JirkaRybka commentedMy menu_links table currently contain 376 entries (some 250 real items, plus bogus links from 5.x->6.x upgrade, not manually removed yet :-/), so that's just 2 pages on the pre-patch UI.
Page initialization times (approximately) on my desktop (Firefox 2.0.0.6 / Linux Mandriva / Pentium III 500 MHz) - measured in seconds, also counted how many times I needed to manually confirm the "script not responding, do you want to wait further?" message:
Before patch, page 1: some 18-19 seconds (1x confirm)
Before patch, page 2: some 18 seconds (1x confirm)
After patch (single page): something around 35 seconds (3x confirm!)
Conclusion: Time depends on menu (page) size directly.
I know that 500MHz is not exactly modern hardware, but if new machines are some 5 times faster, that's still (estimated) around 7 seconds / 380 items. My menu is not *that* big, but I repeatedly saw mentions that some people put all their nodes (thousands, even!) into menu (because Drupal lacks a usable categorization system for some use cases, excepting Category module which is buggy and generally not much liked). So this is my point, really: People may have really *big* menus (although I haven't).
Comment #6
Panchocatch: Such a persistent Drag'n'Drop box in fact seems to be a good solution for D7.
For D6: I agree, that removing paging would pose a problem to sites with several thousand menu items. Anyway, "won't fix" wouldn't be a solution at all. At least we need to make sure, the intended 200 items are shown (currently the wrong $sql_count query makes it show only some 80 items). I agree, that we should even raise this to 500 to cover many, if not most sites. Even then, we'll have some support requests about this, but maybe we can't do better for now.
Comment #7
catchWhy not this then?
Comment #8
catch#7 patchified. Fixes the issue for me. I think 200 items per page is sensible tbh both for js timeouts and potential memory issues.
Comment #9
chx CreditAttribution: chx commentedThe menu system is designed to hold tens, even hundreds of thousands of links within the same menu. Removing the pager is not an option.
Comment #10
JirkaRybka CreditAttribution: JirkaRybka commentedchx: We already agreed on that (I believe), but #8 attempts to fix the remaining issue here (second paragraph of initial post), which is incorrect pager count query. I have no clue whether it's done correctly now (no time to check properly...), but #8 needs review, definitely.
Comment #11
PanchoOkay, kept paging in.
But the $sql query should be corrected anyway. And in the $sql_count query the "ml" table synonym is not needed.
Rerolled therefore...
Comment #12
chx CreditAttribution: chx commentedI believe you need the hidden elements to properly build the tree :/
Comment #13
John Bryan CreditAttribution: John Bryan commentedSurely the answer to drag'n'drop functionality for large menus is to display them as a collapseable menu?
Just open to the branch or leaf you want to move or edit. Any menu system to large to be manipulated on a single page this way is to large to be a single menu.
Comment #14
Pancho@John: ??? Sorry, I didn't get what you mean!
Comment #15
John Bryan CreditAttribution: John Bryan commentedOn the menu /admin/build/menu-customize page have the menu being edited as a collapsed but expandable tree structure where branches or items can be drag and dropped e.g. like the folder pane in windows explorer etc. Think "nice-menu"/"DHTML menu" modules but draggable
I.e display the menu structure as a dynamic menu. You could even make it, like a customized 4.7 site I inheritted, so that if a user has menu edit permission they can be edited in situ from any page where the menu is visible!
No need of paging, easy to drag items distantly removed from each other.
Comment #16
sun