cloudflarearchive / backgrid

Finally, an easily stylable semantic HTML data grid widget with a Javascript API that doesn't suck.
http://backgridjs.com
MIT License
2.01k stars 325 forks source link

ClientSideFilter alters collection #536

Open vlady88 opened 10 years ago

vlady88 commented 10 years ago

Hi there,

Great job with Backgrid! I'm using it to edit some tables in my database and it's doing the job very well. However, recently I discovered a strange behavior. I'm using a ClientSideFilter and I noticed that if I call sync() on my collection to save the data in the database while there is some filter applied to the collection, only the filtered data is saved. Is this the normal behavior? I think it's quite dangerous, as it may lead to data loss.

Thanks, Vladimir

amanpatel commented 9 years ago

I agree with this. From an outside API standpoint, I think backgrid components should not change the given collection.

For a suggested current workaround though, you can always access the grid components "collection.fullCollection".

rockerest commented 9 years ago

Another issue that I believe is related to this: Using ClientSideFilter breaks listeners bound to the collection when the filtered results are empty.

I'm not 100% clear on what exactly is happening here, so I'm willing to be corrected, but here's the behavior I'm seeing:

I have a listener on the collection that handles clicks on a row. I filter the collection to have no rows. Instead it shows the "EmptyRow" row. Then, I remove the filters, and all of the rows come back. However, the click event no longer works on any row. This behavior only happens when the table is completely emptied, if it only filters to one or two rows, all events are still fired.

This only appears to occur on backbone.paginator collections. Regular Backbone collections maintain the events properly.

To catch the edge case of a user filtering to no results, I have to listen for events on the internal structure of Backgrid AND on the original collection. I really don't like this, because it means my view is tightly coupled to A) Backgrid, and B) how Backgrid works internally. Also, I have twice the event binding because I have to handle the "initial" state (the table has never been emptied: listen to original collection) and the later state (the table has been emptied: listen to collection.fullCollection).