Welcome!

By registering with us, you'll be able to discuss, share and private message with other members of our community.

SignUp Now!
  • Guest, before posting your code please take these rules into consideration:
    • It is required to use our BBCode feature to display your code. While within the editor click < / > or >_ and place your code within the BB Code prompt. This helps others with finding a solution by making it easier to read and easier to copy.
    • You can also use markdown to share your code. When using markdown your code will be automatically converted to BBCode. For help with markdown check out the markdown guide.
    • Don't share a wall of code. All we want is the problem area, the code related to your issue.


    To learn more about how to use our BBCode feature, please click here.

    Thank you, Code Forum.

JavaScript Problem with event listener scroll

Sapoios

Sapoios

Coder
Hello guys i would like a help with a problem that i cant fix.

I have a horizontal bar with categories. Its category is a link to a page section.

This bar is made for mobile so its full width is beyond screen.

Now i have an event listener that detecta scroll distance of visitor and when finds the section of a link adds a class to make it "active" and bring the corresponding bar element to center of view.

This is achieved with a foreach loop inside listener that checks all the link of categories and if one is between a range of pixels add class active and scrollintoview. Else just remove class active.

The categories are counted before listener because it might be different each time so currently i have 21.

This means that gor every pixel scroll i check 21 links and have 21 results 1 that adds a class and 20 that removes it.

The problem i have is with scrollintoview in which i use options behavior : smooth and inline : center. That normal animates a scroll effect that brings the element into center. Now because i use smooth the animation only happens afrer the scroll ends because scrollintoview view is called far to many times wilhile scrolling. If a left it on auto it works but it jumps right into it with out animation effects.

Is the any way to scroll into view only for the element with the class active or if this element already has active run only once for this element?
 
Sort by date Sort by votes
There no error but

inside scroll listener the if is called for every category and every pixel

that means 21 times for every pixel so even the add class is called (x) times = pixel scrolled and the result is lets say 200 times in 0.01s.
adding the class is instant but the scrollintoview takes 500ms resulting a wait time while scrolling.due to many times called it will be executed after the last time called.

what i need is the "true" of the "if" inside the for each loop called only one time for each category and if already has the active class dont call it.

i already tryed adding a second "if" that checks if the element has the class and the result was the same.

JavaScript: 
$( document ).ajaxComplete(function() {
                document.querySelectorAll('a[href^="#"]').forEach(anchor => {
                    anchor.addEventListener('click', function (e) {
                        e.preventDefault();
                        var scrlt = document.querySelector(this.getAttribute('href')).offsetTop -200;
                        $(document.querySelector("#restaurant_page > .page__content")).scrollTop(scrlt);
                       
                    });
                });
                var mainNavLinks = document.querySelectorAll("ons-list-item > div > a");
                var fullscroll = 0;
                var scrollarray = [];
                mainNavLinks.forEach((link,i) => {
                    var section = document.querySelector(link.hash);
                    var sectionheight = section.offsetHeight;
                    fullscroll += sectionheight;
                    scrollarray[i] = {pagescroll:fullscroll, catname:link , aanum:i};
                });
                var outer = document.querySelector("#restaurant_page > .page__content");
                outer.addEventListener('scroll', function(){
                        var current = document.querySelector("#restaurant_page > .page__content");
                        var page = $(current).scrollTop();
                        scrollarray.forEach(catattri => {
                            var section = document.querySelector(catattri.catname.hash);
                            var carousel = document.querySelector("#carousel_resto_menu_inner");
                            if ((section.offsetTop  < page +250 ) && ((section.offsetTop + section.offsetHeight ) >= page +250)
                            ) {
                               
                                catattri.catname.classList.add("active");
                                console.log("pign");
                                var active = document.querySelector("ons-list-item > div > a.active");
                                active.scrollIntoView({behavior: "smooth",inline:"center"});
                                   
                            } else {
                                catattri.catname.classList.remove("active");
                               
                            }
                        });

                        var current = document.querySelector("#restaurant_page > .page__content");
                        var target = $(current).scrollTop();
                        var carousel = document.querySelector("#carousel_resto_menu");
                        if ((target > 100)){
                            carousel.classList.add("sticky");
                            carousel.classList.remove("close");
                        }else {
                            carousel.classList.add("close");
                        }

                });
               
            });
 
