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.

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

sourceLinks y0 and y1 calculated the wrong way round #92

Open
james-mckenzie-cko opened this issue May 21, 2021 · 1 comment
Open

sourceLinks y0 and y1 calculated the wrong way round #92

james-mckenzie-cko opened this issue May 21, 2021 · 1 comment

Comments

@james-mckenzie-cko
Copy link

I've discovered what appears to be a bug in how the y positions are calculated in the sourceLinks here:

d3-sankey/src/sankey.js

Lines 39 to 52 in bb7233e

function computeLinkBreadths({nodes}) {
for (const node of nodes) {
let y0 = node.y0;
let y1 = y0;
for (const link of node.sourceLinks) {
link.y0 = y0 + link.width / 2;
y0 += link.width;
}
for (const link of node.targetLinks) {
link.y1 = y1 + link.width / 2;
y1 += link.width;
}
}
}

As far as I can tell, this should update y1 rather than y0. These are referring to the top and bottom of the source link, so as it stands, y0 and y1 end up being the wrong way round.

I'd be grateful if someone could confirm that is the case before I submit a PR

@jayaddison
Copy link

@james-mckenzie-cko After a few reads through of the code snippet you linked, my initial reaction (based on vague but not confident memory of the codebase from the past) was: "the origin is in the top-left" -- but then that seemed unintuitive, and so I had to go back and search to confirm why that could be the case. It looks like for SVG-format graphics, the origin is indeed top-left (rather than bottom-left as in many diagram tools).

Might that explain the confusion here? What happens if you switch these around and render the results?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants