Comments

drclaw’s picture

Maybe this post http://drupal.org/node/220711#comment-4486820 will help you? At least until a d7 port can be made.

likewhoa’s picture

Title: Port to D7 » Port Views Slideshow JCarousel Thumbs to D7
Issue tags: +port to d7. d7 porting

subscribing

Jerome F’s picture

subscribing

danny_joris’s picture

subscribing

jjclint’s picture

Any news about this?

imclean’s picture

Status: Active » Needs review
StatusFileSize
new55.05 KB

Here's a first attempt, this patch is against the git master branch and is partly based on a coder upgrade of the code in comment #4 #1074266: Slideshow Type in Slideshow 3 using Views 3.

It doesn't seem to work when admin_menu is enabled, possibly due a settings conflict. Still looking into it but I've been staring at it for too long today.

imclean’s picture

The problem relates to this line in admin_menu/admin_menu_toolbar/admin_menu_toolbar_modue:

drupal_add_css($path . '/admin_menu_toolbar.css', array('preprocess' => FALSE));

Setting it to preprocess=>true (the default) works:

drupal_add_css($path . '/admin_menu_toolbar.css');

I've no idea why this should be the case. Clearing the CSS file completely and leaving the line as is still causes problems so it isn't a css issue as such.

imclean’s picture

Found the problem: I missed the drupal_add_css() for skin.css in views_slideshow_jcarouselthumbs.theme.inc which was still using the D6 version.

It'd be a good idea to rename the css and js files to something more unique, (e.g. views_slideshow_jcarouselthumbs.css, views_slideshow_jcarouselthumbs.js) but this is probably best handled as a separate issue.

imclean’s picture

- Removed reference to jq module which doesn't have a D7 version
- Updated jcarousel to Version 0.2.8. This fixes a conflict with the jcarousel module.
- General tidying up

othermachines’s picture

@imclean

This is great. I'll give it a test run this week.

othermachines’s picture

@imclean

Hey, there. You have perfect timing, as Just a few days ago I had to implement this workaround: http://drupal.org/node/220711#comment-4486820. I plugged in the patched module-- an awesome piece of work. So far I've been test driving and haven't yet taken much of a look at the code.

Just one tiny nit so far:

Notice: Undefined index: views_slideshow_jcarouselthumbs in theme_views_slideshow_jcarouselthumbs_controls() (line 262 of C:\wamp\www\home_d7\sites\all\modules\contrib\views_slideshow_jcarouselthumbs\views_slideshow_jcarouselthumbs.theme.inc).

Should $options['views_slideshow_jcarouselthumbs']['timeout'] be $options['timeout'] ?

imclean’s picture

@othermachines, it certainly should be, thanks. One I missed.

There are also some pointless variable assignments created by the Coder module which need to be removed. See from theme_views_slideshow_jcarouselthumbs_control_previous() onwards, and a few others.

I'll tidy it up a bit more once I'm sure it's all working properly.

jenlampton’s picture

Thanks for all the movement on this. I've applied the patch, but am unable to see how I can make my views_slideshow use jCarousel for the pager. Where are the settings?

imclean’s picture

In the Views Slideshow settings for the view there should now be a dropdown down "Slideshow Type" where you can select "JCarousel Thumbs". It doesn't use the pager settings.

fearlsgroove’s picture

Status: Needs review » Needs work

There's a new 7.x-2.x branch in Git -- please create patches against that.

imclean’s picture

Status: Needs work » Needs review
StatusFileSize
new58.81 KB

Done. There's no 7.x-2.x in the Version drop down for the issue queue yet.

jenlampton’s picture

Version: 6.x-1.x-dev » 7.x-2.x-dev

It's there now :)

imclean’s picture

Nice! Although it looks like 7.x-2.x-dev on the project page is still the D6 version.

jenlampton’s picture

@imclean that's cause we need to provide patches first - that we can later use to update it :)

imclean’s picture

Ah, makes sense. Would be less confusing for others if it wasn't displayed on the project page until the patches have been committed. I think that's possible.

On that note, could people please test the patch in #16.

imclean’s picture

@jenlampton, how did you go with the patch?

fearlsgroove’s picture

Status: Needs review » Needs work

@imclean: Thanks for working on this. Can you please re-roll the patch without indentation changes? There are also several tabs and missing newlines at the end of a file.

Specifically in the .js file, wrapping the script in (function($) { is good, but leave an empty line after the function($) beginning, then leave the rest of the script starting with no indention. See any script in /misc for an example.

views_slideshow_jcarouselthumbs_theme also has a big diff that's mostly indentation.

imclean’s picture

@fearlsgroove: sure thing. Here's an update based on your comments.

I haven't introduced any tabs, they were in the original .js file. To be sure, my IDE uses 2 spaces for the tab key. I've still left the original tabs in, would it be preferable to remove them at this stage or remove them in the 6.x version first?

The function wrapper has been modified as suggested, however the "attach:" introduces some additional indentation to Drupal.behaviors. This means the .js diff still looks a little messy.

Some of the changes remove trailing spaces, some are Coder suggested changes (comma at the end of an array, the structure of arrays).

imclean’s picture

Some additional formatting fixes.

imclean’s picture

Status: Needs work » Needs review

Updated status.

redndahead’s picture

I made some comments and attached a module to this issue #1382324: Rewrite as a pager to the cycle slideshow style Would be nice to see what people think this module accomplishes for them.

jjclint’s picture

@imclean thanks for the drupal 7 port I'll test it today - have to undo the patch from http://drupal.org/node/220711 first.

fearlsgroove’s picture

Status: Needs review » Postponed

I don't recommend anyone spend much time on this particular issue anymore. Please see redndahead's link. That approach is MUCH cleaner with the current version of views slideshow and should port to D7 much easier.

sunfire-design’s picture

@imclean: Thanks for working on this.

But i have a problem after hovering a jcarousel thumb the slideshow doesn't resume.

jenlampton’s picture

@sunfire-design This is the issue queue for creating a D7 port.

if you have a separate problem, you should search for an issue relating to your problem. If you can't find one, then you should open a new issue.

sunfire-design’s picture

Problem solved with views_slideshow_jcarousel patched and optimized to D7

fearlsgroove’s picture

Status: Postponed » Closed (won't fix)