0

Here is my code:

$(document).on('click', '[data-submit-form]', function (event) {
    var form = $(this).closest('form');

    $.ajax({
        url : form.attr('action'),
        type : 'post',
        data: form.serialize(),
        async: false,
        success: function (data) {
            if ($.type(data) === 'object') {
                alert_errors(form, data);
                event.stopImmediatePropagation()
            }
        }
    });
});

This uses AJAX to check the server for errors when the button is clicked. If there were errors, it returns an object containing the errors. In the case of an error, I want to stop all other events from occurring on the clicked button. For example, the button could also be used to close a modal. I want to stop this from happening if there were errors.

This is not working at all. It seems event.stopImmediatePropagation() has no effect on the other events of the button after the alert is shown.

kjdion84
  • 9,552
  • 8
  • 60
  • 87
  • Try using .done and .fail – TGarrett Aug 18 '17 at 14:53
  • one question before i give you a suggestion, why are you using on('click') instead of a more specific selector like $(#form).submit(..) – Santiago Benitez Aug 18 '17 at 14:56
  • using `.done` has the exact same result. – kjdion84 Aug 18 '17 at 15:24
  • I would suspect it's due to the callback you are using. Maybe it isn't as synchronous as you'd hope it was. this isn't really worth debugging, stop using async: false and find another way. – Kevin B Aug 18 '17 at 15:32
  • For example, always cancel the event, then if you want the submit to go through, submit it with form[0].submit() thus bypassing events and submitting the form (such as triggerHandler) – Kevin B Aug 18 '17 at 15:33
  • I'm submitting the form to the server for server-side validation which uses a Laravel controller. You're suggesting that I submit the exact same form a second time should validation pass? Sounds awful. – kjdion84 Aug 18 '17 at 15:40
  • @kjdion84 is that not what you're doing anyway? my point is, take the action you're preventing after the ajax request, rather than waiting to prevent it after. – Kevin B Aug 18 '17 at 15:52
  • 1
    Instead of making something happen on `'click'`, and then trying to prevent that thing from happening under certain circumstances, why not have your other thing (closing a dialog, e.g.) happen as a result of your successful AJAX request in the first place? Then you don't have to use `async: false`, which is generally a bad idea in the first place. – StriplingWarrior Aug 18 '17 at 15:53
  • Because I want to chain events but break propagation under certain circumstances so that my code is easily maintainable? – kjdion84 Aug 18 '17 at 15:55

2 Answers2

2

The problem here has nothing to do with synchronous or asynchronous code.

Yes, if your AJAX call is asynchronous then that would be a problem, because event delegation is synchronous and will not wait for your AJAX call to return. But your call is synchronous, since you set async: true.

The real underlying problem here is time.

The HTTP request you are making -- even though it is synchronous since you set async: false -- takes time to complete, and by the time it has completed, the event delegation of the browser has already happened, thus your event.stopImmediatePropagation() has no effect.

Ideally, you need to move your event.stopImmediatePropagation to another spot, like so:

$(document).on('click', '[data-submit-form]', function (event) {
    var form = $(this).closest('form');

    event.stopImmediatePropagation(); // Do it here

    $.ajax({
        url : form.attr('action'),
        type : 'post',
        data: form.serialize(),
        async: false,
        success: function (data) {
            if ($.type(data) === 'object') {
                alert_errors(form, data);
                // event.stopImmediatePropagation() NOT here
            }
        }
    });
});

EDIT

Your current approach is not the most ideal way to do what you are trying to do, because your modal close and form submit logic are too tightly coupled since they're in the same click event.

Instead of having a submit form click event that also closes the modal, you should make the process slightly more granular with separate functions:

  • One to handle the click event of the button and submit the form
  • One to close the modal, which can be called from anywhere

With this approach, your code can stay as is and you call the closeModal function inside the success callback, like so:

// By default, this will NOT close the modal anymore. You must explicitly
// call 'closeModal' where you want, e.g. in the AJAX success callback.
// This type of granularity will be much more flexible and save you headache.
$(document).on('click', '[data-submit-form]', function (event) {
    var form = $(this).closest('form');

    $.ajax({
        url : form.attr('action'),
        type : 'post',
        data: form.serialize(),
        async: false,
        success: function (data) {
            if ($.type(data) === 'object') {
                alert_errors(form, data);
                closeModal(); // Close modal
            }
        }
    });
});

An approach like this is much cleaner because as your functionality grows and grows, you're going to keep finding yourself with more and more weird scenarios in which you want to call preventDeafult, stopPropagation, stopImmediatePropagation, and so on, and I guarantee you will run into this same problem again as a result. It will just be a mess.

Save yourself the hassle now.

Lansana Camara
  • 9,497
  • 11
  • 47
  • 87
  • I only want to stop propagation if the data type returned from the AJAX call is an object. What you're suggesting would stop all other events on the element from firing regardless of the AJAX response. – kjdion84 Aug 18 '17 at 16:04
  • I think you should not have logic such as "close modal" on a form submit button. They are two separate actions, and should not be combined into one action. Rather, *inside* your form submit logic, you should call a `closeModal()` function or something similar in the success callback of the AJAX response, instead of the `event.stopImmediatePropagation()`. This makes your code more much flexible and you can avoid these strange errors. – Lansana Camara Aug 18 '17 at 16:09
  • Edited answer with an example. – Lansana Camara Aug 18 '17 at 16:14
0

You cannot prevent an event handling when that handling has been already completed. The order of the events is the following:

User clicks the button

  • Immediately:
    1. The AJAX request is sent.
    2. The rest of handling is done.
  • After a few milliseconds: (EDITED) The AJAX response is received but the callback is ignored because this call was synchronous.

Based on this response your code should be something like:

$(document).on('click', '[data-submit-form]', function (event) {
    var form = $(this).closest('form');

    var data= JSON.parse(
        $.ajax({
           url : form.attr('action'),
           type : 'post',
           data: form.serialize(),
           async: false
        }).responseText; //The ajax call returns the response itself

    if ($.type(data) === 'object') {
        alert_errors(form, data);
        //event.stopImmediatePropagation()
    } else {
        //rest of the event handling
    }
});

But anyways I think you should use a async call, as sync ones are deprecated by browsers and even for jQuery (the documentation already removed all the synchronous examples and discourages its use)

Pablo Lozano
  • 10,122
  • 2
  • 38
  • 59
  • How is that possible when I've added `async: false` to the AJAX? You're telling me jQuery still completes event handling even though the AJAX request may not have completed yet? – kjdion84 Aug 18 '17 at 15:48
  • Ooops, you're right, editing, you have a slightly different problem. – Pablo Lozano Aug 18 '17 at 16:02