0

The context:

I have a form that asks for a zipcode, state, and city. Next to each input field I have two spans, one that shows a green checkmark if the input is good, and one that shows a red x if the input is bad. Naturally only one is visible at any given time.

<input type="text" id="Zip" class="form-control">
<span id="ZipOK" style="display:none;" class="glyphicon glyphicon-ok"></span>
<span id="ZipBad" style="display:none;" class="glyphicon glyphicon-remove"></span>

<input type="text" id="State">
<span id="StateOK" style="display:none;" class="glyphicon glyphicon-ok"></span>
<span id="StateBad" style="display:none;" class="glyphicon glyphicon-remove"></span>

<input type="text" id="City">
<span id="CityOK" style="display:none;" class="glyphicon glyphicon-ok"></span>
<span id="CityBad" style="display:none;" class="glyphicon glyphicon-remove"></span>

I have a function that detects input to the zip field and then does a database call to get the state and city from the zip code and auto fill the form.

jQuery('.form-control').keyup(function(){
    validateZipcode();
});

This is the function that does the ajax, autofills the state/city and hides/shows the appropriate feedback spans.

function validateZip() {
    zip = $('#Zip').val();
    if (zip.length === 5 && $.isNumeric(zip)) {
        $.ajax({
            type:"POST",
            url: '?report=ajax&request=getStateAndCityFromZip',
            data:"Zip="+encodeURIComponent(zip),
            success: function(output) {
                output = $.parseJSON(output);
                if (output['State'].length > 0 && output['City'].length > 0) {
                    $('#State').val(output['State']);
                    $('#City').val(output['City']);
                    $('#StateOK').fadeIn();
                    $('#StateBad').hide();
                    $('#CityOK').fadeIn();
                    $('#CityBad').hide();
                    $('#ZipOK').fadeIn();
                    $('#ZipBad').hide();
                } else {
                    $('#ZipOK').hide();
                    $('#ZipBad').fadeIn();
                }
            },
            error: function (xhr, ajaxOptions, thrownError) {
            }});
    } else {
        $('#ZipOK').hide();
        $('#ZipBad').fadeIn();
    }
}

The problem:

This code works as is, however there are some instances where if I put in the zip code extremely fast, both the #ZipBad and #ZipGood spans will end up visible. Typing at relatively normal or slow pace results in expected behavior.

I assume this has something to do with asynchronous calls to validateZip(), but I don't really know enough about javascript to know how to fix this. Does anyone have any ideas?

Spencer Wood
  • 610
  • 6
  • 15
  • 1
    You need to cancel the previous ajax request before making the new one. See: http://stackoverflow.com/questions/4551175/how-to-cancel-abort-jquery-ajax-request – Mike Furlender Jun 16 '15 at 00:33
  • Additionally, you need to debounce the request. Debouncing is kinda like rate-limiting. See here: http://benalman.com/code/projects/jquery-throttle-debounce/examples/debounce/ – Mike Furlender Jun 16 '15 at 00:35
  • _"both the #ZipBad and #ZipGood spans will end up visible"_ `#ZipGood` not appear at `html` , `js` ? What is expected result if `#ZipOK` is hidden an `keyup` event handler called ? – guest271314 Jun 16 '15 at 01:10
  • You might want to [`stop`](http://api.jquery.com/stop/) your animations before hiding the elements – Bergi Jun 16 '15 at 02:11
  • @Bergi _"You might want to stop your animations"_ Yes. Posted this suggstion at answer below. Besides not caching selectors , is there an error or issue with approach at http://stackoverflow.com/a/30857418/ ? – guest271314 Jun 16 '15 at 02:13
  • @guest271314: Let me check… looks like your answer doesn't state the need for stopping the animations, and also makes unnecessary use of the duration and callback parameters for `hide()`. That's probably the reason why it was downvoted. – Bergi Jun 16 '15 at 02:20
  • 1
    @guest271314: `.hide()` (without arguments) does not animate anything. rtfm. – Bergi Jun 16 '15 at 02:28
  • @Bergi _".hide() (without arguments) does not animate anything."_ Can demonstrate ? No 400ms default duration to set `display` property of element ? What is "rtfm" ? – guest271314 Jun 16 '15 at 02:30
  • 1
    @guest271314 [RTFM](https://en.wikipedia.org/wiki/RTFM): "*[With no parameters, the matched elements will be hidden immediately, with no animation.](http://api.jquery.com/hide/)*". You can try it out yourself if you need to see a demonstration. – Bergi Jun 16 '15 at 02:36
  • @Bergi _"With no parameters, the matched elements will be hidden immediately, with no animation."_ Could not find that text at http://api.jquery.com/hide . There is still the **duration (default: 400)** duration http://api.jquery.com/hide/#hide-duration-complete . see http://jsfiddle.net/sanku1ma/ – guest271314 Jun 16 '15 at 02:44
  • 1
    @guest271314: You are using the callback parameter in your example. That's not exactly "*with no parameters*". – Bergi Jun 16 '15 at 02:47
  • @Bergi Found text at documentation. Yes, you are correct. http://jsfiddle.net/sanku1ma/1/ – guest271314 Jun 16 '15 at 02:55

1 Answers1

3

If the user types fast, then you can end up with multiple animations in process at the same time and the .hide() which is not an animation will go in the wrong sequence, potentially ending up with the wrong display. The animations queue in order, the .hide() does not so things can get out of sequence.

There are also some edge cases where you could end up with multiple ajax calls in flight at the same time, but this would require typing 5 characters in the zip field, then quickly backspacing and typing a fifth character again and this could end up confusing things too.

You can defend against the animations by using .stop(true) before every animation or visibility change to stop any currently running animations and you can defend against the multiple ajax calls by canceling any previous call like this:


Here's one way you could implement this:

var lastValidateAjax;
function validateZip() {
    var zip = $('#Zip').val();
    if (zip.length === 5 && $.isNumeric(zip)) {
        if (lastValidateAjax) {
            lastValidateAjax.abort();
        }
        lastValidateAjax = $.ajax({
            type:"POST",
            url: '?report=ajax&request=getStateAndCityFromZip',
            data: {Zip: zip},
            success: function(output) {
                lastValidateAjax = null;
                output = $.parseJSON(output);
                if (output.State.length > 0 && output.City.length > 0) {
                    $('#State').val(output.State);
                    $('#City').val(output.City);
                    $('#StateOK, #CityOK, #ZipOK').stop(true).fadeIn();
                    $('#StateBad, #CityBad, #ZipBad').stop(true).hide();
                } else {
                    $('#ZipOK').stop(true).hide();
                    $('#ZipBad').stop(true).fadeIn();
                }
            },
            error: function (xhr, ajaxOptions, thrownError) {
                lastValidateAjax = null;
            }
        });
    } else {
        $('#ZipOK').stop(true).hide();
        $('#ZipBad').stop(true).fadeIn();
    }
}

FYI, I also took the opportunity to use multiple items in the same select and to use jQuery chaining to simplify the code.

See this other answer for a discussion of jQuery's .abort() for ajax calls:

Abort Ajax requests using jQuery

Community
  • 1
  • 1
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • _"If the user types fast, then you will have multiple ajax calls in process at the same time."_ This is not correct. See `if (zip.length === 5 && $.isNumeric(zip)) {` at OP – guest271314 Jun 16 '15 at 02:03
  • 1
    @guest271314 - there is one edge case that could result in multiple ajax calls which is typing 5 number, then quickly hitting backspace and then another number. That probably isn't what the OP is running into, but it should be defended against also. – jfriend00 Jun 16 '15 at 03:10
  • _"there is one edge case that could result in multiple ajax calls which is typing 5 number, then quickly hitting backspace and then another number"_ Multiple ajax calls would be expected result here ? – guest271314 Jun 16 '15 at 03:34
  • 1
    @guest271314 - multiple ajax calls in flight at the same time is just another opportunity for things to get out of sequence or overlapping animations so those things need to get defended against. Ajax results are not guaranteed to arrive in the order sent out, but in a situation like this you only ever want to process the last request sent. It's just one more thing to make sure doesn't cause a problem. – jfriend00 Jun 16 '15 at 03:37
  • Yes, ok. Would caching queries , results further reduce possibilities for issues ? i.e.g. http://jqueryui.com/autocomplete/#remote-with-cache ; not calling ajax if query previously cached ? – guest271314 Jun 16 '15 at 03:43
  • @guest271314 - caching just makes some responses faster and saves the server some work, it doesn't eliminate the possibility of overlapping responses. – jfriend00 Jun 16 '15 at 03:47
  • Worked great, thank you! Wasn't aware of jQuery chaining, that makes things much cleaner. – Spencer Wood Jun 16 '15 at 22:07