Functions getNextNode() and getPreviousNode() operate differently based on whether node is open or closed

[rob-stoecklein]

Hi Marco,

I'm having an issue with the getNextNode() and getPreviousNode() functions: They operate differently depending on whether the next (or previous) node is open or closed, which is causing us a problem.

As an example, take the following simple tree:

Category-1
	Topic-1a
	Topic-1b
Category-2
	Topic-2a
	Topic-2b
Category-3

If the current node is "Topic-1b", and we call {topic-1b-node}.getNextNode(), then "Category-2" is returned, as expected.

We then take that "Category-2" code and call {category-2-node}.getNextNode(), it returns "Topic-2a", also as expected.

However, if the Category-2 node is closed, then the code operates differently:

If the current node is "Topic-1b", and we call {topic-1b-node}.getNextNode(), then "Category-2" is returned, as expected.

But, when we then take that "Category-2" code and call {category-2-node}.getNextNode(), it returns "Category-3", which is unexpected. Shouldn't it still return "Topic-2a"?

I looked through the code and found this:

public getNextNode(includeChildren = true): Node | null {
        if (includeChildren && this.hasChildren() && this.is_open) {
            // First child
            return this.children[0];
        } else {
            if (!this.parent) {
                return null;
            } else {
                const nextSibling = this.getNextSibling();
                if (nextSibling) {
                    // Next sibling
                    return nextSibling;
                } else {
                    // Next node of parent
                    return this.parent.getNextNode(false);
                }
            }
        }
    }

It is checking the this.is_open property.

Is this as intended?

If so, can we add a flag indicating that we'd like to get the next node, even if it's not currently open?

Thanks,
Rob

[rob-stoecklein]

PS: I should have said earlier, but I believe we have a work-around in our code. Regardless, we're interested in whether this is the desired operation or not. Thanks again!

[mbraak]

The getNextNode and getPreviousNode functions work like this by design. The getNextNode function does the same thing as pressing the 'down' key. So, if a parent node is closed, it skips the child nodes.

Maybe I should rename these functions to getNextVisibleNode and getPreviousVisibleNode.

[rob-stoecklein]

Those function names are definitely clearer. I would suggest doing that (although it might break someone's code).

You might want to add a note to this effect in the documentation, that they only work on open nodes.

And it would be nice to get methods that get next/previous whether the nodes are open or not.

Thanks!

[mbraak]

Version 1.7.0 is just published:

• The getPreviousNode and getNextNode now work as expected.
• The original functions are renamed to getPreviousVisibleNode and getNextVisibleNode.