-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Multiple scatter plot fixes #956
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
Conversation
- by moving the line data.join out of the hasLines block
- by moving the marker and text data.joins out of the hasMarkers / hasText switch board
- now works as intended for <text> nodes
- so that <text> are properly translate on range relayout calls
|
||
join.each(function(d) { | ||
var sel = d3.select(this).select('text'); | ||
Drawing.translatePoint(d, sel, xa, ya); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A small thing but should or could we enable transitions here? I can imagine it being a matter of just following the fancy d3 transitions-work-like-everything-else pattern that the points follow. I left this out previously (though it had other update problems.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That will be for a future minor version bump (ref #933 (comment))
|
||
function assertScatterModeSizes(lineSize, pointSize, textSize) { | ||
var gd3 = d3.select(gd), | ||
lines = gd3.selectAll('g.scatter.trace .js-line'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Yeah. I was debating adding a bunch of d3 tests since technically it's testing internal behavior that's subject to change without affecting the look of the plots. But realistically, I think this is best.
I say 💃🏻 |
fixes #952
but also I noticed that
Plotly.restyle(gd, 'mode', '')
is broken onmaster
for scatter traces (since PR #802). I think I got all the cases fixed on this branch.@rreusser