Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prev method only works for first item #19

Closed
rshimshock opened this issue Apr 9, 2016 · 3 comments
Closed

prev method only works for first item #19

rshimshock opened this issue Apr 9, 2016 · 3 comments

Comments

@rshimshock
Copy link

The other issue I found is if I use the next method, the ticker operates as expected. However, if I use the prev method, the first item moves "down", but every subsequent item moves "up".

After reviewing the code, I believe I found the issue. The moveDown function passes a dir value of 'down' into animate the first time the code is run. However each time after the first, the nextUsePause function is run.

The problem is nextUsePause only calls method.next.call, and nowhere is method.prev.call called:

    nextUsePause: function() {
      var state = $(this).data('state');
      var options = state.options;
      if(state.isPaused || state.itemCount < 2) return;
      methods.next.call( this, {animate:options.animate} );  // problem code
    },

There either needs to be the addition of a separate prevUsePause function, or a conditional in nextUsePause. I was able to make this work using the conditional approach. Here are the changes I made.

1) Store "up" and "down" as state variables when in the "next" and "prev" methods:

    next: function(attribs) {
      var state = $(this).data('state');
      if(!state) return undefined;
      if(state.animating || state.itemCount < 2) return false;
      internal.restartInterval.call( this );
      state.direction = 'up';  // add this line here
      internal.moveUp(state, attribs);
    },

    prev: function(attribs) {
      var state = $(this).data('state');
      if(!state) return undefined;
      if(state.animating || state.itemCount < 2) return false;
      internal.restartInterval.call( this );
      state.direction = 'down';  // add this line here
      internal.moveDown(state, attribs);
    },

2) Remove "dir" variables passed in to moveUp and moveDown:

    moveUp: function(state, attribs) {
      internal.animate(state, attribs);  // 'up' has been removed, no longer needed
    },

    moveDown: function(state, attribs){
      internal.animate(state, attribs);  // 'down' has been removed, no longer needed
    },

3) Remove "dir" variable passed to animate, and add as a variable from state:

    animate: function(state, attribs) {  // dir has been removed
      var height = state.itemHeight;
      var options = state.options;
      var el = state.element;
      var dir = state.direction;  // added dir variable

4) Add conditional logic to nextUsePause function:

    nextUsePause: function() {
      var state = $(this).data('state');
      var options = state.options;
      if(state.isPaused || state.itemCount < 2) return;
      if(state.direction == 'up') {  // new conditional added here
        methods.next.call( this, {animate:options.animate} );
      } else {
        methods.prev.call( this, {animate:options.animate} );
      }
    },

I'd appreciate any feedback you might have. The previous option started working as expected once these updates were made.

@richhollis
Copy link
Owner

Thanks for this also and also for the great detail provided. Will look to address this one too this week. I will update shortly. Thanks!

@richhollis
Copy link
Owner

Hi @rshimshock

Further to my comments in #18

There is an HTML test page in this repository which creates some test tickers and has controls for animation along with prev/next link buttons. When I test the prev/next everything looks good and works as expected (as designed).

This control doesn't support different directional scrolling but it should let you go prev/back as expected. If you are seeing different results to that when using the control as designed then please re-open this issue. Thanks!

Test page:
https://github.com/richhollis/vticker/blob/master/test/test_ticker.html

@rshimshock
Copy link
Author

This control doesn't support different directional scrolling but it should let you go prev/back as expected.

Our issue is the original vTicker code does support this.

It's not a difficult thing to add. If it's not something you want to support we will probably need to go back to using the original vTicker code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants