Use behaviors nstead of $(document).ready(). This fixes an issue with at least one other module ( http://drupal.org/project/getlocations ): http://drupal.org/node/1409186#comment-5656404

Drupal.settings within the function could now be changed to settings but I've left it alone for this patch.

Potentially related: #1279870: Doesn't work with iPad

CommentFileSizeAuthor
#6 use-drupal-7-jquery-wrapper.patch585 bytespat redmond
#3 megamenu-behaviors.patch594 byteshackwater
megamenu-behaviors.patch602 bytesimclean

Comments

imclean’s picture

imclean commented
Status: Active » Needs review

Updated status.

Anonymous’s picture

Anonymous (not verified) commented
Assigned: Unassigned » erykmynn
Priority: Normal » Critical
Issue tags: +lowhanging, +review
hackwater’s picture

hackwater commented
StatusFileSize
new megamenu-behaviors.patch594 bytes

This patch helped ease a conflict between my theme's .js file and the megamenu.js file. When I used the Drupal.behaviors.theme_name to attach my Javascript and cycle through some theme items, it broke the megamenu dropdown functionality. This patch resolved the issue, letting me use Javascript in my theme and the megamenu in my nav.

I've slightly modified the original patch to follow the suggested way of managing Javascript in Drupal 7. I don't know that this counts as RTBC, but I wanted to chime in.

Dinesh Kumar Sarangapani’s picture

Dinesh Kumar Sarangapani commented
Status: Needs review » Needs work 
+++ b/megamenu.jsundefined
@@ -184,5 +185,6 @@ $(document).ready(function() {
 })(jQuery);

There should be no new line at end of the file

imclean’s picture

imclean commented

The error is stating there isn't a newline when there should be. See: http://drupal.org/node/1319154

If there isn't a newline in the original file I'm not sure this patch is the place to fix it. A separate patch to fix such coding issues in all files could be more appropriate.

pat redmond’s picture

pat redmond commented
Assigned: erykmynn » Unassigned
Status: Needs work » Reviewed & tested by the community
StatusFileSize
new use-drupal-7-jquery-wrapper.patch585 bytes

Here is a patch which will use D7 jquery wrapper. It also uses the megamenu module name, rather than exampleModule from post #3

Now that I look at it, this is the same as the first patch from several months ago. This is ready to be committed.

If the maintainers don't have the capacity to deal with this ATM, I am happy to become a co-maintainer in order to get these patches committed.

Anonymous’s picture

Anonymous (not verified) commented

Thank Pat, I've granted you write access to the repository. I appreciate your help in nocking off these minor issues that have been laying around.

Anonymous’s picture

Anonymous (not verified) commented
Assigned: Unassigned » pat redmond
pat redmond’s picture

pat redmond commented

Hey FilmKnurd - thanks! I am planning to fix a few issues as I have a site going live shortly, and I'd love to use this module on it.
I just commited this, but I commited it to master rather than 7.x-1.x. Can you check that for me, and let me know if it is OK?

Anonymous’s picture

Anonymous (not verified) commented

Oh yeah, master isn't used. You'll have to commit it to the 1.x branch.

pat redmond’s picture

pat redmond commented

OK, super. I've committed to 7.x-1.x
I notice there is a 7.x-2.x in version control, but it seems empty..?

Anonymous’s picture

Anonymous (not verified) commented

We keep getting false starts on the over-haul. JohnAlbin was going to head up the re-write first, but then he got busy with all this responsive nonsense :-) rgarand has the re-write almost done... but hasn't posted it yet. Not sure why.

thekatic’s picture

thekatic commented

I noticed some problems after replacing $(document).ready() with Drupal behaviors.

When using megamenu with modules or custom code that calls attachBehaviors, you will notice decrease in page performance. In my case Views exposed filters with Ajax.
So while you hitting search button, page will become slower and slower. This will happen because click actions are not unbound but instead added to the previous action.

Is this a known problem which will be fixed in 7.x-2.x?

ram4nd’s picture

ram4nd commented
Assigned: pat redmond » Unassigned
Issue summary: View changes
Priority: Critical » Normal
Status: Reviewed & tested by the community » Needs work
Issue tags: -lowhanging, -review