Skip to content
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

the filter & server mode now play nicely with the colReorder extension #532

Merged
merged 8 commits into from Apr 25, 2018
2 changes: 1 addition & 1 deletion DESCRIPTION
@@ -1,7 +1,7 @@
Package: DT
Type: Package
Title: A Wrapper of the JavaScript Library 'DataTables'
Version: 0.4.8
Version: 0.4.9
Authors@R: c(
person("Yihui", "Xie", email = "xie@yihui.name", role = c("aut", "cre")),
person("Joe", "Cheng", role = "ctb"),
Expand Down
2 changes: 2 additions & 0 deletions NEWS.md
Expand Up @@ -14,6 +14,8 @@

- The server-side processing mode now supports data with nested `data.frame`s in its columns (thanks, @shrektan, #530 #525).

- The `colReorder` extention now works with the column filters and the server-side processing mode (thanks @shrektan, #532 #531 #160).

## NEW FEATURES

- The filters of `Date` or `Datetime` columns now display the same format and timezone as the column content if `formatDate()` is applied on these columns (thanks, @shrektan, #522 #241).
Expand Down
32 changes: 26 additions & 6 deletions inst/htmlwidgets/datatables.js
Expand Up @@ -257,15 +257,28 @@ HTMLWidgets.widget({
options.ajax.dataSrc = function(json) {
DT_rows_all = $.makeArray(json.DT_rows_all);
DT_rows_current = $.makeArray(json.DT_rows_current);
return json.data;
var data = json.data;
if (!colReorderEnabled()) return data;
var table = $table.DataTable(), order = table.colReorder.order(), flag = true, i, j, row;
for (i = 0; i < order.length; ++i) if (order[i] !== i) flag = false;
if (flag) return data;
for (i = 0; i < data.length; ++i) {
row = data[i].slice();
for (j = 0; j < order.length; ++j) data[i][j] = row[order[j]];
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if there is a way to reorder the data on the server side. If there is, the implementation could be much easier (data[order] in R instead of a nested loop in JS). Of course, it depends on whether we have the order information on the server side.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the datatables library doesn't send the reordered column info to the server by default. We need to register a callback in the event column-reorder, which sends the info to the R side. If this method is better, I guess it's not very difficult to make the change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, let's just forget it. I'm not too concerned about the performance of the nested loops here since data should typically be small enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the concern too... For the deep copy and nested loops... but as you said, it should be fine in the most cases.

return data;
};
}

var thiz = this;
if (instance.fillContainer) $table.on('init.dt', function(e) {
thiz.fillAvailableHeight(el, $(el).innerHeight());
});

// If the page contains serveral datatables and one of which enables colReorder,
// the table.colReorder.order() function will exist but throws error when called.
// So it seems like the only way to know if colReorder is enabled or not is to
// check the options.
var colReorderEnabled = function() { return "colReorder" in options; };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it have to be a function? I mean, does this work?

var colReorderEnabled = "colReorder" in options;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I used the non-function version at first and worked fine...

However, I overthinked that the options may get changed somewhere else after the table being initialized... It's OK to change it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. It should not matter much. I'm fine to use a function here.

var table = $table.DataTable(options);
$el.data('datatable', table);

Expand Down Expand Up @@ -613,22 +626,29 @@ HTMLWidgets.widget({
if (typeof filter === 'undefined' || !$td.data('filter')) return true;

var r = filter.val(), v, r0, r1;
var i_data = function(i) {
if (!colReorderEnabled()) return i;
var order = table.colReorder.order(), k;
for (k = 0; k < order.length; ++k) if (order[k] === i) return k;
return i; // in theory it will never be here...
}
v = data[i_data(i)];
if (type === 'number' || type === 'integer') {
v = parseFloat(data[i]);
v = parseFloat(v);
// how to handle NaN? currently exclude these rows
if (isNaN(v)) return(false);
r0 = parseFloat(scaleBack(r[0], scale))
r1 = parseFloat(scaleBack(r[1], scale));
if (v >= r0 && v <= r1) return true;
} else if (type === 'date' || type === 'time') {
v = new Date(data[i]);
v = new Date(v);
r0 = new Date(r[0] / scale); r1 = new Date(r[1] / scale);
if (v >= r0 && v <= r1) return true;
} else if (type === 'factor') {
if (r.length === 0 || inArray(data[i], r)) return true;
if (r.length === 0 || inArray(v, r)) return true;
} else if (type === 'logical') {
if (r.length === 0) return true;
if (inArray(data[i] === '' ? 'na' : data[i], r)) return true;
if (inArray(v === '' ? 'na' : v, r)) return true;
}
return false;
};
Expand Down