rails-ujs icon indicating copy to clipboard operation
rails-ujs copied to clipboard

`data-disable` and `data-disable-with` with Turbolinks redirects

Open mrhead opened this issue 8 years ago • 1 comments

When Rails responds to an XHR request with redirect_to http://localhost:3000/, then it is converted to:

Turbolinks.clearCache()
Turbolinks.visit("http://localhost:3000", {"action":"replace"})

If the request was created by rails-ujs, then it is also processed by rails-ujs and the generated JavaScript is evaluated.

This is great, because this is expected. The issue is that it doesn’t work so great with data-disable and data-disable-with. Once the response is processed then the disabled element is enabled again for a little while until the page is replaced by Turbolinks.visit().

You can see it on the following example. I’ve added an artificial delay to the controller actions to make it more obvious:

turbolinks-ujs

Unfortunately this isn’t a made up issue and this is happening regularly in our real world application. From time to time users tend to click on a re-enabled element which is causing server side issues. I think that it would be the best to not re-enable disabled elements if the response is containing a Turbolinks redirect.

I know that this is not a rails-ujs issue and I am not sure if this should be solved in Turbolinks or rails-ujs (or somewhere else). Neither of them looks like a good candidate, because these libraries are independent and they shouldn't know about each other. But I would like to open a discussion and find the best solution. Also, I will be happy to submit a patch once we agree on a way how to solve this.

mrhead avatar Jan 23 '17 19:01 mrhead

Thank you for the issue. I believe this fix make sense to be in turbolinks-rails. What we could do is change the enable hook to be triggered only when Turbolinks.visit finish, only if turblinks is enabled.

Turbolinks-rails knowing that rails-ujs exists if fine since rails-ujs is basic infrastructure of Rails fraamework

rafaelfranca avatar Jan 24 '17 03:01 rafaelfranca