Upvote 0 Downvote
Hi,

I don't think I understand the problem fully and I haven't gone through the full code either. But looks like the issue is with the scroll event being fired too frequently.

If that's the case, you might want to learn about denounce technique in JavaScript. A quick Google search will give you an idea. Also, consider using for loop instead of forEach for performance. You can break out of for loop once your condition is met while forEach will iterate through each element no matter what.

Hope it helps :)
 
Upvote 0 Downvote
joe said:
Hi,

I don't think I understand the problem fully and I haven't gone through the full code either. But looks like the issue is with the scroll event being fired too frequently.

If that's the case, you might want to learn about denounce technique in JavaScript. A quick Google search will give you an idea. Also, consider using for loop instead of forEach for performance. You can break out of for loop once your condition is met while forEach will iterate through each element no matter what.

Hope it helps :)
Click to expand...


Great that is what i want for loop is it possible to help me a bit in remaking the loop?

JavaScript: 
scrollarray.forEach(catattri => {
                            var section = document.querySelector(catattri.catname.hash);
                            var carousel = document.querySelector("#carousel_resto_menu_inner");
                            if ((section.offsetTop  < page +250 ) && ((section.offsetTop + section.offsetHeight ) >= page +250)
                            ) {
                              
                                catattri.catname.classList.add("active");
                                console.log("pign");
                                var active = document.querySelector("ons-list-item > div > a.active");
                                active.scrollIntoView({behavior: "smooth",inline:"center"});
                                  
                            } else {
                                catattri.catname.classList.remove("active");
                              
                            }
                        });
 
Last edited:
Upvote 0 Downvote
Sapoios said:
Great that is what i want for loop is it possible to help me a bit in remaking the loop?

JavaScript: 
scrollarray.forEach(catattri => {
                            var section = document.querySelector(catattri.catname.hash);
                            var carousel = document.querySelector("#carousel_resto_menu_inner");
                            if ((section.offsetTop  < page +250 ) && ((section.offsetTop + section.offsetHeight ) >= page +250)
                            ) {
                             
                                catattri.catname.classList.add("active");
                                console.log("pign");
                                var active = document.querySelector("ons-list-item > div > a.active");
                                active.scrollIntoView({behavior: "smooth",inline:"center"});
                                 
                            } else {
                                catattri.catname.classList.remove("active");
                             
                            }
                        });
Click to expand...

Not sure what exactly this code does, but for loop will only help if you don't need to iterate through all the items every time.

Just use the break keyword to break out of the loop when needed.
 
Upvote 0 Downvote
joe said:
Not sure what exactly this code does, but for loop will only help if you don't need to iterate through all the items every time.

Just use the break keyword to break out of the loop when needed.
Click to expand...
scrollarray is an array with
1) the name of the hash of the anchor
2) the scroll point wich each section starts
3) an index number

So what this for each does is

Check if the current scrolled position is between each of the sections.
In the section that is between adds a class to the anchor and brings it center of the page horizontally.
For the rest removes the class.

What it would be best for me is that while the scroll position is in between a section do add class only once and center only once. If the section change the remove the class from the previous section and do the add class and center to the new but only once
 
Upvote 0 Downvote
Unfortunately i couldn't set it up in code pen dont know why what ever i try didnt work there. But i post this video to demonstrate the issue. Focus on the scroll delay of the top bar.
To view this content we will need your consent to set third party cookies.
For more detailed information, see our cookies page.
 
Upvote 0 Downvote
Ok, that helps to understand the problem better but still not perfectly.

I went though your full code again and modified it a little bit:

JavaScript: 
function debounce(func, wait, immediate) {
    var timeout;
    return function() {
        var context = this, args = arguments;
        var later = function() {
            timeout = null;
            if (!immediate) func.apply(context, args);
        };
        var callNow = immediate && !timeout;
        clearTimeout(timeout);
        timeout = setTimeout(later, wait);
        if (callNow) func.apply(context, args);
    };
};


