Better way of doing this?

JavaScript
1

Here is the code snippet that I’m trying to make better

var slider = $("#slider");

    slider.on("click", "#up", function () {
      var ul = $(".slider-small-box");
      var marginTop = ul.css("margin-top");
      var ulHeight = ul.height();

      if(ulHeight + parseInt(marginTop, 10) <= 555) {
        ul.css("marginTop", "0px");
      } else {
        var s = parseInt(marginTop, 10);
        s = s - 185;
        ul.css("marginTop", s + "px");
      }
    });

The problem with the above code is that it have to search the DOM for “.slider-small-box” everytime the “up” button is clicked (the “up” button is a button tag like this: <button id="up">UP</button>). Everything is working fine and smoothly but I want to write better code.
You might be asking “So why don’t you just reference the “.slider-small-box” element outside of the click event?”. The reason is that the element “.slider-small-box” and the button “#up” are both dynamically inserted into the page using AJAX and because of that I can’t reference them outside the “click” event function.

I’ve tried to find a solution for that and I stumbled upon jQuery deferreds. Here is how the code looks like when I use jQuery deferreds:

  $.sliderResults().done( function () {

    var ul = $(".slider-small-box");
    var marginTop = ul.css("margin-top");
    var ulHeight = ul.height();
    var up = $("#up");    

    up.on("click", function () {
      if(ulHeight + parseInt(marginTop, 10) <= 555) {
        ul.css("marginTop", "0px");
      } else {
        var s = parseInt(marginTop, 10);
        s = s - 185;
        ul.css("marginTop", s + "px");
      }
    });
  }

The problem with the above code is that the “click” event can only be executed once, so that when I click the “up” button again nothing happens.

2

Use $(document).on('click', '#up', function(e) {... instead.

3

What is the difference?
I don’t see any difference. I will stay unable to reference “.slider-small-box” outside the event function.
What I mean is that I will stay unable to do this:

      var slider = $("#slider");
      var ul = $(".slider-small-box");
      var ulHeight = ul.height();

    slider.on("click", "#up", function () {
      var marginTop = ul.css("margin-top");

      if(ulHeight + parseInt(marginTop, 10) <= 555) {
        ul.css("marginTop", "0px");
      } else {
        var s = parseInt(marginTop, 10);
        s = s - 185;
        ul.css("marginTop", s + "px");
      }
    });

Compare the above code with the first block of code in the post to understand what I’m trying to do.

4

It registers the event to the document object, instead of the element itself. Allowing you to dynamically register events on elements created after the JS has fully loaded.

You can. You can’t access new ones because you’ve assigned the class array to the ul variable outside of the click event. If you’re looking to get away from searching the DOM and rebuilding the array of elements, you can maintain and update the array of elements yourself. But, that’s really just micro-optimization and not worth the effort.

6

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.