1

Suppose this is the logic that I want :

// On fail return status = 500, else success will return PDF bytestream
function request() {
  return $.ajax({
            url: url,
            async:false,
            // other properties
        });
}

$('#action').click(function(e) {  

    $("#loading").fadeIn("slow"); // display loading

    while(true) {
        if(request().status != 200) {
            delay(3000);
        } else { break; }
    }

    $("#loading").fadeOut("slow"); // hide loading
});

For testing delay without iteration and request, this code works well :

    $(document).ready(function(){
        $('#action').click(function(e) {  
            $("#loading").fadeIn("slow");
            setTimeout(function() {
                $("#loading").fadeOut("slow");
            }, 5000);
            e.preventDefault();
        });
    });

Problem started when I put some loops and request inside, like this :

    var isOk;
    $(document).ready(function(){
        $('#action').click(function(e) {  

            $("#loading").fadeIn("slow");

            while(true) {
                request().always(function(jqXHR){
                    console.log(jqXHR);
                    if(jqXHR.status == 500) {
                        isOk = false;
                        setTimeout(function(){console.log("False")}, 3000);
                    } else { isOk = true; }
                });

                if(isOk) {break};
            }

            $("#loading").fadeOut("slow");
            e.preventDefault();
        });
    });

It's like there's no delay in between iteration. The loading symbols fadeIn and fadeOut instantly. Any idea how to do the correct delay for iteration ?

Mrye
  • 699
  • 3
  • 10
  • 31

4 Answers4

3

Use promises (technically, jQuery Deferred in this example but same concept) to fix the flow, and save yourself a lot of headache:

$('#action').click(function(e) {  
  $('#loading').fadeIn('slow'); // display loading
  $.ajax(url).always(function (e) {
    $('#loading').fadeOut('slow'); // hide loading
  });
});

Note that I'm using .always()... that way even if there's an error, we'll still hide the loading indicator. You can add another handler for .fail() to catch errors.

Even better yet, use .show() and .hide(), and use CSS animations for any fading styling like this. While unlikely to matter much in this case, it makes these transitions extremely optimized in the browser engine, out of the JavaScript, while separating your application logic from styling even more.

Brad
  • 159,648
  • 54
  • 349
  • 530
  • Agree entirely... +1 – HelpingHand Oct 15 '16 at 17:44
  • He's retrying the ajax call if it fails, so this will fade out after the first call even if it's still retrying (your code doesn't retry, but he has this behavior in the question). – Jecoms Oct 15 '16 at 17:46
  • @Jecoms Yes, as it should. I just added a comment explaining that. If you want to re-try, you should have a function that filters the error with a `.then()`, transparently retrying. The outer promise shouldn't resolve or fail until all those retries are done. – Brad Oct 15 '16 at 17:47
  • @Jecoms yup, for futher info.. the request that i make is for checking current report status which is done asynchronous on server side.. if not ready, i will delay and retry request again.. I'll try everyone suggestion first – Mrye Oct 15 '16 at 17:54
  • 1
    @Mrye How you handle your retries is irrelevant to the fact that you should be using promises and a flow like this to handle your fade in/out of `#loading`. As I said, you can add a `.then()` to this to retry and filter if you want, or you can use some existing code: http://stackoverflow.com/a/12450161/362536 – Brad Oct 15 '16 at 17:57
1

You should call .fadeOut() when ajax request is resolved, like this:

$(document).ready(function(){

    $('#action').click(function(e) { 
        e.preventDefault(); 

        $("#loading").fadeIn("slow");

        performRequest();

    });

    function performRequest() {

            request()
                .always(function(jqXHR){
                    console.log(jqXHR);

                    if(jqXHR.status === 500) {
                       console.log("False");
                       var t = setTimeout(function() { 
                           performRequest();    // retry after 3sec.
                           clearTimeout(t);
                       }, 3000);
                    } else { 
                       onRequestSuccess(); // success
                    }
            });
    }

    function onRequestSuccess() {
         $("#loading").fadeOut("slow");
    }

});
Gab
  • 491
  • 3
  • 6
  • This is just clear and simple! no timeouts, no delays. – Gab Oct 15 '16 at 17:45
  • It's not your fault, just the OP's question contains code that is much more memory intensive than it needs to be. – HelpingHand Oct 15 '16 at 17:46
  • @Gab i need delays sry – Mrye Oct 15 '16 at 17:59
  • @HelpingHand I can't get your point, i've just given a correct answer... This snippet also retry if xhr fails, as the question suggest. – Gab Oct 15 '16 at 18:00
  • @Mrye I've added a 3 seconds delay when the request fails – Gab Oct 15 '16 at 18:02
  • It is a correct answer, but @Myre could be doing this in a much cleaner, less memory intensive way. Don't take it to heart man. He should be using css animations as Brad suggested. – HelpingHand Oct 15 '16 at 18:04
  • @Gab nicely done. Exactly followed the logic and requirement. But I will modify as suggested by the others. Thanks. – Mrye Oct 15 '16 at 18:54
0

Since the ajax call is asynchronous, it doesn't wait before executing the next line of code.

If you want to fadeOut on finish, run fadeOut() inside the callback function of the ajax call, or within this function.

Put the following right where you have //other properties:

complete: function(jqXHR, textStatus) {
  $("#loading").fadeOut("slow");
}

You have to use delay(), setTimeout() otherwise.

Delay:

$("#loading").fadeIn(2000);
$("#loading").delay(2000).fadeOut(2000);

setTimeout:

$("#loading").fadeIn(2000);
setTimeout(function(){
  $("#loading").fadeOut(2000);
}, 2000);

Just another suggestion, handle errors in your Ajax call, rather than in while loops:

error: function(jqXHR, textStatus, errorThrown) {
  setTimeout(function() {
    request();
  }, 3000);
}

And another, take @Brad 's advice and use CSS Animations instead.

Community
  • 1
  • 1
HelpingHand
  • 1,045
  • 11
  • 26
0

Here is what I was talking about. In this example, the fadeOut is executed after fadeIn is done as a callback. I included a timeout to show you can do that as well.

$(function() {
  $("#test").fadeIn(2000, function() {        
        setTimeout(function() { 
          $("#test").fadeOut(2000);
        }, 1000);
    });
});
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<div id="test" style="display:none">
  TEST DIV
</div>

If you want the loading to fade in before each ajax call and fade out after success, then you'll need to handle the fade out in your ajax success callback. Using your code, this would fade out upon completion.

$(document).ready(function(){
    $('#action').click(function(e) {  

        $("#loading").fadeIn("slow");

        while(true) {
            request().always(function(jqXHR){
                console.log(jqXHR);
                if(jqXHR.status == 500) {
                  setTimeout(function(){console.log("False")}, 3000);
                } else { 
                    $("#loading").fadeOut("slow");
                    break;
                }
            });
        }            
        e.preventDefault();
    });
});
Jecoms
  • 2,558
  • 4
  • 20
  • 31