Search code examples
javascriptmedia-queries

Javascript multiple media queries doesn't work


Somebody made this code for me online but it doesn't work. It still goes through the 2 other codes so when you click it goes through the first media query code then the second and then sometimes also the third. How can this be fixed. This is very important for me because i have a only one page website which is scrollable through the whole page. And we all know mobile users are the highest rate of users. So i really hope someone can fix this for me and explain me why this version isn't working.

Help is really really appreciated.

$(document).ready(function() {
    function mediaSize() {
        var isTabletLandscape = window.matchMedia('(min-width: 768px) and (max-width: 1366px) and (orientation: landscape)');
        var isPhoneLandscape = window.matchMedia('(min-width: 320px) and (max-width: 812px) and (orientation: landscape)');
        var isPhonePortrait = window.matchMedia('(min-width: 320px) and (max-width: 812px) and (orientation: portrait)');
        if (isTabletLandscape.matches) {
            //code for tablet landscape
            $('#menu ul li #page3').on('click', function(){
                $('html,body').animate({scrollTop: $(".footer").offset().top  * 0.78}, 850);
            });
        } else if (isPhoneLandscape.matches) {
            //code for phone landscape
            $('#menu ul li #page3').on('click', function(){
                $('html,body').animate({scrollTop: $(".footer").offset().top  * 0.89}, 850);
            });
        } else if (isPhonePortrait.matches) {
            //code for phone portrait
            $('#menu ul li #page3').on('click', function(){
                $('html,body').animate({scrollTop: $(".footer").offset().top  * 0.69}, 850);
            });
        } else {
            //code for desktop
            $('#menu ul li #page3').on('click', function(){
                $('html,body').animate({scrollTop: $(".footer").offset().top  * 0.99}, 850);
            });
        }   
    }
    mediaSize();

    window.addEventListener('resize', mediaSize, false);
});

Solution

  • Every time you call mediaSize() it's attaching a new click event listener. And this one, because it fires continuously while the window is being resized, adds a whole boatload of listeners:

    window.addEventListener('resize', mediaSize, false);
    

    (You asked me in comments to explain that further: the resize event fires continuously the whole time the user is resizing the window. So while the user is dragging the window borders around, the browser is calling mediaSize over and over, setting probably hundreds of duplicate event handlers on the same element.)

    One solution would be to cancel existing listeners using off() before setting a new one. It would be simpler, though, to only attach a single listener and have it respond to the current screen size, instead of constantly replacing listeners with slightly different ones whenever the screen size changes:

    $(document).ready(function() {
    
      // $('#menu ul li #page3') is unnecessarily complicated. IDs are unique, so all you need is #page3
      $('#page3').on('click', function() {
        // personally I'd check window.innerWidth here instead, but if this works, great:
        var isTabletLandscape = window.matchMedia('(min-width: 768px) and (max-width: 1366px) and (orientation: landscape)');
        var isPhoneLandscape = window.matchMedia('(min-width: 320px) and (max-width: 812px) and (orientation: landscape)');
        var isPhonePortrait = window.matchMedia('(min-width: 320px) and (max-width: 812px) and (orientation: portrait)');
    
        // you're doing the same thing in all of these conditions, except for the
        // number you're multiplying offset.top by, so let's just set 
        // that as a variable here:
        var multiple = 0.99;
        if (isTabletLandscape.matches) {
          multiple = 0.78;
        } else if (isPhoneLandscape.matches) {
          multiple = 0.89
        } else if (isPhonePortrait.matches) {
          multiple = 0.69;
        }
    
        // and now we only need to have one 'animate' block:
        $('html,body').animate({
          scrollTop: $(".footer").offset().top * multiple
        }, 850);
      })
    });
    

    This way you don't need to bother rebinding anything on window resize; the function will check the screen size only when it needs to.