Closed Arnedeklerk closed 1 year ago
I didn't check if what follows is the problem, but independently on that, this is a bad way to pass targets to click handlers, when it isn't necessary:
$(".viewGeneNetwork").bind("click", { x: tableData }, function (e) {
e.preventDefault();
var geneNum = $(e.target).attr("id").replace("viewGeneNetwork_", "");
var values = e.data.x[geneNum];
// Generate Network in KnetMaps.
generateCyJSNetwork(api_url + '/network', { list: [values[1]], keyword: keyword, isExportPlainJSON: false }, false);
});
Here, the click handler just needs the gene accession, so it could receive just that, instead of having to mess-up with row indices put at the end of element IDs. In fact, I suspect that the filter breaks this by creating a mismatch between the indices that are rendered and those that should be used with the original table data (e.data
).
Moreover, it seems that the only way we have to open the network view is generateCyJSNetwork()
, which is another bad thing, for it forces the invoker to use the API URL and format the request parameters. The invoker doesn't want to know about the API, for it would like to have a better level of abstraction, like:
function openNetworkView ( { keywords = '', genes = [], chrRegions = [] } )
{
// if all of the parameters are empty => fail
// if genes not empty && genes is a string and not an array => genes = [ genes ]
// prepare params for generateCyJSNetwork() and call it
}
This could be used in general, and in the case at issue, it could be used as a click handler, in the form: openNetworkView ( { genes: 'AT1G16980' } )
. The loop that attaches such invocations to the HTML table rows has the gene accession at hand, so it's easy to place it in the call string. I'm proposing to use the named parameters pattern, so that optional parameters can be omitted from the call.
Happy, closing
Surprised we haven't caught this earlier, of course a good reason for improved testing in 6.0.
Steps to recreate:
@lawal-olaotan please make this very high priority to fix