WSTreeView - confusing loader mechanism

The code responsible for loading node data is confusing, although it works well. The image below maps out the primary functions.

Left side list public functions, right side list internal functions.

While this may seem simple when we have the drawing, it's not simple when just looking at the code, and the real issue here is that these functions (except SelectAll) accept callbacks which makes things more complicated as they are passed around, but wrapped in new anonymous callbacks.

function getData(node, cb)
function recursivelyLoadAllNodes(targetNode, cbComplete, cbProgress, isSubCall)
function loadNodeData(node, cb)
this.Reload = function(keepSelections, cb)
this.EnsureData = function(cb)
this.SelectAll = function(selected, targetNode)

For instance both EnsureData, Reload, and SelectAll defines a callback that ends up being invoked by getData's callback. It's difficult to figure out when code is being executed, why and where. We also have to make sure that all callbacks containing similar logic gets updated when we fix or improve things.

In an ideal scenario we would need just one getData function that can be called by OnToggle, EnsureData, SelectAll, and Reload. Something like:

function getData(node, recursively, clearSelection, completedCallback, progressCallback)

This new version of getData can both load root nodes, load hierarchies under a specific node recursively, or load just the immediate children of a given node, and it provide the necessary callbacks.

// Load root nodes (initial request for data)
getData(null, false, false, null, null);

// Load specific node, e.g. OnToggle:
getData(toggledNode, false, false, null, null)

// Reload data and wipe control value
getData(toggledNode, false, true, null, null)

// Ensure data (load all nodes)
getData(null, true, false, null, null)

// Select all nodes
getData(null, true, false, function() { /* all nodes loaded - select them now */ }, null)

Support for chain loading of nodes (progressive mode) may complicate things.

Furthermore getData may need to expose additional flags to trigger different behaviour depending on the use case - for instance the purpose of the original callback function in getData was to provide different logic for populating the TreeView.

TODO: Create a simple table to outline the behaviour in the different functions. Use it to identify the common denominators so we know exactly what "modes" the new getData(..) function needs.

Jemt / Fit.UI

WSTreeView - confusing loader mechanism #83