0

I have several divs, all with the same class (post), and I have a simple navigation with UP and DOWN. 

<nav>
<a href="" id="down">Down</a>
<a href="" id="up">Up</a>
</nav>

<div class="post one">Lorem ipsum dolor sit amet.</div>
<div class="post two">Lorem ipsum dolor sit amet.</div>
<div class="post three">Lorem ipsum dolor sit amet.</div>

Everytime the user clicks in Down I want to scroll down one div at the time. Everytime the users click Up I want to go up one div at the time.

I almost got it, but there are some errors if you click down twice and then you want to go up twice again, then the scroller gets stuck in the middle. Also if you are at the very bottom and you click Down one more time, it starts from the top. Also if you click one up and one down, the scrolling it's not following the logic that it should.

Please see it here: http://jsfiddle.net/grovve/bs6443y4/

I assume it's something connected with my variable "$currentElement" and what is exactly the value of this variable at a given time, am I right? What exactly am I missing to make it work as it should?

Terry
  • 48,492
  • 9
  • 72
  • 91
user2089160
  • 91
  • 3
  • 9

3 Answers3

3

The reason why you are getting an error is simple — if you are at the bottom of the list (i.e. the last element), $currentElement.next() will return nothing. The similar thing happens when you are at the top. Therefore, you should use if to check of the next or previous element exists upon the click handler being triggered. We can use .length to evaluate this condition.

Meanwhile, you might want into looking at using .stop() before actually adding another animation to the queue. If you do not use it, you will end up adding too many items to the jQuery animation queue upon each mouseclick, which we do not want — it might lead to jerky animations or animations that take very long to finish. Therefore, we call .stop(true) in front of each animation to clear the entire queue.

Finally, when we are traversing the DOM using .prev() and .next(), we are accepting all elements — so we should include the selector, i.e. .prev('.post') and .next('.post') respectively to achieve the desired behavior.

var $currentElement = $(".post").first();

$("#down").click(function () {
    var $nextElement = $currentElement.next('.post');
    // Check if next element actually exists
    if($nextElement.length) {
        // If yes, update:
        // 1. $currentElement
        // 2. Scroll position
        $currentElement = $nextElement;
        $('html, body').stop(true).animate({
            scrollTop: $nextElement.offset().top
        }, 1000);
    }
    return false;
});

$("#up").click(function () {
    var $prevElement = $currentElement.prev('.post');
    // Check if previous element actually exists
    if($prevElement.length) {
        // If yes, update:
        // 1. $currentElement
        // 2. Scroll position
        $currentElement = $prevElement;
        $('html, body').stop(true).animate({
            scrollTop: $prevElement.offset().top
        }, 1000);
    }
    return false;  
});

See updated and working fiddle here: http://jsfiddle.net/teddyrised/bs6443y4/7/

Terry
  • 48,492
  • 9
  • 72
  • 91
  • Nice one with checking for if there's a next element and clearing the animation queue. – wawa Jan 20 '15 at 18:00
  • Thank you, your comments make sense. the problem that I see its that not it doesnt go to the third div, it just remains between the 1 and 2. Do you know why is that? – user2089160 Jan 20 '15 at 18:00
  • @user2089160 I can't seem to replicate your issue. I'm using Chrome 39, navigating to last div (green one) as per usual... – Terry Jan 20 '15 at 18:02
  • when you click up on the first div, it goes to the second div – indubitablee Jan 20 '15 at 18:03
  • You are right. I refreshed my browser again and it works. Many thanks for the help, very much appreciated! – user2089160 Jan 20 '15 at 18:07
  • @user2089160 I realized the problem is that when we traverse the DOM using `.next()` and `.prev()` we are accepting *any* element. In this case we want to specify that we are only looking for elements with the `post` class, so we should use `.next('.post')` and `.prev('.post')` respectively. Updated answer and fiddle :) – Terry Jan 20 '15 at 18:08
  • that makes a lot of sense. I didnt know you could specify the class within .next and .prev. Also, adding that extra variable makes sense as well. thanks! – user2089160 Jan 20 '15 at 18:12
  • How come mine isn't scrolling right and left like the example provided? https://jsfiddle.net/k66gLuLk/3/ I am sorting the div at page load. Thanks – Si8 Jun 10 '16 at 18:32
0

Take a look at the comment in the function for up. You have used a var there, therefor you're in the wrong scope.

var $currentElement = $(".post").first();

$("#down").click(function () {
    $currentElement = $currentElement.next();
    $('html, body').animate({
        scrollTop: $currentElement.offset().top
    }, 1000);
    return false;
});

$("#up").click(function () {
    /* here's your problem! the var tells js to only use currentElement in this function not globally, the variable on the first line isn't overwritten!
    var currentElement = $currentElement.prev();
    to fix it simply remove the var! */
    $currentElement = $currentElement.prev();
    $('html, body').animate({
        scrollTop: $currentElement.offset().top
    }, 1000);
    return false;  
});
wawa
  • 3,497
  • 2
  • 22
  • 43
  • Still wrong, you need the ``$`` symbol in front. Also see above for extra logic to foolproof the scrolling, otherwise an error is thrown. – Patrick Roberts Jan 20 '15 at 18:01
  • @PatrickRoberts true. I missed that. I've adjusted it now, but I'd recommend using the method Terri proposed, since it checks if there's a next element at all. – wawa Jan 20 '15 at 18:05
  • How come mine isn't scrolling right and left like the example provided? https://jsfiddle.net/k66gLuLk/3/ I am sorting the div at page load. Thanks – Si8 Jun 10 '16 at 18:32
0

If you want up/down to wrap the posts from bottom to top, try this code forked from Terry's answer:

var $currentElement = $(".post").first();

    $("#down").click(function () {
        var currentElement = $currentElement.next(".post");
        // Check if next element actually exists
        if(currentElement.length) {
            // If yes, update:
            // 1. $currentElement
            // 2. Scroll position
            $currentElement = currentElement;
        } else {
            $currentElement = currentElement = $(".post").first();
        }

        $('html, body').stop(true).animate({
            scrollTop: $(currentElement).offset().top
        }, 1000);
        return false;
    });

    $("#up").click(function () {
        var currentElement = $currentElement.prev(".post");
        // Check if previous element actually exists
        if(currentElement.length) {
            // If yes, update:
            // 1. $currentElement
            // 2. Scroll position
            $currentElement = currentElement;
        } else {
            $currentElement = currentElement = $(".post").last();
        }

        $('html, body').stop(true).animate({
            scrollTop: $(currentElement).offset().top
        }, 1000);
        return false;
    });

See demo here: http://jsfiddle.net/bs6443y4/8/

Patrick Roberts
  • 40,065
  • 5
  • 74
  • 116
  • How come mine isn't scrolling right and left like the example provided? https://jsfiddle.net/k66gLuLk/3/ I am sorting the div at page load. Thanks – Si8 Jun 10 '16 at 18:32