$(document).ajaxComplete(function () {
    document.querySelectorAll('a[href^="#"]').forEach(anchor => {
        anchor.addEventListener('click', function (e) {
            e.preventDefault();
            var scrlt = document.querySelector(this.getAttribute('href')).offsetTop - 200;
            $(document.querySelector("#restaurant_page > .page__content")).scrollTop(scrlt);
        });
    });
    var mainNavLinks = document.querySelectorAll("ons-list-item > div > a");
    var fullscroll = 0;
    var scrollarray = [];
    mainNavLinks.forEach((link, i) => {
        var section = document.querySelector(link.hash);
        var sectionheight = section.offsetHeight;
        fullscroll += sectionheight;
        scrollarray[i] = { pagescroll: fullscroll, catname: link, aanum: i };
    });
    var outer = document.querySelector("#restaurant_page > .page__content");
    outer.addEventListener('scroll', debounce(function () {
        var current = document.querySelector("#restaurant_page > .page__content");
        var target = $(current).scrollTop();
        scrollarray.forEach(catattri => {
            var section = document.querySelector(catattri.catname.hash);
            if ((section.offsetTop < page + 250) && ((section.offsetTop + section.offsetHeight) >= page + 250)) {
                catattri.catname.classList.add("active");
                console.log("pign");
                var active = document.querySelector("ons-list-item > div > a.active");
                active.scrollIntoView({ behavior: "smooth", inline: "center" });
            } else {
                catattri.catname.classList.remove("active");
            }
        });
        var carousel = document.querySelector("#carousel_resto_menu");
        if ((target > 100)) {
            carousel.classList.add("sticky");
            carousel.classList.remove("close");
        } else {
            carousel.classList.add("close");
        }
    }, 500));
});

I have added a debounce function which will prevent scroll callback function from firing every millisecond. Instead it will fire at interval of 0.5 second only while page is being scrolled. Also, I removed a few unused or unwanted variables and cleaned up the scroll callback function. Please check the new code, try implementing and see if gives any better result.
 
Upvote 0 Downvote
joe said:
Ok, that helps to understand the problem better but still not perfectly.

I went though your full code again and modified it a little bit:

JavaScript: 
function debounce(func, wait, immediate) {
    var timeout;
    return function() {
        var context = this, args = arguments;
        var later = function() {
            timeout = null;
            if (!immediate) func.apply(context, args);
        };
        var callNow = immediate && !timeout;
        clearTimeout(timeout);
        timeout = setTimeout(later, wait);
        if (callNow) func.apply(context, args);
    };
};


$(document).ajaxComplete(function () {
    document.querySelectorAll('a[href^="#"]').forEach(anchor => {
        anchor.addEventListener('click', function (e) {
            e.preventDefault();
            var scrlt = document.querySelector(this.getAttribute('href')).offsetTop - 200;
            $(document.querySelector("#restaurant_page > .page__content")).scrollTop(scrlt);
        });
    });
    var mainNavLinks = document.querySelectorAll("ons-list-item > div > a");
    var fullscroll = 0;
    var scrollarray = [];
    mainNavLinks.forEach((link, i) => {
        var section = document.querySelector(link.hash);
        var sectionheight = section.offsetHeight;
        fullscroll += sectionheight;
        scrollarray[i] = { pagescroll: fullscroll, catname: link, aanum: i };
    });
    var outer = document.querySelector("#restaurant_page > .page__content");
    outer.addEventListener('scroll', debounce(function () {
        var current = document.querySelector("#restaurant_page > .page__content");
        var target = $(current).scrollTop();
        scrollarray.forEach(catattri => {
            var section = document.querySelector(catattri.catname.hash);
            if ((section.offsetTop < page + 250) && ((section.offsetTop + section.offsetHeight) >= page + 250)) {
                catattri.catname.classList.add("active");
                console.log("pign");
                var active = document.querySelector("ons-list-item > div > a.active");
                active.scrollIntoView({ behavior: "smooth", inline: "center" });
            } else {
                catattri.catname.classList.remove("active");
            }
        });
        var carousel = document.querySelector("#carousel_resto_menu");
        if ((target > 100)) {
            carousel.classList.add("sticky");
            carousel.classList.remove("close");
        } else {
            carousel.classList.add("close");
        }
    }, 500));
});

