Idea for v2

[worthlutz]

@jakezatecky

I have an idea for a breaking change to CheckboxTree. I'll be working on this full time over the next couple of weeks. I'm not sure if you will be interested so I thought I'd describe it here and see what you think. This change is driven by my needs for a tree component to manage map layers in my application.

The problem I am having is that the node returned as a function parameter to onCheck is not completely valid. The checked array is not useful to me and I do not use it in my application. I do realize that others may use this array. In my application a check change will turn on/off layers in a map. When I get the changed node, I must go down to the leafs and make the appropriate layer visibility changes. Sometimes the parent is necessary in this processing also (radio groups, another enhancement I'll discuss later).

The tree configuration is more easily managed by a human in object form thus the nodes prop used in Checkboxtree. I am proposing to flatten the tree before passing it to CheckboxTree as a flatTree prop. This plus a root prop will be the props that control the component instead of nodes, checked and expanded. I propose adding the checked and expanded properties to the nodeShape as shown below.

const nodeShape = {
    label: PropTypes.node.isRequired,
    value: PropTypes.oneOfType([
        PropTypes.string,
        PropTypes.number,
    ]).isRequired,

    checked: PropTypes.bool,  // new
    disabled: PropTypes.bool,
    icon: PropTypes.node,
    showCheckbox: PropTypes.bool,
    title: PropTypes.string,
};

const nodeShapeWithChildren = PropTypes.oneOfType([
    PropTypes.shape(nodeShape),
    PropTypes.shape({
        ...nodeShape,
        children: PropTypes.arrayOf(nodeShape).isRequired,
        expanded: PropTypes.bool,  // new
    }),
]);

With these changes, the example from your README.md would look like this:

import { useState } from 'react';
import { flattenTree }, CheckboxTree from 'react-checkbox-tree';
import 'react-checkbox-tree/lib/react-checkbox-tree.css';

const nodes = [{
  value: 'mars',
  label: 'Mars',
  children: [
    { value: 'phobos', label: 'Phobos' },
    { value: 'deimos', label: 'Deimos' },
  ],
}];

const [ flattenedTree, root ] = flattenTree(nodes);

function Widget() {
  const [flatTree, setFlatTree] = useState(flattenedTree);
  const [root, setRoot] = useState(root);

  const processNode = (node) => {
    // process node with changed check here
    // node.parent and node.children are available as properties of node
    // In my application, the leaf nodes are map layers that get turned
    // on or off depending on the check status of the leaf
    // If a parent node check is changed, all children need processing
  }

  const checkChangeHandler = (node, updatedTree, root) => {
    // if checkedArray is needed ( I don't use this )
    const checkedArray = updatedTree.getChecked();

    processNode(node);  // node has the new changed checked value
    
    setTree(updatedTree);
    setRoot(root);
  }

  const expandChangeHandler = (node, updatedTree, root) => {
    // if expandedArray is needed ( I don't use this )
    const expandedArray = updatedTree.getExpanded();

    setTree(updatedTree);
    setRoot(root);
  }

  return (
    <CheckboxTree
      nodes={nodes}
      flatTree={flatTree}
      root={root}
      onCheck={checkChangeHandler}
      onExpand={expandChangeHandler}
    />
  );
}

Please let me know if you are interested in these changes and I'll can discuss it with you more.

[jakezatecky]

I am certainly open to making major changes to the API in v2.0. It is something I have been mulling about myself. The current iteration of checked and expanded presents several challenges when implementing certain features and bug fixes.

Having an alternative mechanism to fetch checked and expanded is quite acceptable. My original purpose for developing this library concerned itself with leaf nodes in a large hierarchical tree of airports and their parent cities, states, and countries.

I would like more details on the flatTree and root properties, and why use them as opposed to the nodes property maintaining state.

As an FYI, there is a branch, v2.0-dev, that has several more commits than the master branch. Depending on the proposals, I think it would be more appropriate to target that branch.

[worthlutz]

I've branched off v2.0-dev and started work. Would it work if I started a pull request and you follow along there giving comments and suggestions?

[jakezatecky]

Yes, that would be fine. My commentary may lag a day or two depending on my availability.

[worthlutz]

I would like more details on the flatTree and root properties, and why use them as opposed to the nodes property maintaining state.

The reasoning for this choice is that flatTree is easier to use programmatically as one. can access a node by it's key. Today I worked on modifying NodeModel into TreeModel and NodeModel. This now works well to create a flatTree to describe the tree. All the necessary control properties are added to the nodes if not already defined.

The code below shows the the example using the new models. I am not sure I like this. I think that maybe flatTree should be moved to state inside CheckboxTree. I'll think on this more.

The TreeModel has function properties which can be used to modify the TreeModel instance like expandAllNodes. For this reason, the instance can be exposed and modified programmatically. I hope to have some more details soon.

import { useEffect, useState } from 'react';
import { TreeModel }, CheckboxTree from 'react-checkbox-tree';
import 'react-checkbox-tree/lib/react-checkbox-tree.css';

const nodes = [{
  value: 'mars',, 
  label: 'Mars',
  children: [
    { value: 'phobos', label: 'Phobos' },
    { value: 'deimos', label: 'Deimos' },
  ],
}];

function Widget({nodes}) {
  const [tree, setTree] = useState([]);

  const processNode = (node) => {
    // process node with changed check here
    // node.parent and node.children are available as properties of node
    // In my application, the leaf nodes are map layers that get turned
    // on or off depending on the check status of the leaf
    // If a parent node check is changed, all children need processing
  }

  const checkChangeHandler = (node, updatedTree, root) => {
    // if checkedArray is needed ( I don't use this )
    const checkedArray = updatedTree.getChecked();

    processNode(node);  // node has the new changed checked value
    
    setTree(updatedTree);
    setRoot(root);
  }

  const expandChangeHandler = (node, updatedTree, root) => {
    // if expandedArray is needed ( I don't use this )
    const expandedArray = updatedTree.getExpanded();

    setTree(updatedTree);
    setRoot(root);
  }

  useEffect(() => {
    const newTree = new TreeModel(nodes);
    setTree(newTree);
  },[nodes]);

  return (
    <CheckboxTree
      nodes={flatTree}
      onCheck={checkChangeHandler}
      onExpand={expandChangeHandler}
    />
  );
}

[jakezatecky]

The reasoning for this choice is that flatTree is easier to use programmatically as one. can access a node by it's key
[...]
I think that maybe flatTree should be moved to state inside CheckboxTree. I'll think on this more.

I think that is fine for internal use, but I am unsure if this would have much utility for users. I would prefer to keep things simple for users and thus keep any additional complexities internal to the component.

---

I notice that the new code you provide does not use the root or tree properties. I understand the flatTree variable now, but still am unsure what root encapsulates.

[worthlutz]

I think that is fine for internal use, but I am unsure if this would have much utility for users. I would prefer to keep things simple for users and thus keep any additional complexities internal to the component.>

I agree with keeping it simple for the user. One reason for having nodes as initial state and converting to a TreeModel to maintain that state is that the tree returned to the user can have methods to get information from the tree such as the checked array.

Example:

onCheck(node, tree) {
  const checked = node.checked;
  const checkedArray = tree.getChecked();
  const rootNode = tree.getNode(tree.rootKey);  // walk this tree with the parent/children properties of each node.

  setTree(tree);  // save state to update CheckboxTree
}

When a parent node is checked in my application, I need to go to all leaf nodes below that parent and turn on/off the map layers defined in the leaf nodes. This may be a deep tree under a parent. Writing this makes me think that maybe there should be a getChecked in the NodeModel to simplify it for the user.

The root you were asking about above is the root node of the tree. I'm imagining the initial nodes variable being either a single nodeShape or an array of nodeShape. When passed to the constructor of TreeModel if nodes is an array, this array will be the children property of the root node which will get added during flattening.

Display of the root node, showRoot prop will default to false.

This discussion is very helpful for my thought process so keep asking questions. Please let me know if you have strong preferences as I can bend toward those. This is your component. :)

