Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error on node with 2 children #15

Open
tgillet1 opened this issue Apr 8, 2012 · 6 comments · May be fixed by #24
Open

Error on node with 2 children #15

tgillet1 opened this issue Apr 8, 2012 · 6 comments · May be fixed by #24
Labels
bug help wanted
Milestone
Backlog

Comments

@tgillet1
Copy link

tgillet1 commented Apr 8, 2012

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
Copy link

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
Copy link

mrgcohen commented Sep 7, 2012

works great so far. thanks

@fedossov
Copy link

Thank you very much!

@colinchan
Copy link

Thanks, this really helped!

@Floppy Floppy linked a pull request Apr 23, 2013 that will close this issue
Open
@Floppy
Copy link

Floppy commented Apr 23, 2013

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).

@Floppy Floppy mentioned this issue Apr 24, 2013
Open
@crates
Copy link

crates commented Jun 30, 2014

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;

@pwalsh pwalsh added the bug label Dec 15, 2015
@pwalsh pwalsh added this to the Backlog milestone Dec 15, 2015
tonghuikang pushed a commit to tonghuikang/bubbletree that referenced this issue Oct 27, 2019
color and fix rufuspollock-okfn#15 with #224
05743c6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug help wanted
Projects
None yet
Milestone
Backlog
Development

Successfully merging a pull request may close this issue.

Handle undefined left and right nodes for small trees
8 participants
@Floppy @pwalsh @mrgcohen @crates @tgillet1 @PhilippFS @fedossov @colinchan