Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rails 4 + Turbolinks + Destroyed fields + Error on create / update #244

Closed
fredericboivin opened this issue Nov 21, 2014 · 20 comments
Closed

Comments

@fredericboivin
Copy link

With Rails 4 and Turbolinks, code is not reloaded / executed on resource creation fail as demonstrated in the documentation of Turbolinks :

Turbolinks will be enabled only if the server has rendered a GET request.

Some examples, given a standard RESTful resource:

    POST :create => resource successfully created => redirect to GET :show
        Turbolinks ENABLED
    POST :create => resource creation failed => render :new
        Turbolinks DISABLED

This means that since cocoon.js is not wrapped in a function running on page:load / document.ready, the code is not executed on creation/update fail and the following function [ $(".remove_fields.existing.destroyed").each (i, obj) ->] does not run, thus destroyed_fields are not hidden

Simple fix would be to wrap the vendor js provided with the Cocoon gem with in :

ready = ->
 ...( )....
$(document).ready(ready)
$(document).on('page:load', ready)
@nathanvda
Copy link
Owner

Good find. Can you make a pull request?

@itsNikolay
Copy link

If you like simple solutions 😎
#258

@simi
Copy link
Contributor

simi commented Jan 18, 2015

I think #258 is better solution.

@nathanvda
Copy link
Owner

I disagree.

@simi
Copy link
Contributor

simi commented Jan 19, 2015

I don't think that $(document).on('page:load', ready) should be part of cocoon. It is useless without turbolinks.

@nathanvda
Copy link
Owner

The only part that should be in the page:load is the hiding of the removed fields. Listening to an event that is never triggered (without turbolinks) has near to no cost, afaik. Anyway: the js as it is now will not work correctly with this solution, so using jquery.turbolinks is not a solution.

@bricesanchez
Copy link

Do you have any update and/or fix with cocoon and turbolinks ?

@itsNikolay
Copy link

@bricesanchez as I remember I made it work with jquery-turbolinks gem, without any addition JavaScript code

@bricesanchez
Copy link

@itsNikolay Could you provide me an example ?

When i try to use it with turbolinks, link_to_add_association goes mad when i click on it and clone multiple times my content instead of just one time.

It works when i do a complete refresh of the page.

I'm on rails 4.2.0, it's an refinerycms app (3.0.0), i've disabled my custom JS to report this issue.

This is my application.js manifest :

//= require jquery
//= require jquery.turbolinks
//= require jquery_ujs
//= require turbolinks

//= require cocoon

I've 2 forms in 2 pages (one by page).

@nathanvda
Copy link
Owner

@bricesanchez sorry, I have been flooded with work so was not able to continue working on this.

The only case where this error has effect is on invalid forms that are posted with deleted elements, and those are not hidden. So, imho, this is a bit of an edge case. The jquery.turbolinks as mentioned as mentioned above will not solve it, as it will attach the click-handler multiple times.

Simple work-around for now (if possible): disable turbolinks on that specific page by adding “data-no-turbolink” to the tag.

I will try to update cocoon asap including this fix (e.g. somewhere around the weekend 🙏 )

@bricesanchez
Copy link

@nathanvda, according to jquery.turbolinks, we have to require turbolinks at the end of our application.js manifest, it fixed my problem.

//= require jquery
//= require jquery.turbolinks
//= require jquery_ujs
//
// ... your other scripts here ...
//
//= require turbolinks

Thanks for your reply and you're great job on this Gem! 😄

@wyaeld
Copy link

wyaeld commented May 9, 2015

I ran into similar problems here, although more complex because I have a server side validation to ensure the last instance of the nested model cannot be removed (validates at least 1 of them present).

This means the opposite of the above, because I need the destroyed fields to not be hidden if they specifically were the thing that failed validation

I've solved it (seemingly) with the following:

# _form.html.erb
<%= javascript_tag "resetCocoonDestroyed();" if @my_model.errors[:codes].present? %>
// cocoon_form_reset.js
this.resetCocoonDestroyed = function() {
    return $('.remove_fields.existing.destroyed').each(function() {
      var el = $(this);
      el.removeClass('destroyed');
      return el.prev("input[type=hidden]").val("0");
    });
  };

This means any subsequent form submissions won't try to remove the element that I rejected.

This is working with Turbolinks3, and without jquery.turbolinks, and I didn't have to add data-no-turbolink anywhere on the form

@nathanvda How are you coming with your overall solution?

@Tensho
Copy link
Contributor

Tensho commented May 19, 2015

@nathanvda, not related to turbolinks, just basically cover destroyed fields hiding with document ready callback ^_^ #287

@RealSilo
Copy link

@Tensho , @nathanvda , @wyaeld Guys, has sby already solved this issue properly? I'm pretty beginner at JS, so for me it's not that easy to figure this out on my own.

@nathanvda
Copy link
Owner

I fixed the edgecase, so cocoon is now completely turbolinks compliant and should now always work as expected.

Only for @wyaeld there will be some impact: you might have to re-show the hidden fields 😦

@wyaeld
Copy link

wyaeld commented Feb 25, 2016

Ok thanks @nathanvda

I'll have a look next time I upgrade, that system using it doesn't get a lot of change, so should be fine.

@RealSilo
Copy link

RealSilo commented Mar 1, 2016

Thanks @nathanvda! I will upgrade right away!

@brianfalah
Copy link

I still have this problem with Rails 5.1 and the last version of cocoon (1.2.11).
I'm not using turbolinks, but I get this same problem in the same situation, the code inside $(document).on("ready page:load turbolinks:load", function() { never gets executed after the view is rerendered (after a validation fails).

@nathanvda
Copy link
Owner

@brianfalah which jquery version are you using? The ready event is deprecated in newer versions, and is left there for compatibility reasons. Old turbolinks requires both page:load and ready, turbolinks 5 triggers turbolinks:load also on normal page ready.

Easy solutions (for now): use an older jquery version or use turbolinks.

There is an open PR #447 to fix this, I will try to get it merged.

@brianfalah
Copy link

Ok many thhanks @nathanvda. That is my problem, absolutely.
Please, if you can get it merged it would be great. Because I am having some other problems in my app with turbolinks.
Apart of that, it shouldn't be mandatory to have turbolinks installed.

Thanks, for your quick response

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants