Nice menus compatibility

I understand part of the problem, but not quite all of it.

If you disable DHTML Menu entirely, does the primary menu then do what you want?

If so, then the patch I wrote for #352005: Ubercart DHTML conflict - use module-specific selector might fix this one. It makes DHTML Menu use a special CSS class that ensures it only adds its Javascript functionality to the menus that it actually controls.

This way, you should be able to use NiceMenus for your primary horizontal menu, and DHTML Menu for your sidebar menu, without causing a conflict.

Thanks. Actually the Nice Menus part seems to work fine (it is CSS only I think unless using IE, which I am not). It is the dhtml_menu that stops functioning (as described). I was wondering if the fact that the nice menu block does not have the 'no_dhtml' class may disrupt the dhtml_menu for its own menus.

Tested with 6.x-3.4 in the hope that the Ubercart fix would help me. Still does not work (as described above). Is there any more info I can provide to help this issue? Thanks.

I looked into this a bit. It is not a JS issue (confirmed by creating a working static html copy without nice_menus and then manually adding the nice_menus part -- both dhtml_menu and nice_menus will work fine). The problem is in the menu being generated by dhtml_menu. When nice_menus is present all the expanded menus are missing from the html source (all rendered as leaf). In dhtml_menu_theme_menu_item all the $has_children flags are 0 and so the expanded menu never gets rendered. Next step is to figure out why $has_children is incorrectly 0 when nice_menus is present. Any ideas?

I continued to look at this and can get nice_menus to work with a kludge. The problem seems to be that when nice_menus calls theme('menu_item_link') (which it does in 2 places) this is intercepted by dhtml_menu. This should be OK and in fact the primary-links menu (the nice_menu) is rendered correctly, but the *other* dhtml_menus are not. Maybe it's an issue with the dhtml_menu stack? Anyway if I change nice_menus to not call the theme function but a local version of the theme_menu_item_link then all will work OK (not that this is really a solution, just a kludge).

This problem occurs whether primary-links is disabled or not in the dhtml_menu preferences.

Subscribing

Subscribe.

Subscribing.
Looking forward to have Ubercart Catalog working with dhtml menu...

I solved this problem the same as post #5. I redirected Nice Menu's call to theme('menu_item_link') to a local function. I just copied the theme_menu_item_link() code from the API and everything seems to be working now. I've attached the patch file.

I think that the problem is that this module wrongfully assumes that there will be one call to theme('menu_item') for each call to theme('menu_item_link'). As mentioned in #5, this module intercepts calls to these theme functions and adds the link to the dhtml stack—the problem is that items are only removed from the stack on the theme('menu_item') call, a step skipped my the Nice menus module.

My fix for this is to send only the current stack item to the _dhtml_menu_subtree() function, instead of the entire stack, since it can contain extra unneeded items.

And this works? The entire reason for the stack is because the menu has to have a path to get the sub-tree, since it may have to load a sub-tree at any depth. I'll make sure to test this with items that are in the active trail or always expanded.

As expected, this breaks all menus below the active trail. Visit /admin, and all its sub-items appear as leaves, because the "admin" item was already expanded by the core menu.inc (active trail is expanded), and DHTML Menu had to load the items below /admin/build etc. To find /admin/build in the tree, it needs to know where /admin is, requiring the entire stack.

You're right Arancaytar, big -1 for #10. I had noticed that _dhtml_menu_subtree() always returned an empty array if there was a Nice menu on the page, and I thought that the problem was with the stack, but maybe it's something else.

Well, the problem *is* with the stack, naturally. It's just that getting rid of the stack isn't as trivial as a one-liner - it has to be replaced with a better approach.

I have experimented with expanding the tree-indexer already contained in DHTML Menu (for translating mlid into the sortable "weight-mlid-name" key used by the tree) to also index parents. If each item's parent path is contained in the index, then the module only needs the final item in the stack to find it - and any previous calls to theme_menu_item_link by nicemenu are just ignored.

Here's a patch for that. Haven't tested with NiceMenu yet, but it seems to work on its own - good start.

The extra indexing may cost some performance, but not noticably so.

Patch in #14 works fine for me, thanks Arancaytar.

Based on that, I have committed this to DRUPAL-6--3. I'm rushing it a bit, but it's an annoying bug and I could find no problems in the self-review.

This patch requires porting to D7. It is already implemented in the massive patch in #473356: DHTML Menu takes another leap, but needs to be isolated and committed separately. I've found that rewriting the same change three times is a passable substitute for a second pair of eyes.

After this fix, 6.x-3.5 will likely be released soon, but I would appreciate it very much if someone could test the 6.x-3.x-dev snapshot once it updates... getting that tested will greatly speed up the release process.

Thank you very much... 3.5 fixed it perfectly.

Keep up the excellent work, you're fast!!

Yep =)

Okay, here comes a patch for HEAD.

Still applies, and I didn't notice any problems when I tested it two months ago. This being D7, I just commit it and see if it works as it should.

Done.

Augh.

Step 1:
- Upload a patch late at night and tell yourself you're going to test it later
- Two months down the road, commit the patch on the basis that "I don't remember anything breaking"
- Introduce syntax errors.
- Headdesk.

Also, I neglected a logic change required in the _stack() function, where only the popped element must be returned, rather than the whole stack.

Here's an incremental fix for both problems. It's pretty vital to get this in as my module's HEAD is broken since yesterday. >_<

Now in HEAD.

Automatically closed -- issue fixed for 2 weeks with no activity.