I'm using the views_slideshow_thumbnailhover module. I want to add additional theming to the breakout-teaser(s) theme functions so I've overridden them with .tpl.php files in my theme.

The code is identical in my .tpl.php files (except I'm printing $output instead of returning it).

The problem I'm having is that the $id variable is not being properly set in my views-slideshow-thumbnailhover-breakout-teaser.tpl.php. The $id stays as 1 through all the $output .= theme('views_slideshow_thumbnailhover_breakout_teaser', $item, $id, $count); in theme_views_slideshow_thumbnailhover_breakout_teasers, but increments by 1 in the breakout-teaser function. This is really bizarre behaviour and I can only guess it's due to a preprocess function somewhere that's also setting the 'id' variable, however, I've not been able to find anything.

When I modify the theme registry definition of the breakout_teaser theme, changing the parameter 'id' to something like 'slideshow_id' everything works as it should.

I hope what I've said makes /some/ sense ;-). But basically, for whatever reason it's not possible for me to create a custom template for the theme_views_slideshow_thumbnailhover_breakout_teaser function unless I change the theme registry definition.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hadsie’s picture

After some more searching, I think I've found the source of the issue.

In views_slideshow_thumbnailhover.module you define the theme function:

    'views_slideshow_thumbnailhover_breakout_teaser' => array(
      'arguments' => array('item' => NULL, 'id' => NULL, 'count' => NULL),
      'file' => 'views_slideshow_thumbnailhover.theme.inc',
    ),

Using the 'id' as the name of one of the parameters. However, in template_preprocess() in the main drupal theme.inc, the following code is run:

  $variables['id'] = $count[$hook]++;

So drupal is overriding the 'id' value. The reason this works ok in the module in the first place is because you're using theme functions not theme templates. The function definition is theme_views_slideshow_thumbnailhover_breakout_teaser($item, $id, $count) which explicitly defines the $id variable, which can't be done in a template file.

So, the fix for this would be to rename all of the 'id' parameters in the modules hook_theme to something like 'vss_id', and then use that value in the theme code.

redndahead’s picture

Think you can create a patch and attach it? Here is info on how to create a patch http://drupal.org/patch/create

hadsie’s picture

would you prefer if i changed the 'id' argument across all the theme functions or just the one that there's definitely an issue with?

redndahead’s picture

I think consistency would be best so across all of them. I'm haven't taken a hard look at your issue yet so I can't make a definitive statement on that.

hadsie’s picture

Status: Active » Needs review
FileSize
10.51 KB

Here's the patch. It changes id => vss_id

Also, I noticed the call in theme_views_slideshow_thumbnailhover_breakout_teasers():

$js .= theme('views_slideshow_thumbnailhover_breakout_teaser_js', $vss_id, $count);

But I couldn't find this theme function anywhere and it seems to always return nothing. What's it used for?

hadsie’s picture

rerolled against latest 6--2 code.

redndahead’s picture

FileSize
31.88 KB

@hadsie Could you verify that this patch works for you? It goes against the latest and includes fixes for views_slideshow_singleframe as well as extending to javascript values.

hadsie’s picture

I just updated to the latest dev code and this patch fails now. I'll re-roll another one and post it here a bit later today.

redndahead’s picture

Status: Needs review » Needs work

I think this patch is a bust anyway. I changed too many variables. Have a look before you go through a reroll.

hadsie’s picture

FileSize
32.85 KB

Yeah, I went through it and I didn't notice anything bunk. Here's a new one. Still testing it and I'll report back if I run into issues.

hadsie’s picture

Status: Needs work » Needs review
FileSize
22.71 KB

I've just re-rolled the patch against the most recent cvs code from the 2.x branch.

This also fixes a bug in contrib/views_slideshow_thumbnailhover/views_slideshow.js

On line 23:
pager:(settings.pager_event == 'hoverIntent') ? null : '#views_slideshow_breakout_teasers_' + settings.id,

views_slideshow_breakout_teasers_ should actually be views_slideshow_thumbnail_breakout_teasers

Cheers.

redndahead’s picture

Priority: Normal » Critical

Marking as critical to look at before release.

redndahead’s picture

Status: Needs review » Postponed

Sorry I know that you worked hard on this, but I'm going to postpone it for now. You can add these functions to your template preprocess for now and it will override our function so it is themeable just not probably the best way. Will try to look at it early after release.

redndahead’s picture

Priority: Critical » Normal
redndahead’s picture

Status: Postponed » Needs review
FileSize
39.29 KB

Here is an updated patch.

redndahead’s picture

FileSize
39.43 KB

This one changes the id to the view id. Which should hopefully help caching issues.

redndahead’s picture

FileSize
40.04 KB

The view_id setting isn't necessary anymore. Removed that and replaced with vss_id where necessary.

Shane Birley’s picture

Yay! I will try it out and post about what happens.

redndahead’s picture

Status: Needs review » Fixed

I went ahead and committed this. I want to make a 2.1 release on the 20th can you please try the newest dev release that should be dated the 17th or 18th and report back. You will probably need to clear the cache on your site.

Shane Birley’s picture

I have loaded 6.x-2.x-dev from May 18th. Cached the site up to the hilt and I will watch for the next little bit. Javascript error that I was getting is now gone which means all the js is now being allowed to complete.

redndahead’s picture

Yay!! If I don't hear anything from you by the 20th. I'll go ahead and release 2.1. Thanks for testing you don't know how much that helps.

Shane Birley’s picture

I have this running on the site (testing on 5 different sites with different slideshow requirements) with Drupal core cache and Views cache running and it all seems to be working! I will say that this has solved this issue! It has been running since yesterday and the problem has not returned.

I will now do a happy dance... you ROCK!

Status: Fixed » Closed (fixed)

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

Fidelix’s picture

Using the last DEV solved C-O-M-P-L-E-T-E-L-Y the problem in my drupal install.
I'm so frakking happy it worked!

THANK you guys, i mean it!