Fix sankey nodes clipped at bottom edge with user-positioned nodes

[jdonaldson]

Fixes #7946

Overview

Sankey nodes positioned via node.x / node.y near the bottom of the plot area render outside it and get clipped. Two independent paths in src/traces/sankey/render.js cause it; this PR fixes both. #7946 has runnable reproductions, screenshots and measured overflow figures.

1. "Force node position" centres the node on an unbounded point

trace.node.y positions the node's centre, so y near 1 puts up to half the node below the bottom edge, for every arrangement including fixed. Measured: a 135px node at y = 0.98 in a 280px plot area overflows by 61.9px.

Fix: clamp the centre with Lib.constrain(pos[1], nodeHeight / 2, height - nodeHeight / 2). Lib.constrain's swapped-bounds branch handles a node taller than the plot area by pinning it to the top edge (y0 = 0), which is better than letting it hang off the top.

2. resolveCollisionsTopToBottom walks columns off the bottom edge

With arrangement: "snap", overlapping nodes are only ever pushed down, and nothing checks the result against height. Measured: a three-node column requested at y = 0.80/0.86/0.92 in a 380px area ends up 114px and 244px past the bottom, with D never rendering — while the top 244px of the plot area sits empty.

Fix: add the missing counterpart. The bundled dependency already does this — @plotly/d3-sankey@0.7.2 (src/sankey.js:292-299) ends its resolveCollisions with a bottom-bounded upward pass and a top-bounded downward one:

resolveCollisionsBottomToTop(nodes, y1, nodes.length - 1, alpha);  // bottom bound
resolveCollisionsTopToBottom(nodes, y0, 0, alpha);                 // top bound

render.js declares a local function with the same name as the dep's downward helper but implements only that half and never calls a counterpart. This adds resolveCollisionsBottomToTop(columns), bounded by height, called after the downward pass.

It only moves a node when node.y1 exceeds the running bound, so nodes that already fit do not move and the correction propagates upward no further than it must. That is the reason to prefer it over shifting the whole column by the full overflow (what the first commit on this branch did): a whole-column shift moves the top node even when it had no need to move, and pushes a tight column off the top edge instead.

Not fixed, and I don't think it should be here: a column whose stacked height genuinely exceeds height still cannot fit, and its top node ends above y = 0. That is the same outcome the dep produces. A Lib.warn there might be worthwhile as a follow-up — L76-78 already warns on a related fit failure.

Behaviour change — worth calling out

This makes an explicitly requested node.y non-authoritative near the edges: asking for y = 1 now puts the node's bottom at the plot edge rather than its centre. That is the intent, but it is visible to anyone who hand-tuned node.y against the old clipped rendering. Alternatives if you'd rather not change the default: Lib.warn when a position is clamped, or gate it — though I'd argue drawing outside the plot area isn't behaviour worth preserving behind a flag. Happy to go whichever way you prefer.

Tests

Replaces the single test from the first commit with two, one per code path, so neither fix can mask the other:

• keeps an explicitly positioned node inside the plot area — arrangement: 'fixed', so no collision resolution runs and it covers the centring clamp alone.
• keeps a snapped column inside the plot area when collisions cascade — every y is ≤ 0.92, so the clamp cannot be what saves it; only the new pass can.

Both measure node rects against gd._fullLayout._size, which is what plot.js:137,371-372 passes to the renderer as its height.

The test in my first commit was a tautology and this replaces it. It compared node rects to document.querySelector('.sankey').getBoundingClientRect(). Node rects are descendants of that layer, and an SVG group's box is the union of its children, so sankeyRect.bottom is a max over children — necessarily ≥ any child's bottom, and likewise .top is a min. Both assertions held for every possible rendering, patched or not. I found this the hard way: a harness written that way reported "all nodes inside bounds" on a plot that was visibly clipped, with the layer box measuring 336.9px against a 280px plot area.

I also dropped test/image/mocks/sankey_x_y_bottom_clipping.json. It needed a baseline PNG (compare_pixels_test.mjs globs the mocks directory and fails on a missing baseline) which the jasmine test never used; the fixtures are now inline in the tests.

What I have and haven't run

• Verified: both reproductions measured against a real v3.7.0 render, and the two patched functions extracted from the committed source and exercised over the observed geometry plus edge cases (already-valid column unchanged, single overflowing node, column that cannot fit, empty column). The downward-only pass reproduces the browser-measured 244px overflow exactly, which is what gives me confidence the harness models the real thing.
• Not run: the jasmine suite and the image tests — I don't have a working local install for them, so CI here is their first real execution. If the new tests need adjusting I'll turn that around quickly. Flagging it rather than implying a green local run.

[camdecoster]

Thanks for the PR! Could you please open an issue to track the bug and provide steps to reproduce the problem? That will make it easier to review the PR.

[robertclaus]

@jdonaldson just following up if you are still interested in working on this, could you open an issue to track what this is intended to fix?

[jdonaldson]

Hey thanks for the attention, let me cook up a better example. I just jotted this down in the moment so I could track it.