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.
Comment | File | Size | Author |
---|---|---|---|
#17 | 571224-id-3.patch | 40.04 KB | redndahead |
#16 | 571224-id-2.patch | 39.43 KB | redndahead |
#15 | 571224-id-1.patch | 39.29 KB | redndahead |
#11 | id-571224-3.patch | 22.71 KB | hadsie |
#10 | id-571224-2.patch | 32.85 KB | hadsie |
Comments
Comment #1
hadsie CreditAttribution: hadsie commentedAfter some more searching, I think I've found the source of the issue.
In views_slideshow_thumbnailhover.module you define the theme function:
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:
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.
Comment #2
redndahead CreditAttribution: redndahead commentedThink you can create a patch and attach it? Here is info on how to create a patch http://drupal.org/patch/create
Comment #3
hadsie CreditAttribution: hadsie commentedwould you prefer if i changed the 'id' argument across all the theme functions or just the one that there's definitely an issue with?
Comment #4
redndahead CreditAttribution: redndahead commentedI 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.
Comment #5
hadsie CreditAttribution: hadsie commentedHere'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?
Comment #6
hadsie CreditAttribution: hadsie commentedrerolled against latest 6--2 code.
Comment #7
redndahead CreditAttribution: redndahead commented@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.
Comment #8
hadsie CreditAttribution: hadsie commentedI 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.
Comment #9
redndahead CreditAttribution: redndahead commentedI think this patch is a bust anyway. I changed too many variables. Have a look before you go through a reroll.
Comment #10
hadsie CreditAttribution: hadsie commentedYeah, 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.
Comment #11
hadsie CreditAttribution: hadsie commentedI'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.
Comment #12
redndahead CreditAttribution: redndahead commentedMarking as critical to look at before release.
Comment #13
redndahead CreditAttribution: redndahead commentedSorry 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.
Comment #14
redndahead CreditAttribution: redndahead commentedComment #15
redndahead CreditAttribution: redndahead commentedHere is an updated patch.
Comment #16
redndahead CreditAttribution: redndahead commentedThis one changes the id to the view id. Which should hopefully help caching issues.
Comment #17
redndahead CreditAttribution: redndahead commentedThe view_id setting isn't necessary anymore. Removed that and replaced with vss_id where necessary.
Comment #18
Shane Birley CreditAttribution: Shane Birley commentedYay! I will try it out and post about what happens.
Comment #19
redndahead CreditAttribution: redndahead commentedI 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.
Comment #20
Shane Birley CreditAttribution: Shane Birley commentedI 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.
Comment #21
redndahead CreditAttribution: redndahead commentedYay!! 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.
Comment #22
Shane Birley CreditAttribution: Shane Birley commentedI 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!
Comment #24
Fidelix CreditAttribution: Fidelix commentedUsing 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!