You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Dec 12, 2021. It is now read-only.
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
The text was updated successfully, but these errors were encountered:
Thanks for suggestions. Here is my opinion about them:
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
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.
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 thef.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 liketable.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 theinsertFields
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 thelink_to_add
is placed in something other than the nested fields container. Something likef.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 thescript
tag and keeps the template in a nice, clean container without anydata-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
The text was updated successfully, but these errors were encountered: