Needs work
Project:
Views Slideshow JCarousel
Version:
7.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Reporter:
Created:
6 Jan 2014 at 11:21 UTC
Updated:
23 Sep 2014 at 01:02 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
aburrows commentedComment #2
aburrows commentedComment #3
aburrows commentedComment #4
jomarocas commentedpatch # works for me
Comment #5
netw3rker commentedI like the idea of combining resources with the jcarousel module team especially since the two modules do the same thing, just via a different mechanism. But, I'm not sure the two will ever quite make that happen. This is mainly because they both provide views handlers. This basically means that If you have one, you really have no reason to use the other, a team needs to standardize on one particular path.
That being said, I'm game for encouraging convergent evolution between the two modules, and this is a good step. There are a couple of issues with the patch that I'd like to see addressed first, and those guys will also need to get motivated to get their 3.x branch more accessible [see my points below after that] before I can see this go in.
1) Your patch does degrade properly which is good, but it doesn't test for version dependencies. right now that's not a problem because jcarousel-7.x-2.x is the most commonly available version, but when/if they release 7.x-3.x this code will fail because paths wont line up correctly. if you can get a version check in, that'll make me feel better.
2) the includes of the jcarousel js libraries go directly to their packaged versions (not supposed to do that - see below) I would much prefer a call to a jcarousel function that loads the libraries from "somewhere", and our code doesn't need to be conscious of it. - you might have to work with them on that.
here's my discussion point mentioned earlier:
The Drupal standard for handling external libraries is to not distribute them w/in a module - which is what the jCarousel module (incorrectly) does w/in the 7.x-2.x line. There, the version of jcarousel that its using is backdated to (I think) 0.2.4 whereas the most recent 2.x line is 0.2.9. They do have a hidden 3.x dev branch but AFAIK they don't plan to make the leap to 0.3.X anytime soon. I like the idea of coordinating resources, but they don't seem to be moving very fast on this either.