I have added a debounce function which will prevent scroll callback function from firing every millisecond. Instead it will fire at interval of 0.5 second only while page is being scrolled. Also, I removed a few unused or unwanted variables and cleaned up the scroll callback function. Please check the new code, try implementing and see if gives any better result.
Click to expand...
I have tryed before debounce and throttle functions but with debounce i have the same results with fewer calls and with throttle the scroll happens with stages as if it gas low fps. The throttle was a good solution but if the scroll was fast it would jump categories if i set time more than 750. Let me so you what i am trying to achive
To view this content we will need your consent to set third party cookies.
For more detailed information, see our cookies page.

If you can guide me a code for that i can change it to meet my project
 
Upvote 0 Downvote
@Sapoios
Can you try detecting the scroll distance?
Like, onScroll callback should only run code if scroll distance is more than 100px or so?
This will prevent entire code from running for scroll of every single pixel. I guess this will improve the way it works.
 
Upvote 0 Downvote
joe said:
@Sapoios
Can you try detecting the scroll distance?
Like, onScroll callback should only run code if scroll distance is more than 100px or so?
This will prevent entire code from running for scroll of every single pixel. I guess this will improve the way it works.
Click to expand...
So outside of listener put a step of 100px then run listener? That would for sure reduce a lot of the times called.
 
Upvote 0 Downvote
Sapoios said:
So outside of listener put a step of 100px then run listener? That would for sure reduce a lot of the times called.
Click to expand...
Not outside, check for scroll distance inside the listener only.
Put all code inside IF condition which checks for scroll distance. If distance is <100, the code shouldn't execute.

I've created a simple jsfiddle demo which will console.log on scroll of every 100px:

Edit fiddle - JSFiddle - Code Playground

Test your JavaScript, CSS, HTML or CoffeeScript online with JSFiddle code editor.
jsfiddle.net jsfiddle.net

Hope it helps.
 
Upvote 0 Downvote
joe said:
Not outside, check for scroll distance inside the listener only.
Put all code inside IF condition which checks for scroll distance. If distance is <100, the code shouldn't execute.

I've created a simple jsfiddle demo which will console.log on scroll of every 100px:

Edit fiddle - JSFiddle - Code Playground

Test your JavaScript, CSS, HTML or CoffeeScript online with JSFiddle code editor.
jsfiddle.net jsfiddle.net

Hope it helps.
Click to expand...
Well i tryed it and indeed makes a lot of less calls but somewhere i have messed up and smoth is not working. Thought it does not console any errors scrollintoview is not working with smooth on. If i leave only center works. I am confused.
Is it possible a css could prevent smooth scrolling?
 
Upvote 0 Downvote
Sapoios said:
Well i tryed it and indeed makes a lot of less calls but somewhere i have messed up and smoth is not working. Thought it does not console any errors scrollintoview is not working with smooth on. If i leave only center works. I am confused.
Is it possible a css could prevent smooth scrolling?
Click to expand...

I don't have much idea about nitty gritty details of scrollintoview.

Have added any new CSS which may affect the behaviour? If there is any transition, transform or keyframes it might affect it.
 
Upvote 0 Downvote
i figured the problem!!!
the listener listens also the horizontal scroll of the nav bar so it stops every other function when trying to scrolintoview

how can i prevent from listening the horizontal scroll off that div?
 
Upvote 0 Downvote
I tryed if e.target is my nav to do nothing and else to the code but it was still executing the code with both == and ===.
Also i found out that if i use ajaxcomplete() before listener,( because the elements loads with ajax call) , the listener runs 3 times for every pixel and i noticed that with console log the distance every 100 px it showed 3 times on console. Dont know why ajaxcomplete makes listener like what. I changed that with a time out function and logs one time as it should. Now i cant exclude the nav bar from listener. Can i move the potition of the nav bar without scrollintoview? I also tryed scroll left but same effect. Can translate x work?
Like translate y
 
Upvote 0 Downvote

New Threads

Latest posts

Share this page

Buy us a coffee!

Code Academy Advertisement
DataCamp Advertisement

About Us

Code Forum is a community platform where coding enthusiasts can connect with other developers, engage in discussions, ask for help, and share their knowledge with a supportive community. It's a perfect place to improve your coding skills and to find a community of like-minded individuals who share your passion for coding.

Site links

Stay Connected

Back
Top Bottom