Fading and Loader jQuery Improvement

Question

I'm not much of a jQuery coder (more of a PHP coder), except to call a few get values out of class and id and Ajax calls. However, I need to make a checkout system work and one thing I would like to do is to put a loading wait while I send a form through Ajax. When it's finished, it would redirect the user.

So on click:

1. load the loading spinner
2. fade the background out
3. Ajax call
4. when Ajax is done submitting a form which indirectly redirects.

Here is my code, following why I don't like it:

jQuery.fn.center = function () {
            this.css("position","absolute");
            this.css("top", ( $(window).height() - this.height() ) / 2+$(window).scrollTop() + "px");
            this.css("left", ( $(window).width() - this.width() ) / 2+$(window).scrollLeft() + "px");
            return this;
        }

        $( ".buttonFinish" ).on('click', function(event) {
            $('body').fadeOut('slow');
            $('#loader').center();
            $('#loading').center();
            $('#loader').show();
            $('#loading').show();
            //Use the above function as:
            var request = $.ajax({
                url:'{{ URL::route('saveCart') }}',
                type:'POST'
                });

                request.done(function(msg) {
                    $('.order_number').val(msg);
                      $('#summaryForm').submit();
                      event.preventDefault();
                });
            return false;
        });

Let's get a few questions out of the way: {{UR::route('')}} is laravel syntax to call the routing name for URL. loader/loading are two div that I have for the spinner one is text the other is an animation through CSS. (I'd rather CSS it than to load a pix, but I am aware that loader.gif can be small)

My issue (and feeling) that this can be better. I am using CSS with display:none to hide my two div, and then I call fadeOut, show, show, and center center function to center my loading Ajax in order to center things. Is there a compact way to achieve all the 4 steps needed in a clean way?

Answer 1

Let's go through it one section at a time.

jQuery.fn.center = function () {
            this.css("position","absolute");
            this.css("top", ( $(window).height() - this.height() ) / 2+$(window).scrollTop() + "px");
            this.css("left", ( $(window).width() - this.width() ) / 2+$(window).scrollLeft() + "px");
            return this;
        }

• You don't have to explicitly append 'px' to your numeric values. jQuery is smart enough to correctly format your CSS styles.

• Since this is a jQuery plugin, you return this; at the end of the function. This is good, as it preserves chaining.

• If the .center function is only used for the loading divs, consider removing it and instead styling the divs with plain CSS.

  #loader {
      position: fixed;
      top: 50%;
      left: 50%;
      margin-top: -50px; /* half of the height */
      margin-left: -100px; /* half of the width */
  }
  ...
  

---

$( ".buttonFinish" ).on('click', function(event) {
    $('body').fadeOut('slow');
    $('#loader').center();
    $('#loading').center();
    $('#loader').show();
    $('#loading').show();

• As mentioned by @Flambino, this particular code (as presented in the question) will fade out the entire body of the HTML document, which includes everything, even the loader elements. Perhaps the code that you're actually using is slightly different (is the selector .body or #body instead?).

• It's best practice to enclose JavaScript that uses external dependencies inside an IIFE. By doing so, you can use the "safe" name of an external dependency while still being able to alias is to a more convenient identifier locally. In addition, you can freely create variables inside the scope of the IIFE without polluting the global namespace. Your example code is fairly short, but you likely have much more code on the page, and it is a good habit to have regardless. Here's one way to do it:

  (function($) {
      $( ".buttonFinish" ).on('click', function(event) {
          ...
      });
  })(jQuery);
  

• You can select both elements at the same time, just like in CSS: $('#loader, #loading').

• You can chain the .show() and .center() together: $('...').show().center().

---

    //Use the above function as:
    var request = $.ajax({
        url:'{{ URL::route('saveCart') }}',
        type:'POST'
        });

        request.done(function(msg) {
            $('.order_number').val(msg);
              $('#summaryForm').submit();
              event.preventDefault();
        });
    return false;
});

• Since you're not using the request variable for anything other than .done(), you can skip declaring it and simply chain .done() after $.ajax():

  $.ajax({
      ...
  }).done(function(msg) {
      ...
  });
  

As an overall note, the indentation of the code (as it is in the question) seems haphazard, making the control flow more difficult to see than it could be. Making matching braces have the same indentation (and, in general, maintaining a consistent style) will help make the code more readable and understandable.