nested_form icon indicating copy to clipboard operation
nested_form copied to clipboard

Make the fields positionable, before, after, ... a given node (fix Issue #66)

Open elmatou opened this issue 14 years ago • 17 comments

Hi (again) Ryan,

Here is my implementation of the positionable feature. Make the fields positionable, before, after, at the top (prepend) or at the bottom (append) of a given node by adding attributes to the "add" link.

  f.link_to_add("Add", :tasks, :node => 'table', :position => :before)
# OR
  f.link_to_add("Add", :tasks, :node => '.tasks', :position => :top)

Fully tested with jQuery (and in use), not tested with Prototype.

EDIT : the choosen node should be unique (or you will get several new set of fields with same id's) and outside of any .fields set (or you won't be able to remove the added fieldset)

elmatou avatar Aug 02 '11 17:08 elmatou

This seems to be working great on adding a link, but the link_to_remove helper doesn't seem to behave nicely. If I add multiple nodes, removing any of them deletes all of below that node rather than just the node itself. This is because the adding seems to happen inside the fields div in a nested fashion, so rather than having:

<div class="fields">
  <div id="node_1">
     <!-- fields for nested form 1 -->
  </div>
</div>
<div class="fields">
  <div id="node_2">
    <!-- fields for nested form 2 -->
  </div>
</div>

We're getting this:

<div class="fields">
  <div id="node_1">
     <!-- fields for nested form 1 -->
  </div>
  <div class="fields">
    <div id="node_2">
      <!-- fields for nested form 2 -->
    </div>
  </div>
</div>

Looks like the insert needs to be after the nodes enclosing div with class="fields" node (i.e. it's parent node), rather than the node itself

p7r avatar Aug 11 '11 10:08 p7r

Hi Paul, I didn't change the link_to_remove implementation, and it "should" work the same way, by finding the closest '.fields' selector (the first parent tag of the link with the class 'fields') and removing it (more precisely hiding it, and change the _destroy field value.)

The point with my positionable feature is to choose carefully the insertion node (please provide your code if you want any help). '.fields' selector represent all fields for one nested item, by this, I mean that you should never insert new fields inside '.fields'.

These won't work (and will probably behave like you describe it) :

f.link_to_add("Add", :tasks, :node => '.fields', :position => :top)
f.link_to_add("Add", :tasks, :node => '.fields', :position => :bottom)

But these will work :

f.link_to_add("Add", :tasks, :node => '.fields', :position => :after)
f.link_to_add("Add", :tasks, :node => '.fields', :position => :before)

Anyway you shouldn't use '.fields' selector as it can missed if there isn't any child object in your rails resource. Rather than '.fields' you should use a container ('div.nested_items', ...).

Last but not list, are you using Jquery or Prototype ? I hadn't a chance to test it extensivly with Prototype.

elmatou avatar Aug 11 '11 12:08 elmatou

I think you misunderstand - I'm adding :after the #node_1, #node_2, etc.

I changed nested_form.js to the following, and it worked just fine:

    switch(insert_position){
    case 'before':
      var field = $(content).insertBefore(insert_node.parent());
      break;
    case 'after':
      var field = $(content).insertAfter(insert_node.parent());
        break;
    case 'top':
        var field = $(content).prependTo(insert_node.parent());
        break;
    case 'bottom':
        var field = $(content).appendTo(insert_node.parent());
        break;
    default:
        var field = $(content).insertBefore(this.parent());
    }

p7r avatar Aug 11 '11 12:08 p7r

Hi, I think, I understood your issue. You shouldn't insert after, before, top or bottom of '#node1' or so, because these are inside your div with class 'fields'. Be aware that 'fields' class, represent only 1 (one) item of your nested resource.

as I wrote : '.fields' selector represent all fields for one nested item, by this, I mean that you should never insert new fields inside '.fields'. And that is precisly what you are doing by invoking :

f.link_to_add("Add", :tasks, :node => '#node1', :position => :after)

as '#node1' is the direct child of '.fields'

By changing the javascript you are only invoking :

f.link_to_add("Add", :tasks, :node => '.fields', :position => :after)

as '.fields' is the direct parent of '#node1'

BTW :

$(content).insertBefore(this.parent());

Won't work because this is not a jquery element.

I hope it is more clear.

elmatou avatar Aug 11 '11 13:08 elmatou

Everything seems to be working fine for me until I put a fields_for inside of another fields_for. If I click link_to_add on the inner fields_for it adds it to all instances of the outer because the node name is the same...

andrewajo avatar Aug 26 '11 03:08 andrewajo

I think I get your point.

Let's explain the way it insert fields :

In the master branch of nested_form, the fields position is related to the clicked link (in js this), which is a relative path with the link as root and before as position.

In my fork new fields position is directly related to :node => 'table' (or whatever your selector is), which should be an absolutepath (yes, like in your filesystem), because we want to be able to put the fields anywhere on the page.

You should choose a more presice and unique selector for the node (by adding an id or a class to the node), like : :node => 'table.nested_set'.

If the selector is not unique (as it is in your case), Jquery&Prototype will add the html (our fields here) to each matching DOM elements, resulting in your symptoms...

I hope it will fix your issue.

elmatou avatar Aug 26 '11 07:08 elmatou

Sorry I should have been more specific.

It is not an issue of having a unique node name - I have created a specific div with a unique class. Here is an example:

I am working on a database for a transportation company. So I have a nested_form for a Load. That load has many Pickup_Location, and each Pickup_Location has many Items.

When I add several Pickup_Locations it functions as it should.

The issue is that when I go to add Items to any one of these Pickup_Locations, because the Pickup_Locations that have been generated are identical, they all have the same node for Items.

This results in the new Item fields being added to each of the Pickups fields. Hopefully that is clearer?

The only thought I have had so far is to maybe identify which child it is in insert_node in the js file?

andrewajo avatar Aug 26 '11 14:08 andrewajo

Ok, I get it. I didn't use double nested fields, and I didn't treat this case... Of course the position node for the second level will be created several times... not good.

The path should be related to the clicked link (as it is in the release), but in this case we loose the ability to attach the fields anywhere in the DOM, but only as a child, parent or siblings.

More accuratly, for now, the insertion node is related to the parent form of the link.

insert_node = $(this).closest("form").find(insert_node);

Can you gist your resulting HTML please, I don't have an handy app with double nested resources ?

elmatou avatar Aug 26 '11 14:08 elmatou

Let me know if this is ok:

git clone git://gist.github.com/1173694.git gist-1173694

andrewajo avatar Aug 26 '11 15:08 andrewajo

Think I found a fix:

insert_node = $(this).closest("form").find(insert_node);
child = $(this).closest(".fields").index();

// var field = $(content).insertBefore(this); switch(insert_position){ case 'before': var field = $(content).insertBefore(insert_node[child]); break; case 'after': $(content).insertAfter(insert_node[child]); break; case 'top': var field = $(content).prependTo(insert_node[child]); break; case 'bottom': var field = $(content).appendTo(insert_node[child]); break; default: var field = $(content).insertBefore(this); }

haven't tested it yet...

andrewajo avatar Aug 26 '11 16:08 andrewajo

I'm on something similar

if ($(this).closest(".fields").length) {    insert_node = $(this).closest(".fields").find(insert_node) }
else { insert_node = $(this).closest("form").find(insert_node) };

But it fails if you put the add link in a ".fields"

EDIT : Are you sure your solution work with simple nesting ? and without the node option ?

elmatou avatar Aug 26 '11 16:08 elmatou

Will take a look at it and let you know - haven't had a chance to give it a thorough test...

andrewajo avatar Aug 26 '11 17:08 andrewajo

I have switched to this and it works with simple nesting and without the node option:

if (insert_node != this) {
    insert_node = $(this).closest("form").find(insert_node);
    child = $(this).closest(".fields").index();
    insert_node = insert_node[child];
}

// var field = $(content).insertBefore(this); switch(insert_position){ case 'before': var field = $(content).insertBefore(insert_node); break; case 'after': $(content).insertAfter(insert_node); break; case 'top': var field = $(content).prependTo(insert_node); break; case 'bottom': var field = $(content).appendTo(insert_node); break; default: var field = $(content).insertBefore(this); }

Still fails if the link is within a ".fields" though....

andrewajo avatar Aug 26 '11 18:08 andrewajo

I have solved this problem in a different way with my branch, but I think ryanb really need to merge fxposter's integration test branch before adding this kind of feature.

samwgoldman avatar Aug 27 '11 00:08 samwgoldman

Hi Sam, I agree with your point of view about testing. Can you point your solution (a link to a commit or file) ? I didn't found anything in your branch.

elmatou avatar Aug 28 '11 19:08 elmatou

Here's the comparison between our branches: https://github.com/ryanb/nested_form/compare/master...samwgoldman:master

I merged in fxposter's branch, so the diff is a little messy. I think more useful is an example. I am using it here to create nested forms within tables[1]. Look at the 'fields_tag' and 'collection_tag' arguments.

  1. https://github.com/samwgoldman/gradebook/blob/master/app/views/evaluations/_form.html.haml

samwgoldman avatar Aug 28 '11 22:08 samwgoldman

thx Sam, It make sens, even if it is not "fully" postionnable, it is right that added fields will join a kond of"collection".

Anyhow, I will wait for an operational test suite before refactoring this part.

elmatou avatar Aug 29 '11 07:08 elmatou