I think the main difference between your code and mine is I use the foreach in a preprocess and you create a new instance of the foreach. I'm not savy enough with Knockout to say if one is better than the other. I think the main difference is when the foreach actually processes the data. With the preproccess, I think the foreach does it''s processing before the init, and update of my binding is called. If we can figure out the issues bellow, I'll do some research to see if one way is better than the other.
I found two issues with your code. They seem to be the same or similar issues I was having with my code I posted yesterday.
1- When I try to remove an item that appears when the table is first rendered, both rows are removed. I think this is because DataTables will remove rows from the table that it doesn't know about when draw() is called.
2- When I add a row and then try to remove it, the item is removed from the observableArray, but not the table. I think this is because item[0].nodeName.toLowerCase() === 'tr' is never true. I think this is the same issue I ran into yesterday with my code.
If I can get past these two issues, I think this would be a really good bindingHandler. I would still need to pass in other DataTables options and make sure they work too.
I think the bindingHandler you posted today is more "Knockout-esk" so it would be really cool if we could figure out these few issues...
I think this is because DataTables will remove rows from the table that it doesn't know about when draw() is called.
Correct. Rows are read from the DOM at initialisation time, and thereafter must be added via the API. Any rows which DataTables does not know about are removed completely.
2- When I add a row and then try to remove it, the item is removed from the observableArray, but not the table. I think this is because item[0].nodeName.toLowerCase() === 'tr' is never true. I think this is the same issue I ran into yesterday with my code.
Interesting. I don't recall having that problem, but it has been a little while since I tried that code I must admit!
Would you be willing to post up a test case on http://live.datatables.net showing the issues? If you don't have time to do so I'll try to do so myself in the next week or two.
ko.bindingHandlers.DataTablesForEach = {
init: function (element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) {
var nodes = Array.prototype.slice.call(element.childNodes, 0);
ko.utils.arrayForEach(nodes, function (node) {
if (node && node.nodeType !== 1) {
node.parentNode.removeChild(node);
}
});
return ko.bindingHandlers.foreach.init(element, valueAccessor, allBindingsAccessor, viewModel, bindingContext);
},
update: function (element, valueAccessor, allBindingsAccessor, viewModel, bindingContext) {
var value = ko.unwrap(valueAccessor()),
key = "DataTablesForEach_Initialized";
var newValue = function () {
return {
data: value.data || value,
beforeRenderAll: function (el, index, data) {
if (ko.utils.domData.get(element, key)) {
$(element).closest('table').DataTable().destroy();
}
},
afterRenderAll: function (el, index, data) {
$(element).closest('table').DataTable(value.options);
}
};
};
ko.bindingHandlers.foreach.update(element, newValue, allBindingsAccessor, viewModel, bindingContext);
//if we have not previously marked this as initialized and there is currently items in the array, then cache on the element that it has been initialized
if (!ko.utils.domData.get(element, key) && (value.data || value.length)) {
ko.utils.domData.set(element, key, true);
}
return { controlsDescendantBindings: true };
}
};
Zach's solution looks very promising, but it's not in a released version of KO yet. As I was playing with my example again, I noticed var nodes = Array.prototype.slice.call(element.childNodes, 0); in Zach's and that made me remember I was having an issue with that when I was doing something similar to FooTable. (http://stackoverflow.com/questions/20893516/item-removed-from-knockout-observable-array-but-not-from-html-table) So I added the logic and that improved things. So now I have a working example. The only down side is you need to use destroy() every time you add or remove an item. If dt.row.add().draw() and dt.row().remove().draw() worked with out having to use destroy() that would be the best solution.
Here's my most recent binding:
function getTable(elem) {
return $(elem).closest("table");
}
function initDataTable(table) {
if (table)
return table.DataTable();
}
function getDataTable(elem) {
return initDataTable(getTable(elem));
}
function addRow(elem) {
var dt = getDataTable(elem);
if (dt){
dt.row.add(elem);
dt.destroy();
getDataTable(elem);
}
}
function removeRow(elem) {
var table = getTable(elem);
var dt = initDataTable(table);
if (dt) {
dt.row(elem).remove();
dt.destroy();
initDataTable(table);
}
}
ko.bindingHandlers.removeNonElements = {
init: function(element) {
var nodes = Array.prototype.slice.call(element.childNodes, 0);
ko.utils.arrayForEach(nodes, function(node) {
if (node && node.nodeType !== 1) {
node.parentNode.removeChild(node);
}
});
}
};
ko.bindingHandlers.datatable = {
afterRender: function(elements, data) {
if (elements.length === 0) return;
elements.forEach(function(elem) {
if (elem.nodeName && elem.nodeName.toLowerCase() === 'tr') {
addRow(elem);
}
})
},
afterAdd: function(elem, index, data) {
if (elem.nodeName && elem.nodeName.toLowerCase() === 'tr') {
addRow(elem);
}
},
beforeRemove: function(elem, index, data) {
if (elem.nodeName && elem.nodeName.toLowerCase() === 'tr') {
removeRow(elem);
}
},
preprocess: function(value, name, addBindingCallback) {
addBindingCallback("removeNonElements", "true");
addBindingCallback("foreach", "{ data:" + value + ", afterRender: ko.bindingHandlers.datatable.afterRender, afterAdd: ko.bindingHandlers.datatable.afterAdd, beforeRemove: ko.bindingHandlers.datatable.beforeRemove }");
return value;
},
init: function(element, valueAccessor, allBindings, viewModel, bindingContext) {
//FYI: This is empty now, but will be used to add other DataTable
//configurations once the basic add and remover are working.
}
};
As you can see I used foreach in the preprocess instead of overriding it. I don't know if one way is better than the other. I suspect that I may change once I start adding more DataTables options.
@jswenson , @allan Have a look at this... I have a binding that does not require the changes to the knockout source... although those changes are coming. :)
Replies
I came up with a
dt-foreach
binding a little while back that might be of interest, and is very similar to your own:It is however quite different from the original code I wrote for this post which used observables in their raw form.
Anyway, if anyone has any suggestions on how to improve the above, it would be very welcome.
Allan
Hey Allan, Here's a plunk that uses that code: http://plnkr.co/edit/IzrjqszeNYdwlsLYBWyY
I think the main difference between your code and mine is I use the foreach in a preprocess and you create a new instance of the foreach. I'm not savy enough with Knockout to say if one is better than the other. I think the main difference is when the foreach actually processes the data. With the preproccess, I think the foreach does it''s processing before the init, and update of my binding is called. If we can figure out the issues bellow, I'll do some research to see if one way is better than the other.
I found two issues with your code. They seem to be the same or similar issues I was having with my code I posted yesterday.
1- When I try to remove an item that appears when the table is first rendered, both rows are removed. I think this is because DataTables will remove rows from the table that it doesn't know about when draw() is called.
2- When I add a row and then try to remove it, the item is removed from the observableArray, but not the table. I think this is because item[0].nodeName.toLowerCase() === 'tr' is never true. I think this is the same issue I ran into yesterday with my code.
If I can get past these two issues, I think this would be a really good bindingHandler. I would still need to pass in other DataTables options and make sure they work too.
I think the bindingHandler you posted today is more "Knockout-esk" so it would be really cool if we could figure out these few issues...
Correct. Rows are read from the DOM at initialisation time, and thereafter must be added via the API. Any rows which DataTables does not know about are removed completely.
Interesting. I don't recall having that problem, but it has been a little while since I tried that code I must admit!
Would you be willing to post up a test case on http://live.datatables.net showing the issues? If you don't have time to do so I'll try to do so myself in the next week or two.
Allan
Sure, I'll put an example there. In the mean time, here's an example on plunk: http://plnkr.co/edit/IzrjqszeNYdwlsLYBWyY.
Here's one on live.datatables.net: http://live.datatables.net/siburehu/1
This is how I did it:
http://jsfiddle.net/zachpainter77/z8us2aej/
Zach, this looks promising. I'm trying to find more information on beforeRenderAll and afterRenderAll. What version of KO are they in?
Zach's solution looks very promising, but it's not in a released version of KO yet. As I was playing with my example again, I noticed
var nodes = Array.prototype.slice.call(element.childNodes, 0);
in Zach's and that made me remember I was having an issue with that when I was doing something similar to FooTable. (http://stackoverflow.com/questions/20893516/item-removed-from-knockout-observable-array-but-not-from-html-table) So I added the logic and that improved things. So now I have a working example. The only down side is you need to usedestroy()
every time you add or remove an item. Ifdt.row.add().draw()
anddt.row().remove().draw()
worked with out having to usedestroy()
that would be the best solution.Here's my most recent binding:
http://plnkr.co/2IU0ToKIow6nOfX7TUSB
As you can see I used
foreach
in thepreprocess
instead of overriding it. I don't know if one way is better than the other. I suspect that I may change once I start adding more DataTables options.Here's another binding where I override the
foreach
instead of using it in thepreprocess
http://plnkr.co/IzrjqszeNYdwlsLYBWyY?p=preview
@jswenson , @allan Have a look at this... I have a binding that does not require the changes to the knockout source... although those changes are coming. :)
https://www.datatables.net/forums/discussion/31797/knockout-js-3-4-custom-binding-for-jquery-datatables-net#latest