1

I've had the following code working for some time in my Rails app:

 $(function () {

     // Change the link's icon while the request is performing
     $(document).on('click', 'a[data-remote]', function () {
         var icon = $(this).find('i');
         icon.data('old-class', icon.attr('class'));
         icon.attr('class', 'glyphicon glyphicon-refresh spin');
     });

    // Change the link's icon back after it's finished.
    $(document).on('ajax:complete', function (e) {
        var icon = $(e.target).find('i');
        if (icon.data('old-class')) {
            icon.attr('class', icon.data('old-class'));
            icon.data('old-class', null);
        }
    })

}

The code replaces a glyphicon with a "refresh" glyphicon and spins it until the AJAX request is complete, then replaces it with the original glyphicon.

I recently noticed the code had stopped working. The AJAX request works fine, but I no longer get my spinning refresh glyphicon during the AJAX call. It's possible an upgrade to Rails 5.1 has caused the problem.

I do have gem 'jquery-rails' in my Gemfile, so I had thought the Rails 5.1 upgrade was fully backward-compatible. It could be some other change that has cause this to break.

I've updated the first function as follows:

$('a[data-remote]').on('click', function () {
    var icon = $(this).find('i');
    icon.data('old-class', icon.attr('class'));
    icon.attr('class', 'glyphicon glyphicon-refresh spin');
});

This works the first time a button is clicked, but if the same button is clicked again, I get no spinner.

Is there a solution to this problem, preferably using the standard DOM API? I messed around with:

document.querySelectorAll('[data-remote]').onclick = function () { ... }

but haven't had any luck.

Am I even close to a workable answer? Would this be a good candidate for using addEventListener?

moveson
  • 5,103
  • 1
  • 15
  • 32
  • I'm not able to reproduce the behavior you're observing in a Rails 5.1 app. Are you using [Turbolinks](https://github.com/turbolinks/turbolinks) in your app? If so, you should wrap your Javascript inside `$(document).on('turbolinks:load', function(){...})` instead of `$(function(){...})`. – w_hile Jan 10 '18 at 01:09
  • I am not using Turbolinks at this time. I'd like to use it in the future, but I'd like to fix this problem first. – moveson Jan 10 '18 at 01:13
  • Since Turbolinks is not the issue, any other ideas why this would not be working? Using the original code, I've inspected the buttons, and the event handler is not being added. It seems the simplest solution would be to retain the `$(document).on('click', '???')` pattern but find a way to get it to attach to the existing buttons, no? – moveson Jan 10 '18 at 01:28
  • I was using the `$(a[data-remote])` pattern, but on switching to `$(document).on('click', ...)` I can reproduce behavior you mentioned. – w_hile Jan 10 '18 at 01:51
  • Binding to `body` instead of `document` appears to actually trigger the listener function: `$("body").on('click', ...)`. – w_hile Jan 10 '18 at 01:53
  • Brilliant! Yes, that fixes the problem! – moveson Jan 10 '18 at 01:58
  • I'll add an answer to reflect this new development. – w_hile Jan 10 '18 at 02:02

2 Answers2

3

@Jeff is probably right, you're replacing the link element and loosing the listener. This would not be a problem using the $(document).on('click', 'a[data-remote]', ...) pattern, as events fired on elements added to document after declaring the event handler would be caught (see this question). The issue is something seems to be preventing the link click event from bubbling up to the document, and the listener function fails to be called.

Binding to body instead of document appears to resolve the issue:

$("body").on('click', 'a[data-remote]', function () {
     var icon = $(this).find('i');
     icon.data('old-class', icon.attr('class'));
     icon.attr('class', 'glyphicon glyphicon-refresh spin');
 });
w_hile
  • 541
  • 6
  • 7
  • Do you think this is an inadvertent breaking change in Rails 5.1? – moveson Jan 10 '18 at 02:15
  • @moveson I'm not sure, but if the `$(document).on(...)` pattern stopped working when you upgraded to 5.1, then something must have changed in the way click events bubble up through the DOM. – w_hile Jan 10 '18 at 05:32
  • 1
    Not a change in Rails 5.1 as this is strictly a javascript issue. You bound the listener onto the element and once the element was replaced, you lost the listener as well. An analogy would be like buying an insurance policy for a car. You have the policy to the specific car, but the policy is useless if you no longer possess the car. Binding the listener to the body is equivalent to buying an umbrella insurance policy. No matter what car you own, the policy should still cover the new car (new element in this case). – Jeff Jan 10 '18 at 19:11
  • @Jeff Shouldn't binding to `document` have the same effect then? We're not sure why binding to `body` works, but binding to `document` does not. – w_hile Jan 10 '18 at 19:38
  • Im not sure why that isnt working but your best bet is to put a debugger right before the line you add the debugger to see if `$('document')` is the element you are looking for. Then put a debugger inside the document listener to see if it is being executed. – Jeff Jan 10 '18 at 22:27
2

Does your ajax request also refresh the element that has the original listener on it? Whats happening sounds like the listener is gone.

  1. $('a[data-remote]') is clicked and the listener tells the spinner to spin.

  2. The AJAX calls probably replaces some elements on the page including the original $('a[data-remote]') element.

Because the listeners arent added again after ajax page load/refresh, clicking the button the second time will not work.

Just a hypothesis but you can also put a debugger within the click handler and open the google chrome extension tools. See if the script pauses each time you click the button.

Update with basic fix:

function toggleIcons() {
  $('.regularIcon').toggle();
  $('.spinningIcon').toggle();
}

function toggleButtons() {
  $('.postButton').toggle();
  $('.deleteButton').toggle();
}

$('.postButton').click(function(e) {
  e.preventDefault();
  toggleIcons();

  // ajax call here
  $.ajax({
    type: "GET",
    url: "/route",
    data: data,
    dataType: "JSON",
  }).success(function(json) {
    toggleIcons();
    toggleButtons();
  })
});

// Listener for deleteButton
$('.deleteButton').click(function(e) {
  e.preventDefault();
  toggleIcons();

  // ajax call here
  $.ajax({
    type: "GET",
    url: "/route",
    data: data,
    dataType: "JSON",
  }).success(function(json) {
    toggleIcons();
    toggleButtons();
  })
});

Relies on you making sure both buttons are styled in the same place on the DOM.

Jeff
  • 650
  • 5
  • 16
  • Yes, that appears to be the problem. The script pauses only the first time the button is clicked. How would I add the handler back to the element after replacing it? – moveson Jan 10 '18 at 01:09
  • Youre going down a bad pattern but if you need to fix it, on the success callback of the AJAX request, just call the initial function that added the listener you have above. `$('a[data-remote]').on('click', ....` – Jeff Jan 10 '18 at 01:11
  • What's a better pattern? I have about 50 buttons on the page that need to have this event handler added when the page is first loaded. – moveson Jan 10 '18 at 01:11
  • I would probably either toggle classes on elements. For instance, on click of `$('a[data-remote]')`, hide the "nonloading" icon with jQuery `.hide()` and then call `.show()` on a hidden spinner. Then when the ajax call is complete, just reverse it. – Jeff Jan 10 '18 at 01:13
  • @Jeff Turbolinks could be loading the page body without attaching the listener. See my comment on the original question. – w_hile Jan 10 '18 at 01:14
  • @w_hile I'm not using Turbolinks, so that can't be the problem. – moveson Jan 10 '18 at 01:15
  • @Jeff is probably right, you're replacing the link element and loosing the listener. – w_hile Jan 10 '18 at 01:18
  • @Jeff I understand how this answer would fix the spinner problem, but I need the element to be replaced. The button is a toggle. It makes a POST call, shows the spinner, changes color. If clicked again it makes a DELETE call, shows the spinner, changes back. Not sure if I can do all that by showing and hiding things, but I'm open to suggestions. – moveson Jan 10 '18 at 01:23
  • Okay in that case, you can always have the two buttons on the page and show the other one once the ajax request is done. I will update my answer to explain what I mean. – Jeff Jan 10 '18 at 01:28
  • @moveson Hopefully that helps and you dont have to replace the buttons on the page. I'm not sure what your toggle buttons are doing, but deleting an item in your database should not be a "toggle" button. A toggle button would be something that makes something true/false like making a user active or inactive. – Jeff Jan 10 '18 at 01:34
  • The toggle button creates or deletes a subscription. Very similar to making a user active or inactive, but it needs to go out and create an AWS SNS subscription, and I need to keep track of the SNS id number in the database in a separate table. Thanks for your help and for the code example. I'll play with it and see if I can get it to work. – moveson Jan 10 '18 at 01:36
  • I'm accepting the answer from @Jeff, as that fixes my problem. I upvoted your answer as well. Thanks for taking the time to help out. – moveson Jan 10 '18 at 02:15
  • You accepted the wrong one but its okay. As long as someone reads this later on and learns – Jeff Jan 10 '18 at 06:10
  • 1
    Correction: I'm accepting the answer from @w_hile, as that fixes my problem. Sorry, I had the names mixed up. But both answers were very helpful to my understanding. – moveson Jan 10 '18 at 17:38