i-like-robots/jQuery-Slideshow

oncomplete callback function doesn't work from 1.6.0 or later

Closed this issue · 2 comments

There's either a bug affecting the oncomplete callback function or we need a line of documentation explaining how it's supposed to be used. I went back release by release to test if it used to work the way I expected it to and it worked in versions 1.5.2 and 1.5.1. I didn't go any further back from there.

Here's a condensed version of what I have in my HTML

    <div class="slideshow" data-oncomplete="saveStep">...</div>

I'm naming my callback function in the data-oncomplete property of the div tag. The name of my callback function is saveStep, which just stores the index number of the currently viewed slide in a cookie so a user can return to the same slide that was last viewed easily.

The error I keep getting is:
TypeError: 'undefined' is not a function (evaluating 'this.opts.oncomplete.call(this, this.current)') (slideshow.js:147)

Line 147 of version 1.7.0:

this.opts.oncomplete.call(this, this.current);

If I change it the following, it works:

saveStep.call(this, this.current);

So the slideshow appears to be accessing the function that I setup, but it won't access it when it's being referenced from this.opts.oncomplete.

That's odd, so I write this.opts.oncomplete to log and I see that it's being interpreted as a string instead of a function when called this way.

I also tried every version in between 1.7.0 and 1.5.2 and had the same problem on all of them. I didn't test the onupdate callback, but I'm assuming it would have the same issue for me.

BTW, this plugin was perfect for me. Much thanks!

BTW, if anyone else is having this same issue and going back to a previous version isn't an option and you don't want to modify the original code, you can get around this by directly setting the contents of the oncomplete property when invoking the slideshow.

var slideshow = $('.slideshow').slides(),
api = slideshow.data('slides');

api.opts.oncomplete = function(current) {
  alert('the index number is: '+current);
}

api.opts.onupdate = function(current) {
  alert('put your function actions here');
}

@hyperlinked thanks for your detailed information. As soon as I started reading I thought "That's because it's a string" so thank you for saving me time checking that too ;-)

I can't think of a reasonable solution to this because functions should be passed by reference within JavaScript and any way of interpreting the string gleaned from the DOM into JS (eval) is best avoided. The other option I can think of is to test if the given property is available within the global namespace, I.E.:

if (typeof this.opts.oncomplete === "string" && window[this.opts.oncomplete]) {
  window[this.opts.oncomplete].call(...);
}

But I think that's somewhat unexpected behaviour. A test to see whether or not the option is callable is probably a sensible step to avoid errors and adding a precautionary note about usage too. However, I don't think adding functionality to interpret a string and best guess what the author meant is a good feature.