Download & Extend
SimpleMenu » Issues

Working version for Drupal 6

Posted by starbow on January 19, 2008 at 12:42am
Project:SimpleMenu
Version:5.x-5.0
Component:Code
Category:feature request
Priority:normal
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

Here is a patch for 5.x-4.0 to get it working with Drupal 6. Because I just can't live without this module!

AttachmentSize
simplemenu.patch11.31 KB
Login or register to post comments

Comments

#1

Posted by m3avrck on January 26, 2008 at 10:05pm

Thanks for the patch! I'll explore Drupal 6 soon with some ideas to further improve this module, thanks for the starter patch!

#2

Posted by coltrane on February 14, 2008 at 11:05pm
Status:needs review» needs work

Patch didn't apply cleanly for me to 5.x-4.0 and Drupal 6.0.

(Stripping trailing CRs from patch.)
patching file simplemenu.css
(Stripping trailing CRs from patch.)
patching file simplemenu.info
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file simplemenu.info.rej
(Stripping trailing CRs from patch.)
patching file simplemenu.js
(Stripping trailing CRs from patch.)
patching file simplemenu.module

Continuing anyways and manually adding core = 6.x to simplemenu.info and enabling the module the navigation menu doesn't show in the simplemenu. Going to the simplemenu config page and submitting it with Navigation set as the menu doesn't fix it nor does selecting another menu.

Also a javascript 404 error is given on every page for simplemenu/menu.

#3

Posted by ax on February 17, 2008 at 12:52am
Version:5.x-4.0» 5.x-5.0
Status:needs work» needs review

try this patch (against latest 5 version, ie. either DRUPAL-5 or DRUPAL-5--5-0). based on starbows patch, with the following changes:

* updated superfish.js to latest version 1.4.1 to work with Drupal 6's jQuery 1.2.3.
* moved superfish.js out of simplemenu.js to ease further upgrades
* don't append <ul class="menu"> (the menu returned by simplemenu_theme_menu_tree()) to <ul id="simplemenu">, but make former the latter (by adding 'id="simplemenu"' to it). this dom hierarchy is what superfish,js and *.css expect (and why the menu didn't show before).
* removed the "Add devel module links" functionality. the much changed Drupal 6 devel.module now returns its links through a custom menu, so it can be put anywhere in the menu tree shown by simplemenu.
* fixed simplemenu_expanded_tree_page_data(). this shouldn't be a copy of menu_tree_page_data(), which returns a menu based on the current page, but of menu_tree_all_data(), which returns the *whole* menu for a custom menu. thats why i renamed it to simplemenu_tree_all_data(). modified it to accept any menu item as root param, not just the root of a custom menu).

this works for me as of today.

TODO:
* get (a modified version of) simplemenu_tree_all_data() into core. the functionality of getting all menu items below a certain root item certainly is not only needed by simplemenu. menu_tree_all_data() unfortunately doesn't do this - it only provides (as alternative to getting the whole tree of a custom menu) the tree from the root of that custom menu down to a certain item. but not all items below a certain item.

AttachmentSize
simplemenu_d6.patch 18.08 KB

#4

Posted by ax on February 17, 2008 at 2:02am

missed a case. we don't want the "Select the menu to display." (eg. "Administer") to display, only its submenus (eg. Content Management, Site building, etc.)

AttachmentSize
simplemenu_d6.patch 18.07 KB

#5

Posted by jbrauer on February 17, 2008 at 8:43pm
Status:needs review» reviewed & tested by the community

+1

This patch works like a charm against DRUPAL-5

#6

Posted by ax on February 18, 2008 at 12:43am

does anyone mind if i commit this to DRUPAL-6--1 and create a 6.x-1.x-dev release?

#7

Posted by thund3rbox on February 18, 2008 at 12:57am
Status:reviewed & tested by the community» needs work

Patch failed for me:
"patching file simplemenu.info
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file simplemenu.info.rej
patching file simplemenu.js
patching file simplemenu.module
patching file superfish.js
"

Added "core = 6.x" manually to .info file, enabled and works fine (so far).

AttachmentSize
simplemenu.info_.rej_.txt 383 bytes

#8

Posted by ax on February 18, 2008 at 1:24am
Status:needs work» reviewed & tested by the community

did you patch against a clean DRUPAL-5/DRUPAL-5--5-0 simplemenu install? there are no patching errors, i just tried again. you probably had simplemenu.info patched already. setting back to RTBC.

#9

Posted by m3avrck on February 18, 2008 at 1:39am

I'll properly create a 6.x version and branch once this patch has been ironed out and I can fix a few other things.

#10

Posted by jbrauer on February 18, 2008 at 4:56am

Regarding #7 I got this same error when I tried to patch against the tarball and solved it the same way. When I patched a fresh CVS copy of DRUPAL-5 the patch applied correctly.

#11

Posted by marafa on February 22, 2008 at 11:52am

is this downloadable as a module yet?

#12

Posted by m3avrck on February 22, 2008 at 5:11pm

I'll get this packaged up this weekend...

#13

Posted by ax on February 26, 2008 at 6:27pm

@m3avrck: my offer of committing this to DRUPAL-6--1 and creating a 6.x-1.x-dev release is still valid :)

#14

Posted by anantagati on February 27, 2008 at 12:10pm

Patch from #4 works good. Applied on DRUPAL-5--5-0.

#15

Posted by thomas_green on February 27, 2008 at 4:27pm

I'd love it if you'd check it in to DRUPAL-6--1.

#16

Posted by ax on February 28, 2008 at 1:18pm

@m3avrck: you give me a go?

#17

Posted by m3avrck on February 28, 2008 at 7:43pm

reviewing all of this and committing it in a few :)

#18

Posted by m3avrck on February 28, 2008 at 9:48pm
Status:reviewed & tested by the community» fixed

thanks guys this has been committed!

there is still an issue if you use the "black & blue" theme as the menu doesn't have the black bg anymore.

no time to look at that now but if someone has a patch for that let me know!

#19

Posted by 00drup00 on February 29, 2008 at 6:36pm
Title:Working version for Drupal 6» Working version for Drupal 6 - IE6 bug?
Version:5.x-5.0» 6.x-1.x-dev
Category:feature request» support request
Assigned to:Anonymous» 00drup00
Status:fixed» active

Did anyone check this version to work properly in IE6?

It displays right in ff and ie7.
But not in ie6, see attachment.

Is this a js or css problem?

Appreciate your comments.

Thank you.

AttachmentSize
smp-mn-ie6.jpg 22.93 KB

#20

Posted by ax on March 1, 2008 at 3:07am
Title:Working version for Drupal 6 - IE6 bug?» Working version for Drupal 6
Version:6.x-1.x-dev» 5.x-5.0
Category:support request» feature request
Assigned to:00drup00» Anonymous
Status:active» fixed

works for me with the original theme. note what m3avrck said above:

there is still an issue if you use the "black & blue" theme as the menu doesn't have the black bg anymore. no time to look at that now but if someone has a patch for that let me know!

please submit a new issue (and if possible a patch) for that one. thanks!

#21

Posted by Anonymous (not verified) on March 15, 2008 at 3:14am
Status:fixed» closed (fixed)

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