Scalable menu selector

I'm getting on the wagon to assist with this UI enhancement. To describe it in a way that I understood:

- Go to admin/build/menu/item/22/edit on a new D6 install. The drop-down select for parent contains the entire menu tree.
- Such a large tree is a huge scalability problem.
- We should replace this with a drill-down type of solution of a select list for each parent. Selecting a parent creates another select list of all that item's children.

Probably the best news of all of this is that no new javascript will be necessary (I hope) thanks to our new ahah.js file and javascript behaviors. We'll just need to make a menu callback and setup the #ahah properties.

I've made the deal that if chx (or anyone) writes the non-js version with an update button to update parents, I'll write the AHAH implementation.

You could call me the JS guru...
I have it all figured out, but I'm still running into problems with locating the form element...
SO...
I have one big code comment for chx to read when he gets back from his sleep

the status box seems not to work
/me files a bug

Here's a patch that has the ahah request being made properly. Note that it adds a theme function so be sure to clear your cache.

Here's another patch which takes the Update button out of the parents array and puts it into a separate wrapper element. Chx informed me the previous method was a bad idea, causing a recursion error.

I tested this on FF 2.0.0.9 on WinXP, IE6 on WinXP, and IE7 on Vista and it works well. I think this is a big improvement over the select list that can get very long.

Two minor problems:

1. On IE6, occasionally when I clicked on one of the items in one of the children boxes, that box and all children boxes of that box (if any) would all turn gray, and I needed to click one of the items in the parent box to "unfreeze" things.

2. In both IE6 and IE7, when I select an item in the child box, the scroll position in that box is shifted such that the selected item is at the top of the visible items in the list. This is a bit inconvenient since, to select the "-----" item, I then have to scroll up in the box. It would be nice if the scroll position did not change, which is the case in FF.

Did some quick testing:
-The UI shouldn't display parents that will result in the menu becoming too deep.
-I'd prefer to see appropriately indented drop down selects on separate lines, rather than the side by side scrolling selection boxes.

I might not have understood the issue cleanly, but the issue cried for a better title. So if I did not understand well, please retitle and explain.

Bug report:
I went to page http://localhost/d6/admin/build/menu/item/9/edit on a fresh d6 install. Then I applied this patch and http://drupal.org/node/193191#comment-634164 and reloaded the page. Then I clicked "parent" and saw the two select boxes, one after another. I clicked "My Account" in the first box. The page did an AHAH update, moving the two select boxes to be side-by-side, and the attached error box popped up.

Ah, I just needed to rebuild my menus, by going to admin/build/themes and saving.

Works for me. Slick! I still hope Nate's drag & drop menu reparenting gets in, but dynamic subselects are a fantastic widget for drupal to have.

I get at "Fatal error: Call to undefined function menu_parent_options() in W:\www\drupal-cvs\modules\menu\menu.module on line 507" after applying this patch and visiting node/add/story, or any other content type.

Can't wait to let http://drupal.org/project/hierarchical_select take advantage of the same techniques!

I've found a bug: if JS is disabled, and you click the "Update" button, then the text fields you've just filled out are emptied.

Also, there's a "/* RTL */" comment in the LTR CSS instead of the RTL CSS :P

OK fresh install, patched before installing then enabled all contrib modules.

Very, very nice. The form refresh on firefox was a little slow, but not too much (and my connection has been terrible today so could be my end). So much better than the huge select list and this will be great for all kinds of stuff. Saving seemed to work fine etc.

Tested in IE7/FF2.

Thought I'd put together a 60-second video of this.
http://blip.tv/file/get/Kentbye_tech-MenuParentSelectors263.mov

Played around with it a bit on FF2.0.0.9 and it looks good.

I like it.

How about moving Weight and Expanded to the Parents fieldset?

In the video, why is the title of the fieldset 'Parents'? Can you select multiple parents or just one? Shouldn't it be 'Parent' (singular)?

Why is the parents fieldset the only collapsed fieldset? That looks inconsistent with the rest of the form.

