Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Here's the project page over at github: https://github.com/joeldbirch/superfish
Comment | File | Size | Author |
---|---|---|---|
#6 | nice_menus-upgrade-superfish-library-2001616-6.patch | 2.86 KB | xiukun.zhou |
#5 | nice_menus-upgrade-superfish-library-2001616-5.patch | 2.09 KB | xiukun.zhou |
Comments
Comment #1
xiukun.zhou CreditAttribution: xiukun.zhou commentedthanks klonos.
http://users.tpg.com.au/j_birch/plugins/superfish/
drupal 7 jquery default version is 1.4.4 not support superfish 1.7, you can using hook_library_alter().
Comment #2
klonosI realize that. The point is to allow users to upgrade to newer versions of the libraries easily without having to write any code. This can be done with #1411268: Support Libraries API for the js files..
Do you think that you can consider creating a new 7.x-3.x branch that will add jQuery Update and Libraries API as requirements (that is common for many modules since D7 core is left so far behind with jQuery). Then completely remove the libraries shipped with the module (besides nice_menus.js I guess) and require people to download and place all required libraries under /sites/all/libraries?
This way, people that want something that simply works out of the box will use the 7.x-2.x branch, while more advanced people that want to upgrade to latest versions can use the 7.x-3.x branch.
Another idea is to do what jQuery Update 7.x currently does. It includes a /replace/jquery directory that in turn has a /1.5 a /1.7 and a /1.8 subdirectories. So, you can also ship the module with multiple versions of the libraries and then simply check for the version of jQuery used and based on that switch to using the respective supported version.
I still believe that no matter what, fixing #1411268: Support Libraries API for the js files. first is the right way to go. This will take the burden of shipping libraries off from you and make it a task of the site builder.
Comment #3
xiukun.zhou CreditAttribution: xiukun.zhou commentedThanks klonos.
I would consider this idea.
#1411250: Upgrade included hoverIntent library to r7 (currently shipping r5) and #2001612: Upgrade included bgiframe library to 3.0.1 (currently shipping 3.0.0) will join.
Comment #4
klonosThanx for understanding and considering this. It's really important to be able to upgrade libraries without having to code something.
Comment #5
xiukun.zhou CreditAttribution: xiukun.zhou commentedHi klonos,
I think this is a very good idea and I have tried to spend some time to try to get to an initial testing version of this feature.
So basically, for now, I have created a simple patch for testing to see if we could detect for different JQuery versions to be supported.
The attached patch:
nice_menus-upgrade-superfish-library-2001616-5.patch
should support:
Download plugins to:
we could modify the file names and folders path if necessary later on.
If necessary, we could surely create another branch for that, but with this patch, I have the feeling updated JQuery functions could be supported.
Let's try to do some testing of this patch and we will see how to move forward after some feedback.
I would greatly appreciate to have your feedback, reviews, testing and reporting on this approach and patch/code.
Please let me know if you would have any questions, comments, issues, recommendations, objections or concerns on the attached patch or any of the aspects discussed in this comment, I would certainly be glad to provide more information, explain in more details or re-roll the patch if necessary.
Thanks very much in advance to all for your feedback, reviews, testing and reporting.
Cheers!
Comment #6
xiukun.zhou CreditAttribution: xiukun.zhou commentedi have updated the patch, Thanks everybody tests the patch
Comment #7
klonosSorry, I simply didn't find time to test this out. I promise I will as soon as I can though.
Comment #8
roymuir CreditAttribution: roymuir commentedJust patched this module and downloaded the files into the Library. I can confirm that this indeed does work. It solved my issue of having to double tap Nice Menu drop down menu parents to activate the drop down on iOS. Thanks very much!
Comment #9
deggertsen CreditAttribution: deggertsen commentedFor some reason when I went from 7.x-2.3 to 2.x with the patch in #6 it broke all the javascript on my site. Not even the menus worked right (no fade in/fade out). I'm not sure what details would help, but as soon as I found out it wasn't working I switched back to 2.3 and everything was fine again.
Comment #10
deggertsen CreditAttribution: deggertsen commentedAh, I figured out my problem. I had to make sure I had jQuery Update set to at least version 1.7 rather than 1.5.
So with this patch jQuery 1.7+ is necessary. 1.8 is better because then when you touch a drop down it doesn't load the page until you've tapped it a second time.
Comment #11
xiukun.zhou CreditAttribution: xiukun.zhou commentedThanks deggertsen, klonos and roymuir to help me test the patch
Comment #12
kkuhnen CreditAttribution: kkuhnen commentedThanks xiukuk.zhou - this patch works for me.
I did initially have the same issue as deggertsen (jQuery update was set to 1.5) but once this was set to 1.7+ everything worked as expected.
Thanks again!
Comment #13
deggertsen CreditAttribution: deggertsen commentedShould this be marked as RTBC? It is working and has been confirmed so by 4 users. It may not be good to commit simply because it would then create a jQuery Update Dependency, but that's up for debate.
Comment #14
klonos...I think that in order to avoid the issue of people forgetting to switch to jQuery 1.7 even after installing jQuery Update we should detect the jQuery version and throw some warning with a link to the jQuery Update settings page.
Even better, in order to keep things clean I say we switch to a new 7.x-3.x branch and make jQuery Update a requirement officially. That accompanied with proper documentation in both the README.txt as well as the project's page would suffice.
Comment #15
DrupalGideonPersonally as a user of this module I would prefer to see a 7.x-3.x release branch with a dependency on jQuery Update module. That makes perfect sense. And the quickest way to get this released.
Comment #16
xiukun.zhou CreditAttribution: xiukun.zhou commentedsee: ed1af8e
Download plugins to:
site/all/libraries/jquery.hoverIntent/jquery.hoverIntent.js
site/all/libraries/jquery.bgiframe/jquery.bgiframe.js
site/all/libraries/superfish/superfish.js
Comment #17
klonos...proposal moved to #2036601: Create a 7.x-3.x branch to support the latest versions of required js libraries - Keep 7.x-2.x compatible with default Drupal.
Comment #18
klonos...1.7.4 is out.
Comment #19
gillarf CreditAttribution: gillarf commentedWhat should I patch this against? Tried it against the current 7.x-2.x-dev version, and the patch failed:
Hunk #1 FAILED at 165.
Comment #20
xiukun.zhou CreditAttribution: xiukun.zhou commentedHi gillarf:
download 7.x-2.5
Comment #21
gillarf CreditAttribution: gillarf commentedThanks - just tried that, and got the same patch error.
The patch looks straightforward though, I'll try to apply the changes manually and see if that works.
Comment #22
klonos...this is now fixed in 7.x-3.x. The library does not mention any minimum/maximum jQuery requirements, so I guess that it could be packported to 7.x-2.x. It does recommend hoverIntent r7 though and that in turn requires jQuery 1.9.1+. I currently have no interest to keep using 2.x, so if somebody tests latest Superfish in 7.x-2.x with hoverIntent r7 without using jquery_update and they report it works, please feel free to reopen this issue and request backport.