1

I have a plunk that demonstrates my issue: http://plnkr.co/edit/PzBrcTX0Vnn01xWy4dk6

This is a table that has a list of "settings". It uses Footable so the list can be sorted and other features of Footable.

Scenario 1: run, push the remove setting button for one or more rows. Notice that the row is removed after the buttons is pushed. This is the expected behavior.

Scenario 2: run, click on the setting column header and ensure that the column is sorted, push the remove button. Notice that the row is not removed from the view.

If you put a break point in ApplicationSettings.js:

var removeItem = function (item) {
    items.remove(item);
};

You can see that in both scenarios the observable array is removing the items as expected, but in scenario 2 the view is not updated as expected.

Jeff
  • 2,728
  • 3
  • 24
  • 41
  • The sort function creates a new array that is not bound to the view. If you need more help, you really need to reduce the amount of code. – Damien Jan 02 '14 at 23:13

1 Answers1

3

The issue comes from the empty text nodes that surround each <tr> element. The foreach binding also tracks those text nodes. See https://github.com/knockout/knockout/pull/709 for a discussion about why it's not possible to just ignore them as a general rule. On the other hand, your custom binding can strip them out.

See how knockout-sortable does this (for similar reasons):

var nodes = Array.prototype.slice.call(element.childNodes, 0);
ko.utils.arrayForEach(nodes, function(node) {
    if (node && node.nodeType !== 1) {
        node.parentNode.removeChild(node);
    }
});

You need to make sure this is run before foreach. I modified your binding to do that: http://plnkr.co/edit/hS6Gb2xLfabSj9K8l03y?p=preview

Michael Best
  • 16,623
  • 1
  • 37
  • 70
  • I added the code from knockout-sortable, but that didn't fix the issues. I tried the hack mentioned here: http://stackoverflow.com/questions/2628050/ignore-whitespace-in-html. Then I removed all of the spaces from the so it's all on one row. This actually fixed the problem. If I push a single space between the and the first it breaks again. So, I either didn't implement the code from knockout-sortable correctly or the code is called after the foreach binding. I'm guessing it's the later. I'll see what I can do about that in the morning and report back... – Jeff Jan 03 '14 at 00:57
  • 1
    I modified your binding to do it before `foreach`. See my edits above. – Michael Best Jan 03 '14 at 05:42
  • The code I posted didn't work for every case. I've updated it now (and the Plunker). – Michael Best Jan 08 '14 at 23:24
  • Wow! What's a case that failed the previous version and how did you go about finding the failure? – Jeff Jan 09 '14 at 17:05
  • I noticed a recent [Knockout-sortable issue](https://github.com/rniemeyer/knockout-sortable/issues/81) about a problem related to the code. My update is slightly different since I didn't use jQuery. – Michael Best Jan 09 '14 at 20:57