More discussion will follow in a new comment below.

[worthlutz]

I tried yesterday to make CheckboxTree an uncontrolled component and keep flatTree internal. I quickly determined for my application the CheckboxTree must be a controlled component. I must move the CheckboxTree state up above my tabs component or into a context to keep from losing the CheckboxTree state when the tab changes.

I've been debating using the flatTree definition of the tree or just use the original nodes prop to maintain state. I have a working fork of CheckboxTree off v1.6 using the original nodes prop for state and not using flatTree. I think using the flatTree may have some benefits.

When using the nodes as state, mutating a node in the object requires mutating all the parent nodes up to the root so the CheckboxTree notices the prop change. This is doable but may be more complicated. This method removes the flatTree completely. In my application this was not a problem as I only need access to the node returned by onCheck and can walk up the tree through the parent property or down through the children property. The nodes object must be carefully maintained.

Using the flatTree, it is easier to access a single node and to mutate a single node. You can still walk the tree if needed or you can run through the keys array to tabulate checked quickly. By converting the initial state, nodes, to the TreeModel and using that to store state, the tree can be made consistent during the flatten process adding missing properties with default values to nodes as needed. The parent node reference is added also.

Currently my TreeModel has this form. Root node can have it's own descriptive label and value.

{
  tree: {
    root_node: {
      label: 'My Tree',
      value: 'root_node',
      parent: null,
      children: [ 'node1', 'node2'  ]
    },
    node1: {
       label: 'node1',
       value: 'node1',
       children: [ ]
    }
    node2: {
       label: 'node2',
       value: 'node2',
       children: [ ]
    }
  }
 rootKey:  'root_node',
  ... various functions like getChecked(), getNode(value) 
}

onCheck passed to CheckboxTree would look something like this:

onCheck(node, tree) {
  const checked = node.checked;
  const checkedArray = tree.getChecked();
  const rootNode = tree.getNode(tree.rootKey);
  setTree(tree);  // to save state
}

I hope to have a working example soon.