Uh oh!
There was an error while loading. Please reload this page.
Non-scaling SVG scatter points - #762
Conversation
| 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
| .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.
etpinard
commented
Jul 21, 2016
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) |
etpinard
commented
Jul 21, 2016
No. I don't see the need. |
Added comment, changed the name to be more descriptive ( |
5d0a2e1 to
96deefeCompare96deefe to
f786b4fCompareetpinard
commented
Jul 21, 2016
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