#828 breaks maintaining selected columns upon re-render

[epruesse]

DT/inst/htmlwidgets/datatables.js

Lines 995 to 1001 in 06d3ae6

	var cleanSelectedValues = function() {
	changeInput('rows_selected', []);
	changeInput('columns_selected', []);
	changeInput('cells_selected', transposeArray2D([]), 'shiny.matrix');
	}
	// #828 we should clean the selection on the server-side when the table reloads
	cleanSelectedValues();

The above code was added in response to #828, where the server side was not cleared when the table is reloaded and the Select extension is in use. However, this code doesn't sync, it wipes. So if user code, e.g. via callback, attempts to retain the row selections made by the user, only updating other parts of the table, the server side gets out of sync again. And since this piece of code is executed after the callbacks, it overrides any Shiny.setInputValue() that might be used to correct the server side.

Example:

DT::datatable(
  callback = DT::JS(
    paste0("var rows = [", rows_str, "];"),
    paste0("var reactvar = '", id, "_rows_selected';"),
    "table.rows(rows).select();",
    "Shiny.setInputValue(reactvar, rows);"
  )
)

The above sets the selected rows according to the numbers in row_str, e.g. 0,1,2, then would send the same back to Shiny (yes, probably off by one due to R's 1-indexing). However, the message is overwritten by the mentioned code.

I'd suggest removing the cleanSelectedValues() call entirely, and instead calling the updateSelectedRows() family of functions defined directly below. That way, R get's a copy of the actual status.

If anyone has a suggestion for a workaround, I'm all ears... Perhaps having a hook towards the end of the renderValue function would be helpful, to be able to override it when it does something other than necessary in a specific case.

[yihui]

Since it's been too long and the original code was written by another author who is no longer active here, I'm not sure what we should do about it. Is this issue more or less the same as #918? If so, I feel it makes sense to make the behavior optional via a user-provided option. Unfortunately I'm not familiar with this part of code and have to rely on someone else's judgment/pull request... Thanks for the report anyway!

[epruesse]

One other simple option would be to add a way to execute JS at the very end of the renderValue function. Preferably with full access to the variables inside that function. That way, client code can just adjust things. May require client apps to be adjusted more with DT updates, but allows for more extensive meddling when needed.

[epruesse]

@yihui See my minimal fix in PR #1126. This repairs it for me. It also should break anything for anyone. It merely sends the actual state if on client side table with Select extension, making it so the _xxx_selected variables actually match the table state. I don't know enough about the code and other combinations to dare to change this more broadly.