austenpayan / skippr

A super simple slideshow plugin for jQuery.
670 stars 133 forks source link

include velocity as module rather than inline #17

Open leviwheatcroft opened 8 years ago

leviwheatcroft commented 8 years ago

inlining velocity.js feels wrong.. I mean I found skippr when I was looking for a slideshow that would leverage velocity because I was already using it elsewhere on the page. I can probably create a PR for this if you think it's worthwhile.

austenpayan commented 8 years ago

I agree, I've had this on my list for a bit, but if you want to tackle it now that would be awesome.

leviwheatcroft commented 8 years ago

ok.. I'm just thinking about the best way to implement this.. I don't have a lot of experience with client side stuff.

I think requireJS or some kind of AMD library would be far too heavy handed, we're trying to reduce code after all.

Maybe we could do something like this?

$.skipperAnimate = $.velocity || $.animate
if (!$.velocity) {
  $.getScript( "velocity.js", function(data, textStatus, jqxhr) {
    if (jqxhr != 200) {
      console.log('error loading velocity')
      return
    }
    $.skipperAnimate = $.velocity
  }
}

The basic idea is to use $.skipperAnimate() instead of $.animate() and $.velocity(). If you happened to call $.skipperAnimate() before velocity had loaded, you'd just get jquery's $.animate, but once velocity is fully loaded, a call to $.skipperAnimate will refer to $.velocity

Just looking at your code, I think there's only a single call to $.velocity for the slide animation? That's the only call we'd have to switch to $.skipperAnimate right ?