Hi
I was investigating whether plotly.js could switch back to using d3-sankey from this repository instead of their fork (plotly/plotly.js#7689). When testing this version, I found that there was one problematic test case related to handling of large padding values: https://github.com/plotly/plotly.js/pull/7696/changes#r2715057996
It appears that this was supposed to have been fixed in 0fa75d8, but this fix was different than the one applied in the plotly fork (plotly@f61523d) and suggested in #63. The fix in plotly has a factor of 2/3 applied when computing the maximum padding, to ensure there is some vertical space left for the nodes, but this factor is missing from the fix in this repository, so the node heights can be reduced to zero.
Was that the intended behaviour or should this be updated to use the same approach as in plotly?
Hi
I was investigating whether plotly.js could switch back to using d3-sankey from this repository instead of their fork (plotly/plotly.js#7689). When testing this version, I found that there was one problematic test case related to handling of large padding values: https://github.com/plotly/plotly.js/pull/7696/changes#r2715057996
It appears that this was supposed to have been fixed in 0fa75d8, but this fix was different than the one applied in the plotly fork (plotly@f61523d) and suggested in #63. The fix in plotly has a factor of 2/3 applied when computing the maximum padding, to ensure there is some vertical space left for the nodes, but this factor is missing from the fix in this repository, so the node heights can be reduced to zero.
Was that the intended behaviour or should this be updated to use the same approach as in plotly?