Skip to content
This repository has been archived by the owner on Dec 12, 2021. It is now read-only.

Suggestions on link_to_add and blueprint functionality #216

Open
tiagoblackcode opened this issue Nov 7, 2012 · 2 comments
Open

Suggestions on link_to_add and blueprint functionality #216

tiagoblackcode opened this issue Nov 7, 2012 · 2 comments
Labels

Comments

@tiagoblackcode
Copy link

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

@lest
Copy link
Collaborator

lest commented Nov 13, 2012

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.

@stsc3000
Copy link

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

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

No branches or pull requests

3 participants