0

EDIT 2 (to make the problem more understandable)

The effect I am trying to achieve is the following: everytime an element enters the viewport an 'is-visible' class is added to it and the same 'is-visible' class is removed from the previous element.

Now I've managed to make it work but I run a for loop to remove all is-visible classes before adding the is-visible class to the element in viewport.

It works but in terms of performance I think it would be better to just remove the class from element[i -1]. And this were I can't get it working.

Here is a simplified fiddle were I try to make the element[i-1] solution work: https://jsfiddle.net/epigeyre/vm36fpuo/11/.

EDIT 1 (to answer some of the questions asked)

I have corrected an issue raised by @Catalin Iancu (thanks a lot for your precious help) by using a modulus operator ((i+len-1)%len).

ORIGINAL QUESTION (not really clear)

I am trying to get the previous element in a for loop (to change its class) with following code :

for (var i = 0; i < array.length; i++) {
    if(array[i-1] && my other conditions) {
        array[i-1].classList.remove('is-visible');
        array[i].classList.add('is-visible');
    }
}

But it's not removing the class for [i-1] element.

Here is a more complete piece of code of my module (this is running within a scroll eventlistener):

var services = document.getElementsByClassName('services'),
    contRect = servicesContainer.getBoundingClientRect();

for (var i = 0; i < services.length; i++) {
    var serviceRect = services[i].getBoundingClientRect();

    if ( !services[i].classList.contains('active') && Math.round(serviceRect.left) < contRect.right && services[i-1]) {
        services[i-1].classList.remove('is-visible');
        services[i].classList.add('is-visible');
    }
}

Thanks for your help!

Improve this question
8
  • 2
    Um, no, it only gets the previous one. But since it is in a for loop, it will get the previous one for each iteration. Commented Sep 27, 2017 at 15:01
  • This sentence doesnt make sense But it seems that [i-1] is getting all previous elements instead of just the previous one. - that really does only read the previous instance from an array. PLease provide a minimal reproducible example to demonstrate your exact problem. Commented Sep 27, 2017 at 15:01
  • In the first iteration i is 0... sooo you get all elements probably because you refer to the 0-1 = -1 array element... or you need to get an error for this. Commented Sep 27, 2017 at 15:03
  • how did you think that " [i-1] is getting all previous elements instead of just the previous one"? what happens ? Commented Sep 27, 2017 at 15:06
  • 1
    Are you sure its not one of the other conditions in your if clause stopping the class being removed when you think it should be? Try stepping through your code. Commented Sep 27, 2017 at 15:08

3 Answers 3

Reset to default
1

Your if(array[i-1] && my other conditions) is always true, except for the very first case where array[-1] doesn't exist. Therefore, it will remove and then add the active class for each element, which will make it seem as only the first element's class has been removed.

What you need is a better if condition or a break statement, when the loop is not needed anymore

for (var i = 0; i < array.length; i++) {
    if(array[i] && i != array.length - 1) {
        array[i].classList.remove('active');
    }
}
array[array.length - 1].classList.add('active');
Improve this answer
Sign up to request clarification or add additional context in comments.

8 Comments

So it means that if(array[i-1]) is not enought to prevent this from happening?
Yes, since if (variable) only verifies if that variable is different from undefined or other equivalent values, like 0, "", etc. Since your elements exist, it will always verify as true.
No, since your array elements are DOM objects, whose class is services. They do not evaluate to simple values like 0 or -1. I have edited my answer to contain a more probable solution to exactly what you want.
Sorry my previous comment was really stupid (my brain needs some time off). I have found another solution using modulos to prevent thos situation from happening. However this doesn't solve my issue. I've posted a jsfiddle in the question if it helps.
I see, it's related to the scroll. So, you want only the current top element from the screen visible, or something like that?
|
0

The problem probably is that based on your code: services[i-1].classList.remove('active'); and services[i].classList.add('active'); the 'active' class you add in current iteration will be removed in next iteration!

So your code has logical errors, array index does not return all prev items!

Improve this answer

1 Comment

Yes but that's what I want, everytime the conditions are met, the previous active class has to be removed. Am I not thinking this right?
0

What if you create a variable that contain the previous element?

var previous = array[0];
for (var i = 0; i < array.length; i++) {
    if(previous && my other conditions) {
        previous.classList.remove('active');
        array[i].classList.add('active');
    break;
    }
    previous = array[i];
}
Improve this answer

1 Comment

I've tried that too... I've posted a jsfiddle in the question if you feel like having a look :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.