i have not tested myself locally but it look and works nice on the demo

since it relies on jquery/cycle and jquery/easing would you may consider it calling the jquery_plugin_add($plugin) function with the jquery_plugin. As there are other modules -or a theme- depending the same .js's

http://drupal.org/project/jquery_plugin
(jquery.easing.1.3.js is not there yet though and can be added)

there are pros and cons of a central js registry for module developers but a central js registry also might go to the core -work in progress
http://drupal.org/node/315100

CommentFileSizeAuthor
#8 views_showcase_485734_8_libraries.patch1.57 KBmiro_dietiker
#5 views_showcase-HEAD_libraries_integration.patch1.53 KBdboulet

Comments

rfsbsb’s picture

rfsbsb
Primary language Portuguese, Brazil
Location Kitchener
commented

Thanks for your comment. I once considered use jquery_plugins but on that time I thought that it could be more annoying than functional. But sure I'll consider it for future!

ica’s picture

ica commented

wise ... i agree on your timing :) -i just wanted to point out the possibility for consideration + central js registry discussions too

~obrigado

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented

jQuery plugins introduced by modules have huge potential to break other modules (introducing similar ones with potentially different versions).

What we really need is an abstraction module that does abstraction / registry only - not enforcing own versions.

Thus modules should then use the api to announce their needs (compatibility of jQuery modules).

Like wysiwyg adds the external editor libraries, it should be configured in a different library folder.

Even if we avoid double loading there's permanent trouble with upgrading libraries and ancountering side effects.

Compatibility information might also be added to the .info files and a good proposal would be a great start.

dboulet’s picture

dboulet commented

Maybe integration with the Libraries API module would be a good solution?

dboulet’s picture

dboulet commented
Version: 6.x-1.0-beta2 » 6.x-2.x-dev
Status: Active » Needs review
StatusFileSize
new views_showcase-HEAD_libraries_integration.patch1.53 KB
miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Needs review » Reviewed & tested by the community

Looks fine to me. I think we should apply this as-is.
Thank you very much for this improvement.

dboulet’s picture

dboulet commented 
+  if (module_exists('libraries')) {

We could also instead use function_exists('libraries_get_path') for a small performance boost.

miro_dietiker’s picture

miro_dietiker
Location Switzerland
commented
Status: Reviewed & tested by the community » Fixed
StatusFileSize
new views_showcase_485734_8_libraries.patch1.57 KB

Committed to 6--2
With comment, fast version.

Status: Fixed » Closed (fixed)

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