Skip to content

Update zero line logic for sploms #2938

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

Closed
alexcjohnson opened this issue Aug 22, 2018 · 0 comments · Fixed by #3015
Closed

Update zero line logic for sploms #2938

alexcjohnson opened this issue Aug 22, 2018 · 0 comments · Fixed by #3015
Assignees
Labels
bug something broken

Comments

@alexcjohnson
Copy link
Collaborator

#2936 fixes zero line logic for regular cartesian plots, but large sploms do their own thing for performance. As @etpinard points out, this should be udpated too:

No need this do in this PR (opening a new issue would be fine), but we'll need to propagate these changes to large-sploms' regl-based grid line logic:

// just like in Axes.doTicks but without the loop over traces
function showZeroLine(ax) {
var rng = Lib.simpleMap(ax.range, ax.r2l);
var p0 = ax.l2p(0);
return (
ax.zeroline &&
ax._vals && ax._vals.length &&
(rng[0] * rng[1] <= 0) &&
(ax.type === 'linear' || ax.type === '-') &&
((p0 > 1 && p0 < ax._length - 1) || !ax.showline)
);
}

where I didn't just reuse the axes.js logic because I wanted to avoid looping over the traces (to compute that hasBarsOrFill boolean). Thinking about this again, looping over traces isn't really a performance hit for sploms (I can't really imagine a user wanting to plot more 10 splom traces on the same graph).

So it would be worth trying to expose and ♻️ this logic here for large-sploms.

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

Successfully merging a pull request may close this issue.

2 participants