Why is the status message shown inside the fieldset? That is also inconsistent with the rest of Drupal.

If we're going to commit more usability improvements before tomorrow night, I'd suggest we focus on (i) the menu module DnD patch, (ii) the taxonomy DnD patch and (iii) the book module DnD patch. Let's tackle the other ones first so we don't risk losing our focus. If the other patched landed, we can look at this one. If not, it will have to wait for D7. Thanks.

Dries, although this is also about usability, I think chx is rightly concerned about potentially many thousands of menu items being loaded into memory all at once. Although I'm a big fan of those other three patches they don't have the (potentially quite bad) scalability issues that this one deals with. Edit: and menu drag and drop is in! so it's just book and taxonomy left now.

Marking to needs work for the fieldset issues as identified in #29 and #30 and I'll go review drag and drop patches now!

subscribing - thanks chx for pointing me to this

Well, Dries' concerns in http://drupal.org/node/191360#comment-635000 does not look like unsolvable within this timeframe. Why not look into them?

@Dries: "Why is the status message shown inside the fieldset? That is also inconsistent with the rest of Drupal."
I believe was my fault in the production of the screencast.
I saved the theme form, but then shut the window before the status message could be displayed because it would've been 7-10 seconds of dead time.
When the theme page was saved, then the status message was incorrectly displayed on that page.
An edge mistake IMHO, and shouldn't reflect negatively on this patch.

chx: Understood. So why not add some docs to this override? :)

OK, committed this for now. Happy hacking in D6 contrib :)

minor UI point: the weight select should be above or below the drill-down selectors. Now it's appearing to the right of the selects and is bumped around when changing selections (at least on FF 2 on linux).

In the code I'm not clear why you are adding a new possible -1 setting to the 'customized' attribute of menu links?

Also, why in the _menu_parent_depth_limit function that moved to menu.inc is the check for 'has_children' omitted compared to the original - seems a cheap check to make to avoid a function call and query.

Also, since you're moving _menu_parent_select_recurse to menu.inc, is the plan to try to re-use this function when doing a drill-down for book module?

Also, I don't know much about JS, but it looks as though you are submitting the entire form every time there is a change? Is this a better approach than rebuilding the form in the cache, as is being done now for the book parent selector? Seems like it would be much slower?

It's going to be a bit hard to re-use this for books, unless you split them apart from the existing book chooser - this contains options like "create a new book" and "none" which don't exist in the context of the menu module.

I tested the patch in Safari and it looked good.
Here's is an updated video:
http://blip.tv/file/get/Kentbye_tech-Menu_Parent_Selectors_V2_476.mov

I had some weirdness in FF2.0.0.9 where the new child options were appearing below the parent instead of next to it.
I'll try to reproduce & capture it.
UPDATE: I was not able to reproduce the FF glitch.
Worked fine as well.

OK quick review in FF2 and IE7, this is looking good.

In FF2 I get the scrollbars regardless of if there's a need to scroll or not. In IE7 they appear depending on the number of items, which is nicer.

In both, it's very, very quick.

Only other thing I noticed was if you set menu options before entering a title in the node form, you get the "you must enter a title" validation error, that's not a big issue but it was a bit of a jolt considering the form hasn't been submitted or previewed by that point.

It's pretty common to fill different bits of the form out at different times, so +1 for me for suppressing errors until a full preview/save.

While I like where this is going, I don't like the way the new menu lists appear on the right. On a small screen resolution, this behaviour becomes a little tricky as bits start falling into a new line.

I would recommend something in the line of sliding menus. It allows greater control over the width without having elements fall due to screen size. dvessel's excellent lumen theme highly demonstrates this concept:
http://si-lumen.us

There was talk of Greggles ripping this out into a separate module but I'm not sure if this materialised:
http://groups.drupal.org/node/2839

I am against holding this up, but would be good if we can get the current functionality in for Beta 3, and then reopen the issue for UI improvements?

