From d223e47846a0609bab3fb76e5778c0b5c6bfe024 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Fri, 21 Apr 2017 10:36:46 -0400 Subject: [PATCH 1/5] Inverse-transform text on layout transition --- src/plots/cartesian/transition_axes.js | 33 +++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/plots/cartesian/transition_axes.js b/src/plots/cartesian/transition_axes.js index b41c50b8cc3..6e1013d3d39 100644 --- a/src/plots/cartesian/transition_axes.js +++ b/src/plots/cartesian/transition_axes.js @@ -17,6 +17,8 @@ var Drawing = require('../../components/drawing'); var Axes = require('./axes'); var axisRegex = /((x|y)([2-9]|[1-9][0-9]+)?)axis$/; +var LAST_TRANSLATION_RE = /translate\([^)]*\)\s*$/; + module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCompleteCallback) { var fullLayout = gd._fullLayout; var axes = []; @@ -149,7 +151,15 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo // scale to individual points to counteract the scale of the trace // as a whole: .selectAll('.points').selectAll('.point') - .call(Drawing.setPointGroupScale, 1, 1); + .call(Drawing.setPointGroupScale, 1, 1) + + subplot.plot.selectAll('.points').selectAll('g') + .each(function () { + var el = d3.select(this); + var existingTransform = el.attr('transform').match(LAST_TRANSLATION_RE); + el.attr('transform', existingTransform || ''); + }); + } @@ -229,6 +239,27 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo .selectAll('.points').selectAll('.point') .call(Drawing.setPointGroupScale, 1 / xScaleFactor, 1 / yScaleFactor); + subplot.plot.selectAll('.points').selectAll('g') + .each(function () { + var el = d3.select(this); + var text = el.select('text'); + var x = parseFloat(text.attr('x')); + var y = parseFloat(text.attr('y')); + + var existingTransform = el.attr('transform').match(LAST_TRANSLATION_RE); + + var transforms = [ + 'translate(' + x + ',' + y + ')', + 'scale(' + (1 / xScaleFactor) + ',' + (1 / yScaleFactor) + ')', + 'translate(' + (-x) + ',' + (-y) + ')', + ]; + + if (existingTransform) { + transforms.push(existingTransform); + } + + el.attr('transform', transforms.join(' ')); + }); } var onComplete; From 0363db5ae678fe4bea1d14b3e520d401cfd78650 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 26 Apr 2017 13:31:07 -0400 Subject: [PATCH 2/5] Fix lint errors --- src/plots/cartesian/transition_axes.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/plots/cartesian/transition_axes.js b/src/plots/cartesian/transition_axes.js index 6e1013d3d39..35f072e4940 100644 --- a/src/plots/cartesian/transition_axes.js +++ b/src/plots/cartesian/transition_axes.js @@ -151,10 +151,10 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo // scale to individual points to counteract the scale of the trace // as a whole: .selectAll('.points').selectAll('.point') - .call(Drawing.setPointGroupScale, 1, 1) + .call(Drawing.setPointGroupScale, 1, 1); subplot.plot.selectAll('.points').selectAll('g') - .each(function () { + .each(function() { var el = d3.select(this); var existingTransform = el.attr('transform').match(LAST_TRANSLATION_RE); el.attr('transform', existingTransform || ''); @@ -240,7 +240,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo .call(Drawing.setPointGroupScale, 1 / xScaleFactor, 1 / yScaleFactor); subplot.plot.selectAll('.points').selectAll('g') - .each(function () { + .each(function() { var el = d3.select(this); var text = el.select('text'); var x = parseFloat(text.attr('x')); @@ -254,7 +254,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo 'translate(' + (-x) + ',' + (-y) + ')', ]; - if (existingTransform) { + if(existingTransform) { transforms.push(existingTransform); } From bf600bcdc2cccb0f97a368b07ecc86b5005f3a3c Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Tue, 2 May 2017 09:32:38 -0700 Subject: [PATCH 3/5] Move textpoint class to g and change transition axes selector --- src/plots/cartesian/transition_axes.js | 4 ++-- src/traces/scatter/plot.js | 3 +-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/plots/cartesian/transition_axes.js b/src/plots/cartesian/transition_axes.js index 35f072e4940..8aa593eaf85 100644 --- a/src/plots/cartesian/transition_axes.js +++ b/src/plots/cartesian/transition_axes.js @@ -153,7 +153,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo .selectAll('.points').selectAll('.point') .call(Drawing.setPointGroupScale, 1, 1); - subplot.plot.selectAll('.points').selectAll('g') + subplot.plot.selectAll('.points').selectAll('.textpoint') .each(function() { var el = d3.select(this); var existingTransform = el.attr('transform').match(LAST_TRANSLATION_RE); @@ -239,7 +239,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo .selectAll('.points').selectAll('.point') .call(Drawing.setPointGroupScale, 1 / xScaleFactor, 1 / yScaleFactor); - subplot.plot.selectAll('.points').selectAll('g') + subplot.plot.selectAll('.points').selectAll('.textpoint') .each(function() { var el = d3.select(this); var text = el.select('text'); diff --git a/src/traces/scatter/plot.js b/src/traces/scatter/plot.js index 57930c6e613..d01b2ddaf16 100644 --- a/src/traces/scatter/plot.js +++ b/src/traces/scatter/plot.js @@ -452,7 +452,7 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition // each text needs to go in its own 'g' in case // it gets converted to mathjax - join.enter().append('g').append('text'); + join.enter().append('g').classed('textpoint', true).append('text'); join.each(function(d) { var sel = transition(d3.select(this).select('text')); @@ -460,7 +460,6 @@ function plotOne(gd, idx, plotinfo, cdscatter, cdscatterAll, element, transition }); join.selectAll('text') - .classed('textpoint', true) .call(Drawing.textPointStyle, trace) .each(function(d) { From 82a68a06d773923d2d6fc609da9a826142b568e6 Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Tue, 2 May 2017 09:57:49 -0700 Subject: [PATCH 4/5] Refactor text point scaling --- src/components/drawing/index.js | 30 ++++++++++++++++++++++ src/plots/cartesian/dragbox.js | 5 +++- src/plots/cartesian/transition_axes.js | 35 +++----------------------- 3 files changed, 38 insertions(+), 32 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index af780f8dd50..8162833104c 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -692,6 +692,36 @@ drawing.setPointGroupScale = function(selection, x, y) { return scale; }; +var TEXT_POINT_LAST_TRANSLATION_RE = /translate\([^)]*\)\s*$/; + +drawing.setTextPointsScale = function(selection, xScale, yScale) { + selection.each(function() { + var transforms; + var el = d3.select(this); + var text = el.select('text'); + var x = parseFloat(text.attr('x')); + var y = parseFloat(text.attr('y')); + + var existingTransform = el.attr('transform').match(TEXT_POINT_LAST_TRANSLATION_RE); + + if(xScale === 1 && yScale === 1) { + transforms = []; + } else { + transforms = [ + 'translate(' + x + ',' + y + ')', + 'scale(' + xScale + ',' + yScale + ')', + 'translate(' + (-x) + ',' + (-y) + ')', + ]; + } + + if(existingTransform) { + transforms.push(existingTransform); + } + + el.attr('transform', transforms.join(' ')); + }); +}; + drawing.measureText = function(tester, text, font) { var dummyText = tester.append('text') .text(text) diff --git a/src/plots/cartesian/dragbox.js b/src/plots/cartesian/dragbox.js index 3c174224e60..df0d64511c9 100644 --- a/src/plots/cartesian/dragbox.js +++ b/src/plots/cartesian/dragbox.js @@ -30,7 +30,6 @@ var constants = require('./constants'); var MINDRAG = constants.MINDRAG; var MINZOOM = constants.MINZOOM; - // flag for showing "doubleclick to zoom out" only at the beginning var SHOWZOOMOUTTIP = true; @@ -747,6 +746,10 @@ module.exports = function dragBox(gd, plotinfo, x, y, w, h, ns, ew) { // as a whole: .select('.scatterlayer').selectAll('.points').selectAll('.point') .call(Drawing.setPointGroupScale, xScaleFactor2, yScaleFactor2); + + subplot.plot.select('.scatterlayer') + .selectAll('.points').selectAll('.textpoint') + .call(Drawing.setTextPointsScale, xScaleFactor2, yScaleFactor2); } } diff --git a/src/plots/cartesian/transition_axes.js b/src/plots/cartesian/transition_axes.js index 8aa593eaf85..44344a35132 100644 --- a/src/plots/cartesian/transition_axes.js +++ b/src/plots/cartesian/transition_axes.js @@ -17,8 +17,6 @@ var Drawing = require('../../components/drawing'); var Axes = require('./axes'); var axisRegex = /((x|y)([2-9]|[1-9][0-9]+)?)axis$/; -var LAST_TRANSLATION_RE = /translate\([^)]*\)\s*$/; - module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCompleteCallback) { var fullLayout = gd._fullLayout; var axes = []; @@ -150,17 +148,11 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo // This is specifically directed at scatter traces, applying an inverse // scale to individual points to counteract the scale of the trace // as a whole: - .selectAll('.points').selectAll('.point') + .select('.scatterlayer').selectAll('.points').selectAll('.point') .call(Drawing.setPointGroupScale, 1, 1); - subplot.plot.selectAll('.points').selectAll('.textpoint') - .each(function() { - var el = d3.select(this); - var existingTransform = el.attr('transform').match(LAST_TRANSLATION_RE); - el.attr('transform', existingTransform || ''); - }); - - + subplot.plot.select('.scatterlayer').selectAll('.points').selectAll('.textpoint') + .call(Drawing.setTextPointsScale, 1, 1); } function updateSubplot(subplot, progress) { @@ -240,26 +232,7 @@ module.exports = function transitionAxes(gd, newLayout, transitionOpts, makeOnCo .call(Drawing.setPointGroupScale, 1 / xScaleFactor, 1 / yScaleFactor); subplot.plot.selectAll('.points').selectAll('.textpoint') - .each(function() { - var el = d3.select(this); - var text = el.select('text'); - var x = parseFloat(text.attr('x')); - var y = parseFloat(text.attr('y')); - - var existingTransform = el.attr('transform').match(LAST_TRANSLATION_RE); - - var transforms = [ - 'translate(' + x + ',' + y + ')', - 'scale(' + (1 / xScaleFactor) + ',' + (1 / yScaleFactor) + ')', - 'translate(' + (-x) + ',' + (-y) + ')', - ]; - - if(existingTransform) { - transforms.push(existingTransform); - } - - el.attr('transform', transforms.join(' ')); - }); + .call(Drawing.setTextPointsScale, 1 / xScaleFactor, 1 / yScaleFactor); } var onComplete; From c7cdc8b0dd5ec12913e34b5e1174c8824360781f Mon Sep 17 00:00:00 2001 From: Ricky Reusser Date: Wed, 3 May 2017 15:55:12 -0700 Subject: [PATCH 5/5] Add test for setTextPointsScale --- src/components/drawing/index.js | 6 +++--- test/jasmine/tests/drawing_test.js | 34 ++++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/src/components/drawing/index.js b/src/components/drawing/index.js index 8162833104c..2e281c145f2 100644 --- a/src/components/drawing/index.js +++ b/src/components/drawing/index.js @@ -699,10 +699,10 @@ drawing.setTextPointsScale = function(selection, xScale, yScale) { var transforms; var el = d3.select(this); var text = el.select('text'); - var x = parseFloat(text.attr('x')); - var y = parseFloat(text.attr('y')); + var x = parseFloat(text.attr('x') || 0); + var y = parseFloat(text.attr('y') || 0); - var existingTransform = el.attr('transform').match(TEXT_POINT_LAST_TRANSLATION_RE); + var existingTransform = (el.attr('transform') || '').match(TEXT_POINT_LAST_TRANSLATION_RE); if(xScale === 1 && yScale === 1) { transforms = []; diff --git a/test/jasmine/tests/drawing_test.js b/test/jasmine/tests/drawing_test.js index 3c20e719a1a..9b6f81b4a07 100644 --- a/test/jasmine/tests/drawing_test.js +++ b/test/jasmine/tests/drawing_test.js @@ -308,4 +308,38 @@ describe('Drawing', function() { expect(el.getAttribute('transform')).toBe('translate(1,2)'); }); }); + + describe('setTextPointsScale', function() { + var svg, g, text; + + beforeEach(function() { + svg = d3.select(document.createElement('svg')); + g = svg.append('g'); + text = g.append('text'); + }); + + it('sets the transform on an empty element', function() { + Drawing.setTextPointsScale(g, 2, 3); + expect(g.attr('transform')).toEqual('translate(0,0) scale(2,3) translate(0,0)'); + }); + + it('unsets the transform', function() { + Drawing.setTextPointsScale(g, 1, 1); + expect(g.attr('transform')).toEqual(''); + }); + + it('preserves a leading translate', function() { + Drawing.setTextPointsScale(g, 1, 1); + g.attr('transform', 'translate(1, 2)'); + expect(g.attr('transform')).toEqual('translate(1, 2)'); + }); + + it('preserves transforms', function() { + text.attr('x', 8); + text.attr('y', 9); + g.attr('transform', 'translate(1, 2)'); + Drawing.setTextPointsScale(g, 4, 5); + expect(g.attr('transform')).toEqual('translate(8,9) scale(4,5) translate(-8,-9) translate(1, 2)'); + }); + }); });