D3 Negative Dists #3126
D3 Negative Dists #3126
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Ignored Deployment
|
| @@ -579,7 +601,9 @@ export const DistributionsChart: FC<DistributionsChartProps> = ({ | |||
| height={height} | |||
| samplesBarSetting={samplesState} | |||
| showCursorLine={nonTitleHeight > 30} | |||
| showPercentileLines={nonTitleHeight > 30} | |||
| showPercentileLines={ | |||
| !anyAreNonnormalized && nonTitleHeight > 30 | |||
There was a problem hiding this comment.
If any were nonnormalized, the percentileLines wouldn't make sense
| @@ -341,6 +348,20 @@ const InnerDistributionsChart: FC<{ | |||
| frame.exit(); | |||
| } | |||
|
|
|||
| drawAxes({ | |||
There was a problem hiding this comment.
This has to be at the end, to make sure that the axis is on top of the plot
| @@ -242,14 +249,14 @@ const InnerDistributionsChart: FC<{ | |||
|
|
|||
| // continuous fill | |||
| //In the case of one distribution, we don't want it to be transparent, so that we can show the samples lines. In the case of multiple distributions, we want them to be transparent so that we can see the other distributions. | |||
| context.fillStyle = isMulti ? getColor(i, 0) : getColor(i, 0.7); | |||
| context.globalAlpha = isMulti ? 0.4 : 1; | |||
| context.fillStyle = isMulti ? getColor(i, 0.5) : getColor(i, 0.7); | |||
| @@ -227,6 +232,8 @@ const InnerDistributionsChart: FC<{ | |||
| context.restore(); | |||
| } | |||
|
|
|||
| context.globalCompositeOperation = "source-over"; | |||
| @@ -160,7 +162,12 @@ const InnerDistributionsChart: FC<{ | |||
| ]); | |||
|
|
|||
| const yScale = sqScaleToD3(plot.yScale); | |||
| yScale.domain([0, Math.max(...domain.map((p) => p.y))]); | |||
| // yScale.domain([0, Math.max(...domain.map((p) => p.y))]); | |||
| if (yAxisTitle) { | ||
| padding.left = padding.left + 35; | ||
| } | ||
| const padding: Padding = calculatePadding({ |
There was a problem hiding this comment.
todo: Discuss the best way to refactor/organize this
There was a problem hiding this comment.
Kudos on fixing this, the output looks much better.
The implementation is making me nervous: there are a few implicit assumptions that you make (calculatePadding being consistent) that look like they hold right now but might break in the future.
I haven't verified it but I think if someone (1) enables y axis; (2) uses big numbers on y scale that would cause long ticks, then the chart would break.
I'll spend 30-60min on reorganize the code, and if it proves too hard then I'll add a few comments and then merge.
Before:
After:
Now it shows the full negative values.
Closes #3122