ok - selector seems to be working well on the menu item editing form except for a very minor complaint that if I have scrolled the select, the newly selected item jumps to the bottom of the select when the selector repopulates.

After saving the item on a page like admin/build/menu/item/362/edit, I did not get properly redirected to the menu overview. Instead I go back to the edit form, and the data in the select is stale - it shows the parent before editing started. However- I wonder if this is a bug in the AHAH JS?

The selector in the node form doesn't seem to work - it doesn't appear. If I submit the node form I get: notice: Undefined index: select in /Users/Shared/www/drupal6/modules/menu/menu.module on line 351.

Note: testing node with FF 2.0 on Max 10.4

chx, how should one test this??

admin/build/menu/item/%/edit is flawless, very speedy, very easy to use.

It's not saving the menu items from the node form for me though - they don't show up when I edit the node again, nor in the menu admin screen.

Stefan: it'll make sense when you see it, just remember to trigger a menu rebuild otherwise you'll get all kinds of craziness :)

chx is right, I wasn't specifying a title. This all works very smoothly.

The only niggle I have is that with js disabled, the "update menu parent" item is inline with the select boxes, it should really be above or below. Maybe take it out of from['wrapper']?

Leaving at RTBC anyway since this that shouldn't hold this patch up.

This is all possible in contribs now with the extension points we added earlier to Drupal 6, right? I would not like to introduce a new UI concept again (with people rushing to find bazillion places to apply it in Drupal), in part because

- we are not covering these bazillion places in Drupal (inconsistent UI for the same tasks)
- we introduce a new UI concept which was debated above with its unfriendliness to not so wide layouts
- we make changes to the ever changing menu code again
- as said, you can do this in contrib now

Please understand that we try to concentrate on stabilizing and fixing bugs in Drupal 6, rather then adding new stuff to it. You are in a unique good position that these changes are doable in contrib for Drupal 6, and can be introduced in Drupal 7, with the UI concepts refined through the Drupal 6 lifetime. Thanks for all your work.

http://drupal.org/node/203010 was duplicate.

subscribe

We still need this, but it no longer applies.

I reckon something like this is the most likely option for removing some of the confusion around 'parent item' (and the menu fieldset in general).

At the moment, we have a long list of almost undifferentiated links in a select - with this, we'd clearly display the top level menu, then children programmatically - and it might be enough to put "select menu position" and avoid parent/child language completely.

So, bump.

Note: Hierarchical Select 3 supports this. It will be ported to Drupal 6 very soon. But, in its current state, it cannot be accepted into Drupal core, it will have to be refactored. In any case: if you want to improve the usability of this, you can, for both Drupal 5 (today) and Drupal 6 (soon).

Second note: this issue also implies fixing a scalability issue: if the menu tree is huge, it'll work very slowly in its current state.

I'd like to see a similar method for choosing taxonomy parents, so something centralised (like hierarchical select) woudl be good for that - haven't looked at it for a while though.

Taxonomy is already supported! But now you mention it, I haven't overridden the form for creating/editing terms yet. Will do that later today.

EDIT: forgot to add this: I'm currently adding support for selecting Book page parents, too!

Wim Leers - I know it's in hierarchical select, I meant getting a comprehensive solution for multiple-depth select forms into core - so we can have a unified UI between menus and taxonomy (and maybe book module on the node form as well).

I know I know, I'm only indicating that it's already supporting every place in core where it can be used :)

Subscribe. This "comprehensive solution for multiple-depth select forms" is an excellent example of a useful new tool in the Drupal UI toolkit.
Let's see what we can do about this @Szeged Drupalcon.

I wish I'd seen this thread earlier - then I would have understood the history behind the strange 'extension stub' which was all that made it into D6 for hierarchical parent selection. I've almost finished porting the Category module to D6, and I've borrowed the AHAH parent-select from book.module for it - would have been better to have implemented something more fancy.

I'll be keeping my eye on Hierarhical Select, and hopefully helping to get it ported to D6. Would be great to get all or part of it into core for 7.

