-
Notifications
You must be signed in to change notification settings - Fork 40
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use itemSelector in internalCount initialization #58
base: master
Are you sure you want to change the base?
Conversation
Please add a test case for this change, otherwise looks good. |
@merk done and witnessed the test turning green from red as I reapplied the internalCount initialization fix :) |
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. |
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 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. |
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) |
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. 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? |
Fixes a bug with an incorrect internalCount when the html does not have collection items as direct children of the collection, i.e: