nested_form icon indicating copy to clipboard operation
nested_form copied to clipboard

Suggestions on link_to_add and blueprint functionality

Open tiagoblackcode opened this issue 13 years ago • 2 comments

First of all, congratulations on such nice gem. I think this kind of feature should be deep inside the core of form builders as it is a very common problem when developing web applications.

I like this gem specially because it has a nice, clean approach. Although, there are a few suggestions I think should be considered on a next release (I also am able to help with that if it goes along).

First, I've noticed the new wrapper => false option that can be sent along the f.fields_for method call which is indeed very useful to provide custom containers to wrap the fields (specially when we want to wrap the fields in a table). Although it is very useful and clean, it gets a bit ugly when it comes to the javascript part.

Every time I want to have such custom wrapping options, I find myself overriding the window.nestedFormEvents.insertFields so it can find the proper container to add the fields into (taking the table example, the container would be something like table.purchase_products_fields tbody. Imagine if you have 3 or 4 nested models inside a form. You'll have to have some programming going on in the insertFields override until you get things right.

This leads my to the following suggestion: why not add the option to provide a container to the link_to_add function, which would keep the user-code simpler and less error-prone? This not only fits the above use case I mentioned, but also applies when the link_to_add is placed in something other than the nested fields container. Something like f.link_to_add "Add", :purchase_products, :container => "table.purchase_products_fields tbody" would be more than enough.

Secondly, and this is because I'm overwhelmingly meticulous is that the blueprint should be wrapped in something other than a div, because it is actually a template. My suggestion is to place it inside a <script type="text/template"></script> tag, which keeps the browsers from evaluating the DOM inside the script tag and keeps the template in a nice, clean container without any data-blueprint workaround going on.

Please note that this is are only suggestions and I am open to discuss this more specifically. I don't intend in any way to criticize the decisions taken which, as I said before, are part of a great job made by all the contributors. My perspective is to make things better and give you some feedback from someone that uses this gem quite a lot!

There are some assumptions that may be outdated as I haven't had the time to check the master branch, but I hope you get the point.

Best regards, Tiago Melo

tiagoblackcode avatar Nov 07 '12 15:11 tiagoblackcode

Thanks for suggestions. Here is my opinion about them:

  1. I'm open to adding new features to window.nestedFormEvents.insertFields but:
    • they should use data- attributes on a link, e.g.: f.link_to_add "Add", :purchase_products, :data => { :container => "table.purchase_products_fields tbody" } which helps us to avoid modifying link_to_add helper
    • they should be properly tested
  2. I'm not a fan of using <script type="text/template"></script> because people may unintentionally have script tags in their blueprints and it'll break html completely.

lest avatar Nov 13 '12 15:11 lest

I gave the container functionality a shot in #278. Our use case was css conflicting when using the class "fields".

stsc3000 avatar Jul 13 '13 00:07 stsc3000