Non-scaling SVG scatter points #762
Conversation
| @@ -484,6 +484,27 @@ lib.setScale = function(element, x, y) { | |||
| return transform; | |||
| }; | |||
|
|
|||
| lib.setPointScale = function(selection, x, y) { | |||
There was a problem hiding this comment.
This is scatter-specific. It's here since related functions are right next to it, but it probably should not in lib/. I just don't think there's a world in which this can be meaningfully generalized.
There was a problem hiding this comment.
I'm ok with this being in lib/. Putting this in src/traces/scatter/set_point_scale.js would be ok too. Your call @rreusser
| @@ -633,7 +633,10 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { | |||
|
|
|||
| subplot.plot | |||
| .call(Lib.setTranslate, plotDx, plotDy) | |||
| .call(Lib.setScale, xScaleFactor, yScaleFactor); | |||
| .call(Lib.setScale, xScaleFactor, yScaleFactor) | |||
| .selectAll('.points').selectAll('.point') | |||
There was a problem hiding this comment.
a one-line comment here would be nice.
|
Nice solution. Looks like isolating the @rreusser Would you mind adding a few unit tests for |
|
@etpinard Should (Specifically, there's a danger of it being a bit expensive when lots of zoom events are happening since it applies to all points, so it's not totally general and abstract) |
No. I don't see the need. |
|
Added comment, changed the name to be more descriptive ( |
rreusser
force-pushed
the
non-scaling-points
branch
from
5d0a2e1 to
96deefe
Compare
rreusser
force-pushed
the
non-scaling-points
branch
from
96deefe to
f786b4f
Compare
|
Very nicely done. 💃 merge away. |
See issue: #760
See related line scaling PR: #761
See codepen example (visible on scrollZoom): http://codepen.io/rsreusser/pen/OXvmjR?editors=0010
This PR compensates for point scaling by applying an inverse scale transform. It's simple, but kind of an ugly hack since it requires stripping any existing scale from points before applying a new scale. That may make it prohibitive, but it was a bit easier than expected, so maybe it's not unreasonable. cc @mdtusz