infinite-networks / InfiniteFormBundle

A collection of useful form types and extensions for Symfony.
169 stars 40 forks source link

Use itemSelector in internalCount initialization #58

Open net02 opened 8 years ago

net02 commented 8 years ago

Fixes a bug with an incorrect internalCount when the html does not have collection items as direct children of the collection, i.e:

<table data-form-widget="collection">
    <thead>...</thead>
    <tbody class="items">
        <tr class="item">...</tr>
        <tr class="item">...</tr>
    </tbody>
</table>
merk commented 8 years ago

Please add a test case for this change, otherwise looks good.

net02 commented 8 years ago

@merk done and witnessed the test turning green from red as I reapplied the internalCount initialization fix :)

merk commented 8 years ago

Talking about this change in our office we realise that it is going to affect nested collections, which we dont seem to have any tests for in the JS tests. We'll have to fix that.

In the mean time, you can work around this issue by setting the data-form-widget on the tbody instead of the table, which is what we do for form collections as table rows.

net02 commented 8 years ago

@merk you were absolutely right, having a nested collection would have messed up the internalCount badly..

I fiddled up with jQuery selectors a bit and came up with the current setup which keeps track of the level of nesting (with the same item selector) and excludes nested collections' items. There's also a new test for it :)

merk commented 8 years ago

The issue here is if someone wanted to customise the itemSelector for nested collections. I'm not sure theres going to be a neat solution to the issue unless we set some kind of requirement that the item selector must be exact for nested collections like '> tbody > .item' (and I have no idea if jquery will support such a query in a .find() call)

net02 commented 8 years ago

Either I am missing something in the big picture or you missed the latest test which actually has a different itemSelector on one level of the "collection tree".

Having a different selector actually simplifies things because there wouldn't be any "false positives" in the internalCount .find() of the parent collection. This being the case, the .not() still prevents any item of an N-th level child collection, with the same selector as one of the parents, being included.

The nestedSelector loop thingy keeps the .not() from excluding true positives.. it's really an how-many-levels-of-nesting-with-the-same-selector-am-i-in count, which works for both "every nested collection has ss the same selector", "every nested collection has its own selector" scenarios, and also "mixed selectors at some level of nesting" (like the test case) or "someone put the whole collection under one or more .item parents which we really don't care".

Could you please show me what I am missing, maybe with a red test?