This is a great tool, especially for dealing with the admin navigation, however I had need of using it for a navigation block that would function as the primary navigational element for a Client site, and I wanted it to "be prettier". Thus I added in the show() hide() functions from jQuery into the code, and I wanted to give the admin some control over the speed of that effect. To reach that end I added an admin page for the dhtml_menu that allows the administrator to determine the speed at which the navigational system would expand.

Default expansion will look no different from the current module, however it can be changed from instant to "slow" or "fast".

Additionally I altered the javascript addition of "block" as the display type to "inline" as this SHOULD fix the double margin bug in IE and won't effect the output of standards compliant browsers.

This is my first attempt at a real tested patch and everything appears to be in order, however I'd prefer it if someone were to look over the code.

I modified both the .module and the .js I was unsure how to upload both, so the module is in this post and the js will be in the next.

Eclipse

CommentFileSizeAuthor
#4 dhtml_menu_1.patch3.39 KBeclipsegc
#3 dhtml_menu_0.patch3.26 KBeclipsegc
#1 dhtml_menu_js.patch976 byteseclipsegc
dhtml_menu_module.patch1.67 KBeclipsegc

Comments

eclipsegc’s picture

eclipsegc commented
StatusFileSize
new dhtml_menu_js.patch976 bytes

The js patch as promised.

eclipsegc’s picture

eclipsegc commented

Seems I missed the access on the admin menu item for this. I'll fix it tomorrow and put up a new patch.

eclipsegc’s picture

eclipsegc commented
StatusFileSize
new dhtml_menu_0.patch3.26 KB

OK, so thanks to James Walker I've got the .js and .module file all contained in 1 .patch file now. Thank you James.

I updated the access to the menu. This should work as expected now.

I'd love some feed back, so let me know what you think.

Eclipse

eclipsegc’s picture

eclipsegc commented
StatusFileSize
new dhtml_menu_1.patch3.39 KB

Ok, after deploying this a couple places another bug was brought to my attention. It's squashed and menus should not stay closed when you tell them to. (heh) so yeah that was a pain.

Eclipse

eclipsegc’s picture

eclipsegc commented

BTW HUGE!!!! thanks goes out to iratik from #jquery. I'd still be VERY screwed on this patch if not for his interest and help.

eclipsegc’s picture

eclipsegc commented

My previous post about the bug is a bit ambiguous, not to mention stated wrong, so I'll reiterate here.

My previous patches suffered from a bug which would leave the most recently closed submenu open on the next page load. At first I thought this was some function of the admin menu, as it was invariably the last thing I closed, but then after some more experimentation and a couple other people playing with it on their own, it was obvious that any submenu did it, I just happened to always close admin last. Anyway, it was a real pain in the tail to kill. I'm still unsure why show()/hide() had that effect, something to do with states being saved before the css was set, but never-the-less this patch takes care of that now.

Eclipse

andrewfn’s picture

andrewfn commented
Status: Needs review » Needs work

Here are a few questions about this patch.

  1. It seems to duplicate what is already in head (correct me if I am wrong). It may do the job better, I'm not sure. However, this is confusing.
  2. The patch can't be applied against head. Quite a lot has gone in since 5.x-0.7
  3. dhtml_menu_menu() is missing the $may_cache argument, which the module documentation warns against doing.
andrewfn’s picture

andrewfn commented

I decided to do some comparative testing of the two methods of getting JavaScript slide effects: the method in head which uses slideUp/Down, and the method in this patch which uses hide/show. The effect of hide/show looks nicer (to me) and is a little more robust.

Method:
slideUp/Down           hide/show
FF/Windows
perfect               perfect
FF/Linux
perfect               perfect
Konqueror
perfect               perfect
IE7       
fails                     breaks lower menus left open
IE6       
fails                     breaks lower menus left open

Testing was done using the Zen theme. I was able to break IE6 and IE7 with hide/show by opening several lower menus, and then opening and closing one near the top. Artifacts from opened menus corrupted the screen.

brmassa’s picture

brmassa commented
Status: Needs work » Fixed

Guys,

its mainly fixed with the new jQuery within D6.

regards,

massa

Anonymous’s picture

Anonymous (not verified) commented
Status: Fixed » Closed (fixed)

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