Subscribe.

subscribing

I'd like to cross-reference this to #360128: Security fix for simplified AHAH callbacks, which could shorten-up this patch a little bit by taking out the menu callback and most of the page callback. Plus we wouldn't have to directly check $_POST.

Subscribe

I'm having this same problem with the Menu Block module. :-p My solution is a total hack though and doesn't scale. I can help out here if the js isn't too crazy. :-)

So, "getting a comprehensive solution for multiple-depth select forms into core - so we can have a unified UI between menus and taxonomy" (#72) seems to be the executive summary of the goal for this patch. Latest actual code is from 18 months ago (#60)

We still need this, can somebody look at the patch does and summarize what it tries to do? Can I be of help here by drawing pictures?

tagging,as yoroy said its pretty crucial someone picks this up

We have this now on content type configuration screens:

Can we say that at least adresses part of this problem or even 'fixed' it for now? Checking if we can move away from 'critical' status here.

It's still not scalable, so I think it's still critical. From a usability perspective it's probably no longer critical. Maybe it's better to mark this as a performance issue instead of a usability issue.

I doubt this will be fixed for D7 in core though. I know I won't — Forms API doesn't allow clean/readable enough code yet.

Subscribing.

Do we have some kind of plan how this should look like? I could implement it.

I don't think one week before alpha is a good time to try and start working on this :)

@casey #87; http://wimleers.com/demo/hierarchical-select/menu

I agree that the solution yoroy referred to in #85 isn't a fix for the real usability bug here, just a band aid that sometimes fixes it if the site builder takes the time to configure it such.

Bugs cannot target D8, as the tree is not open yet.

Fortunately, this is arguably a feature request.

http://drupal.org/project/chosen might be of use here

Tagging as performance because I ran into issues with menu_form_alter() doing a menu_parent_options() for $form['menu']['parent'] (Menu settings > Parent item) loading all {menu_links} which was kicking out 40,000 results and taking 30 seconds to load each /node/add/content_type page

This remains a (potentially severe) scalability issue. I'm working on a site with thousands of menu links, and users who don't have 'bypass node access' are experiencing extremely long load times on the admin page for adding a menu link (greater than 30 seconds wall clock time, and on my local dev copy of the site, it actually exceeds a PHP maximum execution time of 30 seconds). For users who do have 'bypass node access', the page loads reasonably quick, but the parent selector dropdown is still so big as to be nearly unusable (simply trying to open the dropdown causes Firefox to freeze for several seconds on my machine).

(Edit: I should note that there may be some extra influences to access checks on the site I'm looking at that are contributing to the slowdown - at a minimum, Group module is in play - but the scalability of loading so many links at once and the (un)usability of such an overloaded dropdown are still legitimate issues, regardless.)

Looking into this issue a bit further, I have a stupid question. When you go to add or edit a menu link, the name of a particular menu is right there in the URL for the admin page. So why does the parent selector show all links for all menus in the system? Just to provide a convenient way to move a link from one menu to another? It occurs to me that limiting the parent selector to the current menu of the new or edited link could take the edge off of the scalability issue considerably (you could still have thousands of links in one menu, but I'm guessing that's at least a somewhat less common use case than having dozens or hundreds of links each in dozens or hundreds of menus). This would of course only affect the add/edit menu link pages, not the node edit page, and even at that it wouldn't completely address the underlying issue, just a thought on what might be a relatively quick band-aid for at least some situations.

(Edit: I misspoke, the URL only contains a menu name for the add link, not the edit link - but menu link entities do store the name of the menu they belong to so it's not inaccessible in that case either.)

We should probably do this at the widget level rather than the menu feature level, because there are other long select fields (e.g. taxonomy term hierarchy) with this exact problem. So, closing as a duplicate of #2346973: Improve usability, accessibility, and scalability of long select lists.

Note the issue raised in #110 was resolved for Drupal 10.2 via #3110371: When adding a new menu link, restrict the available parents to the current menu