rufuspollock-okfn / bubbletree

Radial Bubble Tree Visualization
http://okfnlabs.org/bubbletree
152 stars 69 forks source link

Error on node with 2 children #15

Open tgillet1 opened 12 years ago

tgillet1 commented 12 years ago

Perhaps I misunderstood something in how to create an appropriate structure, but I found that when I had just 2 children for a node that an error occurred on line 523 of bubbletree.js. I found that on line 181 if node.right == node.left (when there are two nodes at the level), node.right is set to undefined. Then on line 523 node.right.amount is accessed but node.right is undefined, causing the error. I simply included a node.right==undefined check around the statement calculating rad2 and removed the node.right.amount component of the Math.max call in that case. My test case worked after making that change.

PhilippFS commented 11 years ago

For those of you without programming skills, here's the solution in code. Thanks a lot, tgillet1!

            if (node.right==undefined) {
                rad2 = 0 - Math.max(
                    //hw *0.8 - tgtScale * (a2rad(node.parent.amount)+a2rad(node.amount)), // maximum visible part
                    hw * 0.8 - tgtScale * (a2rad(node.parent.amount) + a2rad(Math.max(node.amount*1.15 + node.maxChildAmount*1.15, node.left.amount * 0.85))),
                    tgtScale*a2rad(node.parent.amount)*-1 + hw*0.15 // minimum visible part
                ) + hw;
            }
            else {
            rad2 = 0 - Math.max(
                    //hw *0.8 - tgtScale * (a2rad(node.parent.amount)+a2rad(node.amount)), // maximum visible part
                    hw * 0.8 - tgtScale * (a2rad(node.parent.amount) + a2rad(Math.max(node.amount*1.15 + node.maxChildAmount*1.15, node.left.amount * 0.85, node.right.amount * 0.85))),
                    tgtScale*a2rad(node.parent.amount)*-1 + hw*0.15 // minimum visible part
                ) + hw;
            }
mrgcohen commented 11 years ago

works great so far. thanks

fedossov commented 11 years ago

Thank you very much!

colinchan commented 11 years ago

Thanks, this really helped!

Floppy commented 11 years ago

I just ran into this same problem, and also spent a while thinking I was doing it wrong! I've done a slightly different fix that handles both undefined left and right nodes, and opened a PR. Works for me (TM).

crates commented 10 years ago

Floppy's fix is a little more elegant: less lines of code, and works for more use cases. I'll add it here for reference.

    rad2 = 0 - Math.max(
      hw * 0.8 - tgtScale * (a2rad(node.parent.amount) + a2rad(Math.max(node.amount*1.15 + node.maxChildAmount*1.15, (node.left ? node.left.amount : 0) * 0.85, (node.right ? node.right.amount : 0)  * 0.85))),
      tgtScale*a2rad(node.parent.amount)*-1 + hw*0.15 // minimum visible part
    ) + hw;