How do I stop scroll events from recurring?

Question

Can anyone help me with the events on my site? The issue is, they keep re-occurring each time the scroll event is fired. The navigation menu is under the header but stickies to top on scroll but I want it to go back under the header when we scroll all the way back up. I don't know what I'm doing wrong. Here's my code:

var navigation = document.querySelector("nav");
var borderBottom = navigation.getAttribute("border-bottom");
var currentDistance = (document.documentElement.scrollHeight);

addEventListener ("scroll", function() {

if (currentDistance > "90px") {
navigation.style.position = "fixed";
navigation.style.top = "0";
navigation.style.borderBottom = "3px solid rgb(157,0,53)";
}

else removeEventListener ("scroll", function() {
navigation.style.position = "fixed";
navigation.style.top = "0";
navigation.style.borderBottom = "3px solid rgb(157,0,53)";
}
);
});

Also when the page loads and the scroll event is fired, a div flies in, which is what I want. but my problem is that each time that scroll event is fired, the box keeps flying in instead of the event stopping i.e. keeps RE-OCCURRING. Here's the code I used for it:

var flyInDiv = document.getElementById("flyInDiv");

addEventListener ("scroll", function() {
var programMenu = document.createElement("DIV");

programMenu.className = "programMenu";
programMenu.appendChild(flyInDiv);

document.body.appendChild(programMenu);

programMenu.style.position = "fixed";

});

The URL to my site is agwconitsha.org

Answer 1

You can try using the overflow-x: hidden [or] overflow-y: hidden depending on where your scroll bar is.

Answer 2

I've looked at your code & your site and it looks like you're trying to achieve the following:

• A sticky nav that only becomes sticky when a user scrolls past the header
• A list of service times that 'flys' in once when a user first scrolls, and stays there

TL;DR - I've made a jsfiddle containing this basic behaviour at http://jsfiddle.net/9o7frxp1/4/.

In the above fiddle, I've suggested using CSS classes to control the stickiness of the nav, and the animation of flying div. JavaScript is simply used to turn these classes on and off based on whether the user is scrolling, and what their scroll position is.

This is great for separating the concerns of JS & CSS, allows each language each do what it does best, and means the code is easier to maintain as a result.

To explain things further, let's just take the example of your <nav> for now so this post doesn't get too long. Check out my jsfiddle link above for the flying div as well - I've added comments to the code to explain what's happening, but let me know if it doesn't make sense.

So, we have a nav element:

<nav id="nav">
  Nav
</nav>

We have some styles for the <nav> - some default styles, and then some that will only be applied when it also has a .is-sticky class:

#nav {
  width: 100%;
  height: 40px;
}

#nav.is-sticky {
  position: fixed;
  top: 0;
}

In our JavaScript, we find the <nav>:

var navigation = document.getElementById('nav');

Now, we prepare a function that will fire when the user scrolls. We haven't yet added anything that will listen for scrolling yet - we're just getting the function ready.

function addStickyClass() {
  // Every time this function fires, we calculate the scroll position
  var currentDistance = document.body.scrollTop;

  // If we've scrolled 90px down, add the 'is-sticky' class to our nav.
  // If not, remove the 'is-sticky' class.
  if (currentDistance > 90) {
    navigation.className = 'is-sticky';
  } else {
    navigation.className = '';
  }
}

Cool - we're almost ready to go. Last thing - let's make sure the above function fires each time the user scrolls.

window.addEventListener('scroll', addStickyClass);

This means we're asking every time the user scrolls "Are they 90px (or more) away from the top of the window?" - if they are, the nav should be sticky, and if not, it shouldn't.

stekhn is correct that you haven't quite managed to implement event listeners correctly, and it's causing your page to behave weirdly.

However, in my code example above, I'm suggesting only need to add one event listener and that there's no need to remove it. I've also gone a bit further in suggesting you should imagine this problem in a slightly different way using an alternative combination of JS & CSS, as I mentioned above.

In my code in this answer, I've simplified things a bit to explain more clearly what's happening - you'll see a more elaborate version in the jsfiddle I've posted.

