1
\$\begingroup\$

I have a bit of jQuery that does one thing on mobile, and a different thing on desktop.

On mobile it toggles a class on a div that shares a common class with the link, on desktop it does the same, except on hover.

It all works, but I found that when I had it on mobile and clicked the links a few times in quick succession (maybe 10 -15 presses of different links) my browser would suddenly freeze and then after a few seconds catch up, but every press afterwards would increase in hang-time, even if you gave it a few minutes to catch up.

I did a little bit of browser profiling and it stated that the if statement was the guilty section of code, but I'm not too sure how to make this run more cleanly. Is it the fact I am running this based off of window size?

Codepen

$(window).on('resize', function() {
  // Usually if screen width is less than 480px wide but for testing it's greater than 10px
  if ($(this).width() > 10) {

    $(".main > div a").click(function(e) {

      // Prevent default link behaviour
      e.preventDefault();
      // Once link is clicked do the following instead of default action

      // Add active link to element - Can I have the active class removed somehow too?
      $('.main .item a').removeClass('active');
      $(this).addClass('active');

      // Set the class of the parent div as a varible - the additional class on .item
      var className = '.' + $(this).parent('div').attr('class');

      // Remove class from all .drop_down divs and then add .visible to the one that has the same class as the link you click
      $('.subpages .drop_down').removeClass('visible');
      $('.subpages').find(className).addClass('visible');

      // This is a refresh for Owl Carousel 2 - their default refresh event is broken so I have to use this. Default found here -http://www.owlcarousel.owlgraphic.com/docs/api-events.html#refresh-owl-carousel
      if (document.createEvent) {
        window.dispatchEvent(new Event('resize'));
      } else {
        document.body.fireEvent('onresize');
      }
    });

  } else { // CHANGE WIDTH VALUE TO TEST THIS
    // Else restore click functionality
    $('.main .item a').unbind('click');

    // Remove visible class from all .drop_down divs
    $('.menu_contain .subpages .drop_down').removeClass('mobile_open');

    // Do same as above, but on hover instead
    $(".main > div").hover(function() {
      var className = '.' + $(this).attr('class');
      $('.subpages').find(className).toggleClass("visible");
    });
  }

});

// Run resize function above on page load

$(document).ready(function() {
  $(window).trigger('resize');
});
.main {
  border-bottom: 1px solid #DDDDDD;
}
.main > div {
  display: inline-block;
  margin-right: 10px;
  padding-bottom: 20px;
}
.subpages .drop_down {
  border-bottom: 1px solid #DDDDDD;
  display: none;
  padding: 10px 0;
}
.subpages .drop_down .item {
  margin-right: 10px;
  float: left;
}
.subpages .drop_down.visible {
  display: block;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/1.10.1/jquery.min.js"></script>
<p>Click/hover the links below to show/hide the corresponding section.
  <p>
    <p>You will need to change the value of the if statement on line 3 to test the other half of the function.</p>

    <!-- Main Links -->
    <h3>MAIN LINKS</h3>
    <div class="main">
      <div class="latest">
        <a href="http://codereview.stackexchange.com/">Latest</a>
      </div>
      <div class="news">
        <a href="http://codereview.stackexchange.com/">News</a>
      </div>
      <div class="insights">
        <a href="http://codereview.stackexchange.com/">Insights</a>
      </div>
      <div class="skills">
        <a href="http://codereview.stackexchange.com/">Skills</a>
      </div>
    </div>

    <!-- Associated Pages -->

    <div class="subpages">

      <div class="latest drop_down">
        <h3>Latest</h3>
        <p>Example Content</p>
      </div>

      <div class="news drop_down">
        <h3>News</h3>
        <p>Example Content</p>
      </div>


      <div class="insights drop_down">
        <h3>Insights</h3>
        <p>Example Content</p>
      </div>

      <div class="skills drop_down">
        <h3>Skills</h3>
        <p>Example Content</p>
      </div>

    </div>

\$\endgroup\$
2
\$\begingroup\$

You are firing off the resize event in the click event. The resize event then re-binds the click event so now you have two click events. The next time you click you have three, etc.

You should remove that code from the click event.

if (document.createEvent) {
    window.dispatchEvent(new Event('resize'));
} else {
    document.body.fireEvent('onresize');
}

I updated some of the code with the following:

  • Added an IIFE to create a private scope.
  • Made the resize and click code separate functions.

Here is the updated code:

(function($){

  function reSize() {
    if ( $(this).width() > 10 ) {
      $(".main > div a").off('click').on('click', clickHandler);

      if (document.createEvent) {
        window.dispatchEvent(new Event('resize'));
      } else {
        document.body.fireEvent('onresize');
      }
    } else {
      $('.main .item a').off('click');
      $('.menu_contain .subpages .drop_down').removeClass('mobile_open');
      $(".main > div").off('hover').on('hover', function() {
        $('.subpages').find('.' + $(this).attr('class')).toggleClass("visible");
      });
  }

  function clickHandler() {
    var $t = $(this);
    var subpages = $('.subpages');
    e.preventDefault();
    $('.main .item a').removeClass('active');
    $t.addClass('active');
    var className = '.' + $t.parent('div').attr('class');
    subpages.find('.drop_down').removeClass('visible').end()
            .find(className).addClass('visible');
  }

  $(function(){
    $(window).on('resize', reSize);
    $(".main > div a").on('click', clickHandler);
    $(window).trigger('resize');
  });

})( jQuery );
\$\endgroup\$
  • \$\begingroup\$ Thanks for taking a look Gary. I've just popped your code into CodePen and I'm getting an error for this section here: $(".main > div").off('hover').on('hover' function() {. And will your code here still have the same issue of running that resize event inside the click, because I noticed that code was still in there? \$\endgroup\$ – Lighty_46 May 7 '15 at 8:46
  • \$\begingroup\$ Wait, ignore the note about the resize event, it's not inside! I do need something similar to run when that link is clicked but I will search the Owl Carousel Github for an alternative. \$\endgroup\$ – Lighty_46 May 7 '15 at 9:02
  • \$\begingroup\$ You should be able to do a .trigger('resize') inside the click event above. I remove the click event before re-adding it. \$\endgroup\$ – Gary Storey May 7 '15 at 14:19
  • \$\begingroup\$ Also the error is on the hover line. I forgot a comma. I have updated the code above. \$\endgroup\$ – Gary Storey May 7 '15 at 14:39
  • \$\begingroup\$ Yup thanks Gary, got it all working nicely, just had to close the reSize function because it was missing a closing } and add an 'e' to clickHandler() ... also had to move the if (document.createEvent) { stuff to the bottom of the clickHandler function othrewise I got the error 'Uncaught RangeError: Maximum call stack size exceeded.' \$\endgroup\$ – Lighty_46 May 7 '15 at 15:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.