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

link_to_add_association compatibility with cells 4.0.2 #315

Closed
ryan2johnson9 opened this issue Sep 7, 2015 · 6 comments
Closed

link_to_add_association compatibility with cells 4.0.2 #315

ryan2johnson9 opened this issue Sep 7, 2015 · 6 comments

Comments

@ryan2johnson9
Copy link

Hi,

link_to_add_association is so good that I cannot do without it in my recent refactoring to the use of apotonick/cells

However it raised an error from within the Cocoon::ViewHelpers render_association method because it the render call was not compatible with rendering in cells.

I have written a monkey patch that works for me and I am wondering if you would like to support cells in your project. I will add my monkey patch and if you think it is worthwhile, I could make a pull request.

from within the cell, when you call link_to_add_association, pass in a render option cells_compatible:true like this:

link_to_add_association "add", @form, :association_name,
                          :partial => "_partial_name", render_options:{cells_compatible:true}

(note the leading underscore required in the partial_name)

in some model, or however you like to do monkey patches, (I use a class_extensions.rb file in my models directory) add this code:

require 'cocoon/view_helpers'
# overwrite the cocoon method which renders the partial for adding associations in the link_to_add_association helper method
module Cocoon
  module ViewHelpers
    def render_association(association, f, new_object, form_name, render_options={}, custom_partial=nil)
      partial = get_partial_path(custom_partial, association)
      locals =  render_options.delete(:locals) || {}
      cells_compatible = render_options.delete(:cells_compatible)
      method_name = f.respond_to?(:semantic_fields_for) ? :semantic_fields_for : (f.respond_to?(:simple_fields_for) ? :simple_fields_for : :fields_for)
      f.send(method_name, association, new_object, {:child_index => "new_#{association}"}.merge(render_options)) do |builder|
        partial_options = {form_name.to_sym => builder, :dynamic => true}.merge(locals)
        if cells_compatible
          render view: partial, locals: partial_options
        else
          render(partial, partial_options)
        end
      end
    end
  end
end

This simply overwrites the same method in Coccon which looks like this

def render_association(association, f, new_object, form_name, render_options={}, custom_partial=nil)
  partial = get_partial_path(custom_partial, association)
  locals =  render_options.delete(:locals) || {}
  method_name = f.respond_to?(:semantic_fields_for) ? :semantic_fields_for : (f.respond_to?(:simple_fields_for) ? :simple_fields_for : :fields_for)
  f.send(method_name, association, new_object, {:child_index => "new_#{association}"}.merge(render_options)) do |builder|
    partial_options = {form_name.to_sym => builder, :dynamic => true}.merge(locals)
    render(partial, partial_options)
  end
end
@nathanvda
Copy link
Owner

So if I understand correctly: you are using cells, you are using the cells folder-structure, so you need a different render so it will look for the correct partial. And this is not necessarily global, so on some pages you could use a "normal" link_to_add_association and in some pages you would be rendering a cell, containing some cocoon-form, correctly?

Just checking if it makes sense to be giving an extra parameter, or when using cells, we can always do render view: ... ?

@ryan2johnson9
Copy link
Author

Yes that is correct, I use link_to_add_association sometimes in cells and sometimes as it was designed to be used, though it seems to me a little more than just looking in the right place for the partial.

The number of parameters is different, i.e from cells it is one ( a Hash) normally it is 2 ( partial name, Hash). I think, though I do not understand properly yet, that cells overwrites the global render with this other implementation.

@ryan2johnson9
Copy link
Author

Closing this as it was suggested here trailblazer/cells#317 that it would be cleaner from me to overwrite the render method in Cells.

@asaletnik
Copy link

@ryan2johnson9 I tried to solve that problem without monkey-patching any methods, so I thought if we have to include Cocoon helper into Cells manually, why don't we include dedicated helper? On my fork I have added Cocoon::CellsHelper, that includes mostly standard Cocoon::ViewHelper, only difference is render_association calls render_partial method now, which is different for both helpers.

This way seems a little cleaner than both of your proposed solutions, and works well for me both in Cells and standard Rails views. Please correct me if I'm wrong 😃
asaletnik@7c864f8

@Startouf
Copy link
Contributor

@asaletnik Hey I tried your fix by monkeypatching the relevant methods in my cell. It works for the main part. The only problem remaining is the directory the partial is looked up in. Currently, if the code comes from a FooCell, it will only look up into the inherited cell folders app/cells/foo_and_inherited/ files ending.

But we might want to use a partial in app/cells/bar/ (eg. post_form.fields_for(:comments)) or even (?) app/views/.

If we want to allow looking up files in app/views/, we also need to to look at *html.erb extensions (currently only names in *.erb are resolved)

EDIT

It seems there's an issue with HTML escaping. Apparently the template of the data-attributes is inserted Escaped. This breaks everything.

<a class="add_fields" href="#" data-association-insertion-template=&lt;tr class=&quot.....

@jeffharrington
Copy link

@Startouf I reached the same point as you where the template was being unnecessarily HTML escaped.

Although it's not the most elegant solution in the world, I was able to resolve that by adding an html_safe flag to the output of the partial rendering like so:

if cells_compatible
  render(partial: partial, locals: partial_options).html_safe
else
  render(partial, partial_options)
end

As with any use of the html_safe flag, you need to be careful that you're not opening yourself up to XSS stuff. But with this, you're most likely rendering a partial from the server (and no user input).

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

5 participants