However, even my jsfiddle is still a simplified version of what you have on your site, so you'll need to adapt it - have a go and let me know how you get on.

---

Update

TL;DR:

I actually realised the code can be a bit simpler for the #flyInDiv, and doesn't need any booleans or if statements. Fiddle with a comment explaining at http://jsfiddle.net/9o7frxp1/5/

---

But I still wanted to explain what I was implementing originally below - I reckon it's worth knowing:

So, if you remember, we have a function that's being fired every time a user scrolls:

function addStickyClass() {
  // Every time this function fires, we calculate the scroll position
  var currentDistance = document.body.scrollTop;

  // If we've scrolled 90px down, add the 'is-sticky' class to our nav.
  // If not, remove the 'is-sticky' class.
  if (currentDistance > 90) {
    navigation.className = 'is-sticky';
  } else {
    navigation.className = '';
  }
}

Every time the user scrolls, this function decides whether to add or remove the .is-sticky class to our element.

Speaking of which - there's something else we want our code to decide based on when the user is scrolling - the revealing of your #flyInDiv. Maybe we can include it in this code block?

The logic for the #flyInDiv is this:

• The element is invisible by default when the user first lands on the page, thanks to CSS
• On the user's first scroll, the element should appear, due to an additional CSS class being added
• The element should stay where it is for the remainder of the user's visit to your page. We don't care anymore if the user is scrolling.

Let's look at how that's working in code terms. Firstly, the element is invisible by default thanks to CSS. We have the .is-sticky class for making the element visible, but by default that class isn't on the element:

#flyInDiv {
  width: 100px;
  height: 100px;

  /* Positioned hard right */
  position: fixed;
  right: 0;

  /* Translated an extra 100px to the right, sending it out of view */
  transform: translateX(100px);
}

#flyInDiv.is-sticky {
  /* No translation, so the element should now simply be hard right */
  transform: translateX(0);
}

The stage is set... we need some JavaScript!

Let's go back to our addStickyClass() function - but I'll remove the other code relating to the <nav> from these examples to make things clearer.

var flyingDiv = document.getElementById("flyInDiv");

function addStickyClass() {
  flyingDiv.className = "is-sticky";
}

Remember that this gets fired everytime the user scrolls (thanks to that event listener we added a while ago), so what will happen?

The first time the user scrolls, the element will get the .is-sticky class. And it'll keep getting set every time the user scrolls.

As it happens, this doesn't affect the behaviour of your site, which I mention in my latest fiddle. It's actually fine if this class keeps getting set on the #flyInDiv, because it just means that from the moment the user first scrolls, it just keeps on being visible.

However, strictly speaking, this code only needs to run once. We only want to set the .is-sticky class after the first scroll event. We don't need to keep setting it every time the user scrolls.

For this example, we get away with it because when the code flyingDiv.className = "is-sticky"; gets executed on every scroll, it simply continually overwrites the class name. Result: your flyingDiv always has the .is-sticky class.

However, there are bound to be plenty of times where you do want to enforce something that definitely only happens after the first scroll - eg, let's say for this project you'd needed to use appendChild() to add a new, more complex element into the DOM after the first scroll, rather than simply being able to use CSS to make it visible - we definitely couldn't get away with that code continually repeating itself - it would mean our element kept getting appended!

And this is how you'd do it - let's give our code the knowledge about whether the element is visible or not. That way, it can decide whether it needs to run the code or not.

On scroll, it can continually ask:

• Is the element invisible?
  • No: This must be the first scroll. Make it visible!
  • Yes: This isn't the first scroll. It's already visible. Cool, do nothing.

So firstly we're going to need a variable to evaluate. Let's name it, and set it to false by default. Why? Because the variable is flyingDivIsVisible (read "Is the flying div visible?"), and before anything else has happened on the page, as we already mentioned, the answer should be "no".

Importantly, this happens outside & above any functions. We're setting the stage here, we don't want this code to keep running every time the user scrolls. It's just here to give the code the initial knowledge we already have - that when the user visits the page, but before they scroll, the #flyInDiv is invisible.

