Refactor

[worthlutz]

Hi @jakezatecky ,

This pull request is a little refactoring to possibly speed things up and also set up for the addition of "radio groups". If you can accept this pull request, I will not need to fork your component and will submit my pull request for the addition of "radio groups" to this component as well.

Note: the diff is easier to look at if you look at the 0b219ff commit before the unused functions were removed.

I'm thinking one speed increase will come from not having to call getFormattedNodes in render. The information added to the nodes in that function can be done in flattenNodes where the calculated constant and default values can be saved for each node.

Canceling out most of the speed savings above will be that now renderTreeNodes loops through the entire set of nodes like getFormattedNodes did. This is because all child nodes must be touched to calculate checkstate.

But because the child nodes are processed first in renderTreeNodes, the function getCheckState can be changed to getShallowCheckState which only needs to look at one level of children where before it had to go the the bottom of the nested children each time it was called. This should lead to a net speed increase.

I've done no speed tests to verify any of the above analysis.

Let me know if you have any questions or see any problems with this pull request.

Worth

[worthlutz]

Failed check is Issue #95.

I do not know how to fix that issue. EDIT - fixed by pull request #98 merged below

[jakezatecky]

This may have been used by some people. I personally do not use this, but to maintain backwards compatibility, I would like for the same information to be available.

[worthlutz]

That makes perfect sense for compatibility. I'll add it back.

EDIT: removed incorrect statement.

[worthlutz]

I answered too quick. This object being passed to the this.props.onclick function sent from CheckTree is not really a node even though it is referred to as one in the CheckTree.onClick method. This caused me considerable confusion at first :) thus the comments I added in various functions to keep track. I removed the children property because it was not necessary in changes being made for the Radio Groups #99 . There the original node was to be returned. This change should have been made here in the Refactor request to make this clear.

The original method from CheckTree:

  onCheck(node) {
        // node is object from TreeNode
        const { noCascade, onCheck } = this.props;

        this.toggleChecked(node, node.checked, noCascade);
        onCheck(this.serializeList('checked'), node);
    }

Here are the relevant parts of this function from CheckTree as used in the Radio Groups pull request.

onCheck(nodeInfo) {
        // nodeInfo is object from TreeNode
        const { value, checked } = nodeInfo;

        const flatNode = this.flatNodes[value];
        const { parent, self } = flatNode;

        let { noCascade } = this.props;

        this.toggleChecked(self, checked, noCascade);
        this.props.onCheck(this.serializeList('checked'), self);
    }

So what gets sent out of CheckTree is the original node (self) sent into CheckTree and not the object sent from TreeNode. Doing it this way, the children are already available without the need to add them in in TreeNode.

What may be missing is the value checked which is not in self but could be added for backwards compatibility.

Looking further, onClick should probably be handled the same way.

What do you think?