var flyingDivIsVisible = false;

Next, we add some stuff to our function. Crucially, this new stuff we add is in an if block and only runs when !flyingDivIsVisible is true (or put another way, flyingDivIsVisible === false):

// Tell our code that the div isn't visible.
var flyingDivIsVisible = false;

function addStickyClass() {

  // Only run this code if div isn't visible.
  if (!flyingDivIsVisible) {
    flyingDiv.className = "is-sticky";

    // Tell our code that the div is visible
    flyingDivIsVisible = true;
  }
}

Remember, we set var flyingDivIsVisible = false; above & outside our function? So at least the first time this function runs, flyingDivIsVisible === false will definitely evaluate to true.

BUT, the stuff which only runs when flyingDivIsVisible === false? Well, it happens to also change flyingDivIsVisible = true. Why? Because the stuff that runs is adding the .is-sticky class to #flyInDiv, which means it's visible now - but we need to give our code that knowledge by setting flyingDivIsVisible = true.

So, from now on, every additional time the user scrolls, we'll now have flyingDivIsVisible = true, which means any code in an if (!flyingDivIsVisible) block will no longer run.

Therefore, we've just implemented a bit of code which runs the first time a user scrolls, but never again after that.

Just to make sure this is as clear as possible, let's look at the function running twice in a row and break down what happens. Let's also imagine that we keep asking the code if the div is visible or not. Since we've made a way to give it that knowledge, it can keep telling us. PS, I've used IIFEs in this example - they're functions that run themselves, rather than waiting to be called separately. It makes more sense for this example where we run a block of code synchronously and twice in a row:

// Tell our code the div isn't visible
var flyingDivIsVisible = false;

(function () {
  // Us: Is the div visible?
  // Code: No. You told it me it isn't.

  if (!flyingDivIsVisible) {
    flyingDiv.className = "is-sticky";

    // Tell our code that the div is visible
    flyingDivIsVisible = true;
  }
})();

(function () {
  // Us: Is the div visible?
  // Code: Yes. You told me it was last time I ran this function.

  if (!flyingDivIsVisible) {
    // Nothing in this block runs, because the div is visible.

    flyingDiv.className = "is-sticky";
    flyingDivIsVisible = true;
  }
})();

By the way, as an afterthought, if the #flyInDiv was the only thing this code was controlling, we might as well just remove the event listener after the first scroll as well, so that the code doesn't listen for scrolling anymore (it doesn't need to, right? It's done its job in making the div visible on first scroll).

But as it happens, we were imagining this in the context of the same code as the <nav> showing & hiding, which still needs the event listener to keep asking "Is the user scrolling?" all the time, so we don't want to remove the event listener.

Hope that all makes sense!

Answer 3

The problem is that you are not adding and removing the event listener correctly. Every event listener needs a target, a action and handler.

window.addEventListener("scroll", myfunction);
window.removeEventListener("scroll", myfunction);

I would suggest you hook the scroll event listener to the window object (whole page) and then you do your magic.

You could also use a variable to control the state of you flyIn element with a variable isOpen

var navigation = document.querySelector("nav");
var borderBottom = navigation.getAttribute("border-bottom");
var currentDistance = (document.documentElement.scrollHeight);
var isOpen = false;

window.addEventListener("scroll", navigationHandler);

function navigationHandler() {

    if (currentDistance > "90px") {

        navigation.style.position = "fixed";
        navigation.style.top = "0";
        navigation.style.borderBottom = "3px solid rgb(157,0,53)";
    } else {

        navigation.style.position = "relative";
        navigation.style.borderBottom = "none";
    }
}


var flyInDiv = document.getElementById("flyInDiv");

window.addEventListener("scroll", flyInHandler);

function flyInHandler() {

    if (!isOpen) {
        var programMenu = document.createElement("DIV");
        programMenu.className = "programMenu";
        programMenu.appendChild(flyInDiv);
        document.body.appendChild(programMenu);
        programMenu.style.position = "fixed";

        isOpen = true;
        window.removeEventListener("scroll", flyInHandler);
    }
}