From 9c2be8dfeaf3b2f946128a5983259772022c114e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Fri, 13 Jul 2018 16:03:33 -0400 Subject: [PATCH 01/15] pass dragbox element to mouseEvent ... this reduces the occurance of intermittent failure in the gl2d_click suite on etpinard's laptop --- test/jasmine/tests/gl2d_click_test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/jasmine/tests/gl2d_click_test.js b/test/jasmine/tests/gl2d_click_test.js index f5fb50e9018..84e9784d4d9 100644 --- a/test/jasmine/tests/gl2d_click_test.js +++ b/test/jasmine/tests/gl2d_click_test.js @@ -593,17 +593,19 @@ describe('@noCI @gl Test gl2d lasso/select:', function() { function drag(path) { var len = path.length; + var el = d3.select(gd).select('rect.nsewdrag').node(); + var opts = {element: el}; Lib.clearThrottle(); - mouseEvent('mousemove', path[0][0], path[0][1]); - mouseEvent('mousedown', path[0][0], path[0][1]); + mouseEvent('mousemove', path[0][0], path[0][1], opts); + mouseEvent('mousedown', path[0][0], path[0][1], opts); path.slice(1, len).forEach(function(pt) { Lib.clearThrottle(); - mouseEvent('mousemove', pt[0], pt[1]); + mouseEvent('mousemove', pt[0], pt[1], opts); }); - mouseEvent('mouseup', path[len - 1][0], path[len - 1][1]); + mouseEvent('mouseup', path[len - 1][0], path[len - 1][1], opts); } function select(path) { From c9711dc634367c17d4f52b8be60b33a6c1d144d7 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 14 May 2018 15:48:31 -0400 Subject: [PATCH 02/15] fixup "role" for ax autorange and rangemode --- src/plots/cartesian/layout_attributes.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index 33971a84d24..0ce0a654919 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -77,7 +77,7 @@ module.exports = { valType: 'enumerated', values: [true, false, 'reversed'], dflt: true, - role: 'style', + role: 'info', editType: 'calc', impliedEdits: {'range[0]': undefined, 'range[1]': undefined}, description: [ @@ -91,7 +91,7 @@ module.exports = { valType: 'enumerated', values: ['normal', 'tozero', 'nonnegative'], dflt: 'normal', - role: 'style', + role: 'info', editType: 'plot', description: [ 'If *normal*, the range is computed in relation to the extrema', From bd61c54d518bb4cb6f476902556e6ae42c623767 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 10:30:22 -0400 Subject: [PATCH 03/15] sub fail -> failTest --- test/jasmine/tests/scatter_test.js | 32 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 3f247532f81..9c19dc2d702 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -8,7 +8,7 @@ var Plotly = require('@lib/index'); var createGraphDiv = require('../assets/create_graph_div'); var destroyGraphDiv = require('../assets/destroy_graph_div'); var customAssertions = require('../assets/custom_assertions'); -var fail = require('../assets/fail_test'); +var failTest = require('../assets/fail_test'); var assertClip = customAssertions.assertClip; var assertNodeDisplay = customAssertions.assertNodeDisplay; @@ -561,7 +561,7 @@ describe('end-to-end scatter tests', function() { expect(d3.select(this).classed('plotly-customdata')).toBe(false); }); - }).catch(fail).then(done); + }).catch(failTest).then(done); }); it('adds "textpoint" class to scatter text points', function(done) { @@ -572,7 +572,7 @@ describe('end-to-end scatter tests', function() { text: ['a', 'b', 'c'] }]).then(function() { expect(Plotly.d3.selectAll('.textpoint').size()).toBe(3); - }).catch(fail).then(done); + }).catch(failTest).then(done); }); it('should remove all point and text nodes on blank data', function(done) { @@ -625,7 +625,7 @@ describe('end-to-end scatter tests', function() { assertNodeCnt(3, 3); assertText(['A', 'B', 'C']); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -683,7 +683,7 @@ describe('end-to-end scatter tests', function() { ['apple', 'banana', 'clementine'] ); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -727,7 +727,7 @@ describe('end-to-end scatter tests', function() { ['apple', 'banana', 'clementine'] ); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -763,7 +763,7 @@ describe('end-to-end scatter tests', function() { ['apple', 'banana', 'dragon fruit'] ); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -785,7 +785,7 @@ describe('end-to-end scatter tests', function() { ); }).then(function() { expect(fill()).toEqual('rgb(0, 0, 255)'); - }).catch(fail).then(done); + }).catch(failTest).then(done); }); it('clears fills tonext when either trace is emptied out', function(done) { @@ -828,7 +828,7 @@ describe('end-to-end scatter tests', function() { .then(function() { checkFill(false, 'null out both traces'); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -875,7 +875,7 @@ describe('end-to-end scatter tests', function() { [40, 30, 20] ); }) - .catch(fail) + .catch(failTest) .then(done); }); }); @@ -944,7 +944,7 @@ describe('scatter hoverPoints', function() { expect(pts[1].text).toEqual('banana', 'hover text'); expect(pts[2].text).toEqual('orange', 'hover text'); }) - .catch(fail) + .catch(failTest) .then(done); }); }); @@ -1073,7 +1073,7 @@ describe('Test Scatter.style', function() { 'selected pt 1 w/ set unselected.marker.opacity' ); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -1160,7 +1160,7 @@ describe('Test Scatter.style', function() { 'selected pts 0-2 w/ set selected.marker.color' ); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -1204,7 +1204,7 @@ describe('Test Scatter.style', function() { 'selected pt 0 w/ set unselected.marker.size' ); }) - .catch(fail) + .catch(failTest) .then(done); }); @@ -1287,7 +1287,7 @@ describe('Test Scatter.style', function() { 'selected pts 0-2 w/ set selected.textfont.color' ); }) - .catch(fail) + .catch(failTest) .then(done); }); }); @@ -1433,7 +1433,7 @@ describe('Test scatter *clipnaxis*:', function() { [true, 1] ); }) - .catch(fail) + .catch(failTest) .then(done); }); }); From b1911b3b9f9df542e76adfdba0246022d9a219d9 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 14 May 2018 15:48:00 -0400 Subject: [PATCH 04/15] run Axes.expand on `autorange: false` axes - this way `relayout(gd, 'ax.autorange', true)` does not have to go through costly editType "calc" - this makes the 'calcIfAutorange' edit type obsolete, which the following few commits will cope with --- src/plots/cartesian/autorange.js | 6 ------ src/plots/cartesian/layout_attributes.js | 2 +- test/jasmine/tests/axes_test.js | 13 +++++-------- test/jasmine/tests/gl2d_pointcloud_test.js | 11 ++++------- 4 files changed, 10 insertions(+), 22 deletions(-) diff --git a/src/plots/cartesian/autorange.js b/src/plots/cartesian/autorange.js index 117abcde849..63017327ee0 100644 --- a/src/plots/cartesian/autorange.js +++ b/src/plots/cartesian/autorange.js @@ -210,10 +210,6 @@ function doAutoRange(ax) { } } -function needsAutorange(ax) { - return ax.autorange || ax._rangesliderAutorange; -} - /* * expand: if autoranging, include new data in the outer limits for this axis. * Note that `expand` is called during `calc`, when we don't yet know the axis @@ -236,8 +232,6 @@ function needsAutorange(ax) { * and make it a tight bound if possible */ function expand(ax, data, options) { - if(!needsAutorange(ax) || !data) return; - if(!ax._min) ax._min = []; if(!ax._max) ax._max = []; if(!options) options = {}; diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index 0ce0a654919..3265be8f9ce 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -78,7 +78,7 @@ module.exports = { values: [true, false, 'reversed'], dflt: true, role: 'info', - editType: 'calc', + editType: 'plot', impliedEdits: {'range[0]': undefined, 'range[1]': undefined}, description: [ 'Determines whether or not the range of this axis is', diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index 7b6352a605e..30df3aedbfb 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -1858,15 +1858,12 @@ describe('Test axes', function() { expect(ax._max).toEqual([{val: 6, pad: 10, extrapad: true}]); }); - it('should return early if no data is given', function() { + it('should fail if no data is given', function() { ax = getDefaultAx(); - - expand(ax); - expect(ax._min).toBeUndefined(); - expect(ax._max).toBeUndefined(); + expect(function() { expand(ax); }).toThrow(); }); - it('should return early if `autorange` is falsy', function() { + it('should return even if `autorange` is false', function() { ax = getDefaultAx(); data = [2, 5]; @@ -1874,8 +1871,8 @@ describe('Test axes', function() { ax.rangeslider = { autorange: false }; expand(ax, data, {}); - expect(ax._min).toBeUndefined(); - expect(ax._max).toBeUndefined(); + expect(ax._min).toEqual([{val: 2, pad: 0, extrapad: false}]); + expect(ax._max).toEqual([{val: 5, pad: 0, extrapad: false}]); }); it('should consider range slider `autorange`', function() { diff --git a/test/jasmine/tests/gl2d_pointcloud_test.js b/test/jasmine/tests/gl2d_pointcloud_test.js index 44320b9606f..b43b7c02827 100644 --- a/test/jasmine/tests/gl2d_pointcloud_test.js +++ b/test/jasmine/tests/gl2d_pointcloud_test.js @@ -181,21 +181,18 @@ describe('@gl pointcloud traces', function() { return Plotly.relayout(gd, 'xaxis.range', [3, 6]); }).then(function() { - - expect(scene2d.xaxis._min).toEqual([]); - expect(scene2d.xaxis._max).toEqual([]); + expect(scene2d.xaxis._min).toEqual(xBaselineMins); + expect(scene2d.xaxis._max).toEqual(xBaselineMaxes); return Plotly.relayout(gd, 'xaxis.autorange', true); }).then(function() { - expect(scene2d.xaxis._min).toEqual(xBaselineMins); expect(scene2d.xaxis._max).toEqual(xBaselineMaxes); return Plotly.relayout(gd, 'yaxis.range', [8, 20]); }).then(function() { - - expect(scene2d.yaxis._min).toEqual([]); - expect(scene2d.yaxis._max).toEqual([]); + expect(scene2d.yaxis._min).toEqual(yBaselineMins); + expect(scene2d.yaxis._max).toEqual(yBaselineMaxes); return Plotly.relayout(gd, 'yaxis.autorange', true); }).then(function() { From 35d501d5acda8fa0179ba38fe656a35889bf0fb0 Mon Sep 17 00:00:00 2001 From: etienne Date: Mon, 28 May 2018 09:29:27 -0400 Subject: [PATCH 05/15] handle splom Axes.expand calls w/ undefined xa/ya --- src/plots/cartesian/autorange.js | 1 + src/traces/scatter/calc.js | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/plots/cartesian/autorange.js b/src/plots/cartesian/autorange.js index 63017327ee0..a009e1da986 100644 --- a/src/plots/cartesian/autorange.js +++ b/src/plots/cartesian/autorange.js @@ -232,6 +232,7 @@ function doAutoRange(ax) { * and make it a tight bound if possible */ function expand(ax, data, options) { + if(!ax._min) ax._min = []; if(!ax._max) ax._max = []; if(!options) options = {}; diff --git a/src/traces/scatter/calc.js b/src/traces/scatter/calc.js index 9362e27efd6..2ba49a0ab46 100644 --- a/src/traces/scatter/calc.js +++ b/src/traces/scatter/calc.js @@ -96,8 +96,9 @@ function calcAxisExpansion(gd, trace, xa, ya, x, y, ppad) { yOptions.padded = false; } - Axes.expand(xa, x, xOptions); - Axes.expand(ya, y, yOptions); + // N.B. asymmetric splom traces call this with undefined xa or ya + if(xa._id) Axes.expand(xa, x, xOptions); + if(ya._id) Axes.expand(ya, y, yOptions); } function calcMarkerSize(trace, serieslen) { From a8fb653ddca6dd00407b4362131ba2ceb3e6d511 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Mon, 16 Jul 2018 16:38:23 -0400 Subject: [PATCH 06/15] adapt and :lock: annotations to 'calcIfAutorange'-less edits! --- src/components/annotations/attributes.js | 44 ++++++++++---------- src/components/annotations/calc_autorange.js | 34 +++++---------- src/components/annotations/draw.js | 18 +++----- test/jasmine/tests/annotations_test.js | 37 ++++++++++++++++ 4 files changed, 75 insertions(+), 58 deletions(-) diff --git a/src/components/annotations/attributes.js b/src/components/annotations/attributes.js index be7189e2e31..26e1e7fb0be 100644 --- a/src/components/annotations/attributes.js +++ b/src/components/annotations/attributes.js @@ -19,7 +19,7 @@ module.exports = templatedArray('annotation', { valType: 'boolean', role: 'info', dflt: true, - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Determines whether or not this annotation is visible.' ].join(' ') @@ -28,7 +28,7 @@ module.exports = templatedArray('annotation', { text: { valType: 'string', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the text associated with this annotation.', 'Plotly uses a subset of HTML tags to do things like', @@ -41,14 +41,14 @@ module.exports = templatedArray('annotation', { valType: 'angle', dflt: 0, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the angle at which the `text` is drawn', 'with respect to the horizontal.' ].join(' ') }, font: fontAttrs({ - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', colorEditType: 'arraydraw', description: 'Sets the annotation text font.' }), @@ -57,7 +57,7 @@ module.exports = templatedArray('annotation', { min: 1, dflt: null, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets an explicit width for the text box. null (default) lets the', 'text set the box width. Wider text will be clipped.', @@ -69,7 +69,7 @@ module.exports = templatedArray('annotation', { min: 1, dflt: null, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets an explicit height for the text box. null (default) lets the', 'text set the box height. Taller text will be clipped.' @@ -130,7 +130,7 @@ module.exports = templatedArray('annotation', { min: 0, dflt: 1, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the padding (in px) between the `text`', 'and the enclosing border.' @@ -141,7 +141,7 @@ module.exports = templatedArray('annotation', { min: 0, dflt: 1, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the width (in px) of the border enclosing', 'the annotation `text`.' @@ -152,7 +152,7 @@ module.exports = templatedArray('annotation', { valType: 'boolean', dflt: true, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Determines whether or not the annotation is drawn with an arrow.', 'If *true*, `text` is placed near the arrow\'s tail.', @@ -197,7 +197,7 @@ module.exports = templatedArray('annotation', { min: 0.3, dflt: 1, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the size of the end annotation arrow head, relative to `arrowwidth`.', 'A value of 1 (default) gives a head about 3x as wide as the line.' @@ -208,7 +208,7 @@ module.exports = templatedArray('annotation', { min: 0.3, dflt: 1, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the size of the start annotation arrow head, relative to `arrowwidth`.', 'A value of 1 (default) gives a head about 3x as wide as the line.' @@ -218,7 +218,7 @@ module.exports = templatedArray('annotation', { valType: 'number', min: 0.1, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: 'Sets the width (in px) of annotation arrow line.' }, standoff: { @@ -226,7 +226,7 @@ module.exports = templatedArray('annotation', { min: 0, dflt: 0, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets a distance, in pixels, to move the end arrowhead away from the', 'position it is pointing at, for example to point at the edge of', @@ -240,7 +240,7 @@ module.exports = templatedArray('annotation', { min: 0, dflt: 0, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets a distance, in pixels, to move the start arrowhead away from the', 'position it is pointing at, for example to point at the edge of', @@ -252,7 +252,7 @@ module.exports = templatedArray('annotation', { ax: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the x component of the arrow tail about the arrow head.', 'If `axref` is `pixel`, a positive (negative) ', @@ -265,7 +265,7 @@ module.exports = templatedArray('annotation', { ay: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the y component of the arrow tail about the arrow head.', 'If `ayref` is `pixel`, a positive (negative) ', @@ -332,7 +332,7 @@ module.exports = templatedArray('annotation', { x: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the annotation\'s x position.', 'If the axis `type` is *log*, then you must take the', @@ -350,7 +350,7 @@ module.exports = templatedArray('annotation', { values: ['auto', 'left', 'center', 'right'], dflt: 'auto', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the text box\'s horizontal position anchor', 'This anchor binds the `x` position to the *left*, *center*', @@ -369,7 +369,7 @@ module.exports = templatedArray('annotation', { valType: 'number', dflt: 0, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Shifts the position of the whole annotation and arrow to the', 'right (positive) or left (negative) by this many pixels.' @@ -395,7 +395,7 @@ module.exports = templatedArray('annotation', { y: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the annotation\'s y position.', 'If the axis `type` is *log*, then you must take the', @@ -413,7 +413,7 @@ module.exports = templatedArray('annotation', { values: ['auto', 'top', 'middle', 'bottom'], dflt: 'auto', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the text box\'s vertical position anchor', 'This anchor binds the `y` position to the *top*, *middle*', @@ -432,7 +432,7 @@ module.exports = templatedArray('annotation', { valType: 'number', dflt: 0, role: 'style', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Shifts the position of the whole annotation and arrow up', '(positive) or down (negative) by this many pixels.' diff --git a/src/components/annotations/calc_autorange.js b/src/components/annotations/calc_autorange.js index 3db9ed91253..e87752bb1e7 100644 --- a/src/components/annotations/calc_autorange.js +++ b/src/components/annotations/calc_autorange.js @@ -16,25 +16,11 @@ var draw = require('./draw').draw; module.exports = function calcAutorange(gd) { - var fullLayout = gd._fullLayout, - annotationList = Lib.filterVisible(fullLayout.annotations); - - if(!annotationList.length || !gd._fullData.length) return; - - var annotationAxes = {}; - annotationList.forEach(function(ann) { - annotationAxes[ann.xref] = 1; - annotationAxes[ann.yref] = 1; - }); + var fullLayout = gd._fullLayout; + var annotationList = Lib.filterVisible(fullLayout.annotations); - for(var axId in annotationAxes) { - var ax = Axes.getFromId(gd, axId); - if(ax && ax.autorange) { - return Lib.syncOrAsync([ - draw, - annAutorange - ], gd); - } + if(annotationList.length && gd._fullData.length) { + return Lib.syncOrAsync([draw, annAutorange], gd); } }; @@ -46,14 +32,14 @@ function annAutorange(gd) { // use the arrow and the text bg rectangle, // as the whole anno may include hidden text in its bbox Lib.filterVisible(fullLayout.annotations).forEach(function(ann) { - var xa = Axes.getFromId(gd, ann.xref), - ya = Axes.getFromId(gd, ann.yref), - headSize = 3 * ann.arrowsize * ann.arrowwidth || 0, - startHeadSize = 3 * ann.startarrowsize * ann.arrowwidth || 0; + var xa = Axes.getFromId(gd, ann.xref); + var ya = Axes.getFromId(gd, ann.yref); + var headSize = 3 * ann.arrowsize * ann.arrowwidth || 0; + var startHeadSize = 3 * ann.startarrowsize * ann.arrowwidth || 0; var headPlus, headMinus, startHeadPlus, startHeadMinus; - if(xa && xa.autorange) { + if(xa) { headPlus = headSize + ann.xshift; headMinus = headSize - ann.xshift; startHeadPlus = startHeadSize + ann.xshift; @@ -81,7 +67,7 @@ function annAutorange(gd) { } } - if(ya && ya.autorange) { + if(ya) { headPlus = headSize - ann.yshift; headMinus = headSize + ann.yshift; startHeadPlus = startHeadSize - ann.yshift; diff --git a/src/components/annotations/draw.js b/src/components/annotations/draw.js index f3500145a01..a0b6d4ddbdc 100644 --- a/src/components/annotations/draw.js +++ b/src/components/annotations/draw.js @@ -300,25 +300,17 @@ function drawRaw(gd, options, index, subplotId, xa, ya) { * otherwise the text anchor point */ if(ax) { - /* - * hide the annotation if it's pointing outside the visible plot - * as long as the axis isn't autoranged - then we need to draw it - * anyway to get its bounding box. When we're dragging, an axis can - * still look autoranged even though it won't be when the drag finishes. - */ + // check if annotation is off screen, to bypass DOM manipulations var posFraction = ax.r2fraction(options[axLetter]); - if((gd._dragging || !ax.autorange) && (posFraction < 0 || posFraction > 1)) { + if(posFraction < 0 || posFraction > 1) { if(tailRef === axRef) { posFraction = ax.r2fraction(options['a' + axLetter]); if(posFraction < 0 || posFraction > 1) { annotationIsOffscreen = true; } - } - else { + } else { annotationIsOffscreen = true; } - - if(annotationIsOffscreen) continue; } basePx = ax._offset + ax.r2p(options[axLetter]); autoAlignFraction = 0.5; @@ -402,7 +394,9 @@ function drawRaw(gd, options, index, subplotId, xa, ya) { options['_' + axLetter + 'shift'] = textShift; } - if(annotationIsOffscreen) { + // We have everything we need for calcAutorange at this point, + // we can safely exit - unless we're currently dragging the plot + if(!gd._dragging && annotationIsOffscreen) { annTextGroupInner.remove(); return; } diff --git a/test/jasmine/tests/annotations_test.js b/test/jasmine/tests/annotations_test.js index 63109e4f94c..1039facbd04 100644 --- a/test/jasmine/tests/annotations_test.js +++ b/test/jasmine/tests/annotations_test.js @@ -737,6 +737,43 @@ describe('annotations autorange', function() { .catch(failTest) .then(done); }); + + it('should propagate axis autorange changes when axis ranges are set', function(done) { + function _assert(msg, xrng, yrng) { + var fullLayout = gd._fullLayout; + expect(fullLayout.xaxis.range).toBeCloseToArray(xrng, 1, msg + ' xrng'); + expect(fullLayout.yaxis.range).toBeCloseToArray(yrng, 1, msg + ' yrng'); + } + + Plotly.plot(gd, [{y: [1, 2]}], { + xaxis: {range: [0, 2]}, + yaxis: {range: [0, 2]}, + annotations: [{ + text: 'a', + x: 3, y: 3 + }] + }) + .then(function() { + _assert('set rng / small tx', [0, 2], [0, 2]); + return Plotly.relayout(gd, 'annotations[0].text', 'loooooooooooooooooooooooong'); + }) + .then(function() { + _assert('set rng / big tx', [0, 2], [0, 2]); + return Plotly.relayout(gd, { + 'xaxis.autorange': true, + 'yaxis.autorange': true + }); + }) + .then(function() { + _assert('auto rng / big tx', [-0.22, 3.57], [0.84, 3.365]); + return Plotly.relayout(gd, 'annotations[0].text', 'a'); + }) + .then(function() { + _assert('auto rng / small tx', [-0.18, 3.035], [0.84, 3.365]); + }) + .catch(failTest) + .then(done); + }); }); describe('annotation clicktoshow', function() { From 6e19c6ef6d2ea962cb10f8f3d571f13027f43987 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 17:46:31 -0400 Subject: [PATCH 07/15] remove unused '_' fields in annotations full items --- src/components/annotations/draw.js | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/components/annotations/draw.js b/src/components/annotations/draw.js index a0b6d4ddbdc..7669ab2a2da 100644 --- a/src/components/annotations/draw.js +++ b/src/components/annotations/draw.js @@ -250,11 +250,6 @@ function drawRaw(gd, options, index, subplotId, xa, ya) { var outerWidth = Math.round(annWidth + 2 * borderfull); var outerHeight = Math.round(annHeight + 2 * borderfull); - - // save size in the annotation object for use by autoscale - options._w = annWidth; - options._h = annHeight; - function shiftFraction(v, anchor) { if(anchor === 'auto') { if(v < 1 / 3) anchor = 'left'; From 4e8ff28be9c2e22b4d485b1a87093984f364768a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 12:11:29 -0400 Subject: [PATCH 08/15] :lock: marker.size calc + autorange edit behavior --- src/traces/scatter/attributes.js | 2 +- test/jasmine/tests/scatter_test.js | 44 ++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/src/traces/scatter/attributes.js b/src/traces/scatter/attributes.js index 42c7733ab44..d8a17c934e8 100644 --- a/src/traces/scatter/attributes.js +++ b/src/traces/scatter/attributes.js @@ -273,7 +273,7 @@ module.exports = { dflt: 6, arrayOk: true, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: 'Sets the marker size (in px).' }, maxdisplayed: { diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index 9c19dc2d702..be852cc3a48 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -878,6 +878,50 @@ describe('end-to-end scatter tests', function() { .catch(failTest) .then(done); }); + + it('should update axis range accordingly on marker.size edits', function(done) { + function _assert(msg, xrng, yrng) { + var fullLayout = gd._fullLayout; + expect(fullLayout.xaxis.range).toBeCloseToArray(xrng, 2, msg + ' xrng'); + expect(fullLayout.yaxis.range).toBeCloseToArray(yrng, 2, msg + ' yrng'); + } + + // edit types are important to this test + var schema = Plotly.PlotSchema.get(); + expect(schema.traces.scatter.attributes.marker.size.editType) + .toBe('calc', 'marker.size editType'); + expect(schema.layout.layoutAttributes.xaxis.autorange.editType) + .toBe('plot', 'ax autorange editType'); + + Plotly.plot(gd, [{ y: [1, 2, 1] }]) + .then(function() { + _assert('auto rng / base marker.size', [-0.13, 2.13], [0.93, 2.07]); + return Plotly.relayout(gd, { + 'xaxis.range': [0, 2], + 'yaxis.range': [0, 2] + }); + }) + .then(function() { + _assert('set rng / base marker.size', [0, 2], [0, 2]); + return Plotly.restyle(gd, 'marker.size', 50); + }) + .then(function() { + _assert('set rng / big marker.size', [0, 2], [0, 2]); + return Plotly.relayout(gd, { + 'xaxis.autorange': true, + 'yaxis.autorange': true + }); + }) + .then(function() { + _assert('auto rng / big marker.size', [-0.28, 2.28], [0.75, 2.25]); + return Plotly.restyle(gd, 'marker.size', null); + }) + .then(function() { + _assert('auto rng / base marker.size', [-0.13, 2.13], [0.93, 2.07]); + }) + .catch(failTest) + .then(done); + }); }); describe('scatter hoverPoints', function() { From c75cda8b71d14c8c176e347beaa41dd742939d56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 17:47:28 -0400 Subject: [PATCH 09/15] swap and :lock: 'calcIfAutorange' -> 'calc' for shapes --- src/components/shapes/attributes.js | 26 ++++++------- test/jasmine/tests/shapes_test.js | 59 +++++++++++++++++++++-------- 2 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/components/shapes/attributes.js b/src/components/shapes/attributes.js index 0a4e5d45891..b42790aa01f 100644 --- a/src/components/shapes/attributes.js +++ b/src/components/shapes/attributes.js @@ -19,7 +19,7 @@ module.exports = templatedArray('shape', { valType: 'boolean', role: 'info', dflt: true, - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Determines whether or not this shape is visible.' ].join(' ') @@ -29,7 +29,7 @@ module.exports = templatedArray('shape', { valType: 'enumerated', values: ['circle', 'rect', 'path', 'line'], role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Specifies the shape type to be drawn.', @@ -79,7 +79,7 @@ module.exports = templatedArray('shape', { values: ['scaled', 'pixel'], dflt: 'scaled', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the shapes\'s sizing mode along the x axis.', 'If set to *scaled*, `x0`, `x1` and x coordinates within `path` refer to', @@ -95,7 +95,7 @@ module.exports = templatedArray('shape', { xanchor: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Only relevant in conjunction with `xsizemode` set to *pixel*.', 'Specifies the anchor point on the x axis to which `x0`, `x1`', @@ -107,7 +107,7 @@ module.exports = templatedArray('shape', { x0: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the shape\'s starting x position.', 'See `type` and `xsizemode` for more info.' @@ -116,7 +116,7 @@ module.exports = templatedArray('shape', { x1: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the shape\'s end x position.', 'See `type` and `xsizemode` for more info.' @@ -138,7 +138,7 @@ module.exports = templatedArray('shape', { values: ['scaled', 'pixel'], dflt: 'scaled', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the shapes\'s sizing mode along the y axis.', 'If set to *scaled*, `y0`, `y1` and y coordinates within `path` refer to', @@ -154,7 +154,7 @@ module.exports = templatedArray('shape', { yanchor: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Only relevant in conjunction with `ysizemode` set to *pixel*.', 'Specifies the anchor point on the y axis to which `y0`, `y1`', @@ -166,7 +166,7 @@ module.exports = templatedArray('shape', { y0: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the shape\'s starting y position.', 'See `type` and `ysizemode` for more info.' @@ -175,7 +175,7 @@ module.exports = templatedArray('shape', { y1: { valType: 'any', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'Sets the shape\'s end y position.', 'See `type` and `ysizemode` for more info.' @@ -185,7 +185,7 @@ module.exports = templatedArray('shape', { path: { valType: 'string', role: 'info', - editType: 'calcIfAutorange+arraydraw', + editType: 'calc+arraydraw', description: [ 'For `type` *path* - a valid SVG path with the pixel values', 'replaced by data values in `xsizemode`/`ysizemode` being *scaled*', @@ -224,10 +224,10 @@ module.exports = templatedArray('shape', { }, line: { color: extendFlat({}, scatterLineAttrs.color, {editType: 'arraydraw'}), - width: extendFlat({}, scatterLineAttrs.width, {editType: 'calcIfAutorange+arraydraw'}), + width: extendFlat({}, scatterLineAttrs.width, {editType: 'calc+arraydraw'}), dash: extendFlat({}, dash, {editType: 'arraydraw'}), role: 'info', - editType: 'calcIfAutorange+arraydraw' + editType: 'calc+arraydraw' }, fillcolor: { valType: 'color', diff --git a/test/jasmine/tests/shapes_test.js b/test/jasmine/tests/shapes_test.js index 6e6764df1be..f73a628de14 100644 --- a/test/jasmine/tests/shapes_test.js +++ b/test/jasmine/tests/shapes_test.js @@ -529,12 +529,17 @@ describe('shapes edge cases', function() { }); describe('shapes autosize', function() { - 'use strict'; - var gd; afterEach(destroyGraphDiv); + function assertRanges(msg, x, y) { + var fullLayout = gd._fullLayout; + var PREC = 1; + expect(fullLayout.xaxis.range).toBeCloseToArray(x, PREC, msg + ' - xaxis'); + expect(fullLayout.yaxis.range).toBeCloseToArray(y, PREC, msg + ' - yaxis'); + } + it('should adapt to relayout calls', function(done) { gd = createGraphDiv(); @@ -557,31 +562,55 @@ describe('shapes autosize', function() { } }; - function assertRanges(x, y) { - var fullLayout = gd._fullLayout; - var PREC = 1; - - expect(fullLayout.xaxis.range).toBeCloseToArray(x, PREC, '- xaxis'); - expect(fullLayout.yaxis.range).toBeCloseToArray(y, PREC, '- yaxis'); - } - Plotly.plot(gd, mock).then(function() { - assertRanges([0, 2], [0, 2]); - + assertRanges('base', [0, 2], [0, 2]); return Plotly.relayout(gd, { 'shapes[1].visible': false }); }) .then(function() { - assertRanges([0, 1], [0, 1]); + assertRanges('shapes[1] not visible', [0, 1], [0, 1]); return Plotly.relayout(gd, { 'shapes[1].visible': true }); }) .then(function() { - assertRanges([0, 2], [0, 2]); + assertRanges('back to base', [0, 2], [0, 2]); return Plotly.relayout(gd, { 'shapes[0].x1': 3 }); }) .then(function() { - assertRanges([0, 3], [0, 2]); + assertRanges('stretched shapes[0]', [0, 3], [0, 2]); + }) + .catch(failTest) + .then(done); + }); + + it('should propagate axis autorange changes when axis ranges are set', function(done) { + gd = createGraphDiv(); + + Plotly.plot(gd, [{y: [1, 2]}], { + xaxis: {range: [0, 2]}, + yaxis: {range: [0, 2]}, + shapes: [{ + x0: 2, y0: 2, + x1: 3, y1: 3 + }] + }) + .then(function() { + assertRanges('set rng / narrow shape', [0, 2], [0, 2]); + return Plotly.relayout(gd, 'shapes[0].x1', 10); + }) + .then(function() { + assertRanges('set rng / large shape', [0, 2], [0, 2]); + return Plotly.relayout(gd, { + 'xaxis.autorange': true, + 'yaxis.autorange': true + }); + }) + .then(function() { + assertRanges('auto rng / large shape', [-0.61, 10], [0.86, 3]); + return Plotly.relayout(gd, 'shapes[0].x1', 3); + }) + .then(function() { + assertRanges('auto rng / small shape', [-0.18, 3], [0.86, 3]); }) .catch(failTest) .then(done); From 8f23ef5ba4d335f7f89dbc93b463105e17185e04 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 17:48:09 -0400 Subject: [PATCH 10/15] swap and :lock: 'calcIfAutorange' -> 'calc' for box attributes --- src/traces/box/attributes.js | 16 +++++++-------- test/jasmine/tests/box_test.js | 36 ++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/traces/box/attributes.js b/src/traces/box/attributes.js index fdd2e6e4d7a..215aa421145 100644 --- a/src/traces/box/attributes.js +++ b/src/traces/box/attributes.js @@ -78,7 +78,7 @@ module.exports = { max: 1, dflt: 0.5, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'Sets the width of the whiskers relative to', 'the box\' width.', @@ -88,7 +88,7 @@ module.exports = { notched: { valType: 'boolean', role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'Determines whether or not notches should be drawn.' ].join(' ') @@ -99,7 +99,7 @@ module.exports = { max: 0.5, dflt: 0.25, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'Sets the width of the notches relative to', 'the box\' width.', @@ -111,7 +111,7 @@ module.exports = { values: ['all', 'outliers', 'suspectedoutliers', false], dflt: 'outliers', role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'If *outliers*, only the sample points lying outside the whiskers', 'are shown', @@ -127,7 +127,7 @@ module.exports = { values: [true, 'sd', false], dflt: false, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'If *true*, the mean of the box(es)\' underlying distribution is', 'drawn as a dashed line inside the box(es).', @@ -139,7 +139,7 @@ module.exports = { min: 0, max: 1, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'Sets the amount of jitter in the sample points drawn.', 'If *0*, the sample points align along the distribution axis.', @@ -152,7 +152,7 @@ module.exports = { min: -2, max: 2, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'Sets the position of the sample points in relation to the box(es).', 'If *0*, the sample points are places over the center of the box(es).', @@ -184,7 +184,7 @@ module.exports = { opacity: extendFlat({}, scatterMarkerAttrs.opacity, {arrayOk: false, dflt: 1, editType: 'style'}), size: extendFlat({}, scatterMarkerAttrs.size, - {arrayOk: false, editType: 'calcIfAutorange'}), + {arrayOk: false, editType: 'calc'}), color: extendFlat({}, scatterMarkerAttrs.color, {arrayOk: false, editType: 'style'}), line: { diff --git a/test/jasmine/tests/box_test.js b/test/jasmine/tests/box_test.js index d045267860e..e18a7ac2536 100644 --- a/test/jasmine/tests/box_test.js +++ b/test/jasmine/tests/box_test.js @@ -399,4 +399,40 @@ describe('Test box restyle:', function() { .catch(failTest) .then(done); }); + + it('should update axis range accordingly on calc edits', function(done) { + function _assert(msg, xrng, yrng) { + var fullLayout = gd._fullLayout; + expect(fullLayout.xaxis.range).toBeCloseToArray(xrng, 2, msg + ' xrng'); + expect(fullLayout.yaxis.range).toBeCloseToArray(yrng, 2, msg + ' yrng'); + } + + Plotly.plot(gd, [{ + type: 'box', + y: [0, 1, 1, 1, 1, 2, 2, 3, 5, 6, 10] + }], { + xaxis: {range: [-0.5, 0.5]}, + yaxis: {range: [-0.5, 10.5]} + }) + .then(function() { + _assert('auto rng / no boxpoints', [-0.5, 0.5], [-0.5, 10.5]); + return Plotly.restyle(gd, 'boxpoints', 'all'); + }) + .then(function() { + _assert('set rng / all boxpoints', [-0.5, 0.5], [-0.5, 10.5]); + return Plotly.relayout(gd, { + 'xaxis.autorange': true, + 'yaxis.autorange': true + }); + }) + .then(function() { + _assert('auto rng / all boxpoints', [-0.695, 0.5], [-0.555, 10.555]); + return Plotly.restyle(gd, 'boxpoints', false); + }) + .then(function() { + _assert('auto rng / no boxpoints', [-0.5, 0.5], [-0.555, 10.555]); + }) + .catch(failTest) + .then(done); + }); }); From a3267785605fa078a17a8e53d8f3837993ea8455 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 17:49:28 -0400 Subject: [PATCH 11/15] fix and :lock: ohlc tickwidth edits --- src/traces/ohlc/attributes.js | 2 +- test/jasmine/tests/finance_test.js | 52 ++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/src/traces/ohlc/attributes.js b/src/traces/ohlc/attributes.js index e37213cbdc0..eaf5101a20f 100644 --- a/src/traces/ohlc/attributes.js +++ b/src/traces/ohlc/attributes.js @@ -110,7 +110,7 @@ module.exports = { max: 0.5, dflt: 0.3, role: 'style', - editType: 'calcIfAutorange', + editType: 'calc', description: [ 'Sets the width of the open/close tick marks', 'relative to the *x* minimal interval.' diff --git a/test/jasmine/tests/finance_test.js b/test/jasmine/tests/finance_test.js index 1814f6214c6..df5e8614f02 100644 --- a/test/jasmine/tests/finance_test.js +++ b/test/jasmine/tests/finance_test.js @@ -931,6 +931,58 @@ describe('finance charts updates:', function() { .then(done); }); + it('should be able to update ohlc tickwidth', function(done) { + var trace0 = Lib.extendDeep({}, mock0, {type: 'ohlc'}); + + function _assert(msg, exp) { + var tickLen = gd.calcdata[0][0].t.tickLen; + expect(tickLen) + .toBe(exp.tickLen, 'tickLen val in calcdata - ' + msg); + var pathd = d3.select(gd).select('.ohlc > path').attr('d'); + expect(pathd) + .toBe(exp.pathd, 'path d attr - ' + msg); + } + + Plotly.plot(gd, [trace0], { + xaxis: { rangeslider: {visible: false} } + }) + .then(function() { + _assert('auto rng / base tickwidth', { + tickLen: 0.3, + pathd: 'M13.5,137.63H33.75M33.75,75.04V206.53M54,80.3H33.75' + }); + return Plotly.restyle(gd, 'tickwidth', 0); + }) + .then(function() { + _assert('auto rng / no tickwidth', { + tickLen: 0, + pathd: 'M33.75,137.63H33.75M33.75,75.04V206.53M33.75,80.3H33.75' + }); + + return Plotly.update(gd, { + tickwidth: null + }, { + 'xaxis.range': [0, 8], + 'yaxis.range': [30, 36] + }); + }) + .then(function() { + _assert('set rng / base tickwidth', { + tickLen: 0.3, + pathd: 'M-20.25,134.55H0M0,81V193.5M20.25,85.5H0' + }); + return Plotly.restyle(gd, 'tickwidth', 0); + }) + .then(function() { + _assert('set rng / no tickwidth', { + tickLen: 0, + pathd: 'M0,134.55H0M0,81V193.5M0,85.5H0' + }); + }) + .catch(failTest) + .then(done); + }); + }); describe('finance charts *special* handlers:', function() { From 138a84f613c7c6185c6c1d1d3978fda4754f76e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 17:50:42 -0400 Subject: [PATCH 12/15] :hocho: calcIfAutorange logic in Plotly methods ... including the refAutorange block which is no longer necessary. --- src/plot_api/edit_types.js | 10 ++--- src/plot_api/plot_api.js | 79 +++++--------------------------------- 2 files changed, 12 insertions(+), 77 deletions(-) diff --git a/src/plot_api/edit_types.js b/src/plot_api/edit_types.js index ab9ed4ed84b..83cc9c8655b 100644 --- a/src/plot_api/edit_types.js +++ b/src/plot_api/edit_types.js @@ -15,13 +15,11 @@ var isPlainObject = Lib.isPlainObject; var traceOpts = { valType: 'flaglist', extras: ['none'], - flags: ['calc', 'calcIfAutorange', 'clearAxisTypes', 'plot', 'style', 'colorbars'], + flags: ['calc', 'clearAxisTypes', 'plot', 'style', 'colorbars'], description: [ 'trace attributes should include an `editType` string matching this flaglist.', '*calc* is the most extensive: a full `Plotly.plot` starting by clearing `gd.calcdata`', 'to force it to be regenerated', - '*calcIfAutorange* does a full `Plotly.plot`, but only clears and redoes `gd.calcdata`', - 'if there is at least one autoranged axis.', '*clearAxisTypes* resets the types of the axes this trace is on, because new data could', 'cause the automatic axis type detection to change. Log type will not be cleared, as that', 'is never automatically chosen so must have been user-specified.', @@ -35,15 +33,13 @@ var layoutOpts = { valType: 'flaglist', extras: ['none'], flags: [ - 'calc', 'calcIfAutorange', 'plot', 'legend', 'ticks', 'axrange', + 'calc', 'plot', 'legend', 'ticks', 'axrange', 'layoutstyle', 'modebar', 'camera', 'arraydraw' ], description: [ 'layout attributes should include an `editType` string matching this flaglist.', '*calc* is the most extensive: a full `Plotly.plot` starting by clearing `gd.calcdata`', 'to force it to be regenerated', - '*calcIfAutorange* does a full `Plotly.plot`, but only clears and redoes `gd.calcdata`', - 'if there is at least one autoranged axis.', '*plot* calls `Plotly.plot` but without first clearing `gd.calcdata`.', '*legend* only redraws the legend.', '*ticks* only redraws axis ticks, labels, and gridlines.', @@ -60,7 +56,7 @@ var layoutOpts = { // that shouldn't be used in attributes, to deal with certain // combinations and conditionals efficiently var traceEditTypeFlags = traceOpts.flags.slice() - .concat(['clearCalc', 'fullReplot']); + .concat(['fullReplot']); var layoutEditTypeFlags = layoutOpts.flags.slice() .concat('layoutReplot'); diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 34eec88e08e..5d4ced820a8 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1299,7 +1299,7 @@ exports.restyle = function restyle(gd, astr, val, _traces) { var flags = specs.flags; // clear calcdata and/or axis types if required so they get regenerated - if(flags.clearCalc) gd.calcdata = undefined; + if(flags.calc) gd.calcdata = undefined; if(flags.clearAxisTypes) helpers.clearAxisTypes(gd, traces, {}); // fill in redraw sequence @@ -1597,21 +1597,7 @@ function _restyle(gd, aobj, traces) { } } - // do we need to force a recalc? - var autorangeOn = false; - var axList = Axes.list(gd); - for(i = 0; i < axList.length; i++) { - if(axList[i].autorange) { - autorangeOn = true; - break; - } - } - - // combine a few flags together; - if(flags.calc || (flags.calcIfAutorange && autorangeOn)) { - flags.clearCalc = true; - } - if(flags.calc || flags.plot || flags.calcIfAutorange) { + if(flags.calc || flags.plot) { flags.fullReplot = true; } @@ -1956,37 +1942,21 @@ function _relayout(gd, aobj) { var propStr = containerArrayMatch.property; var componentArray = Lib.nestedProperty(layout, arrayStr); var obji = (componentArray || [])[i] || {}; - var objToAutorange = obji; - var updateValObject = valObject || {editType: 'calc'}; - var checkForAutorange = updateValObject.editType.indexOf('calcIfAutorange') !== -1; - if(i === '') { - // replacing the entire array - too many possibilities, just recalc - if(checkForAutorange) flags.calc = true; - else editTypes.update(flags, updateValObject); - checkForAutorange = false; // clear this, we're already doing a recalc - } - else if(propStr === '') { + if(propStr === '') { // special handling of undoit if we're adding or removing an element // ie 'annotations[2]' which can be {...} (add) or null (remove) - objToAutorange = vi; if(manageArrays.isAddVal(vi)) { undoit[ai] = null; - } - else if(manageArrays.isRemoveVal(vi)) { + } else if(manageArrays.isRemoveVal(vi)) { undoit[ai] = obji; - objToAutorange = obji; + } else { + Lib.warn('unrecognized full object value', aobj); } - else Lib.warn('unrecognized full object value', aobj); } - if(checkForAutorange && (refAutorange(gd, objToAutorange, 'x') || refAutorange(gd, objToAutorange, 'y'))) { - flags.calc = true; - } - else { - editTypes.update(flags, updateValObject); - } + editTypes.update(flags, updateValObject); // prepare the edits object we'll send to applyContainerArrayChanges if(!arrayEdits[arrayStr]) arrayEdits[arrayStr] = {}; @@ -2088,25 +2058,6 @@ function updateAutosize(gd) { return (fullLayout.width !== oldWidth) || (fullLayout.height !== oldHeight); } -// for editing annotations or shapes - is it on autoscaled axes? -function refAutorange(gd, obj, axLetter) { - if(!Lib.isPlainObject(obj)) return false; - var axRef = obj[axLetter + 'ref'] || axLetter, - ax = Axes.getFromId(gd, axRef); - - if(!ax && axRef.charAt(0) === axLetter) { - // fall back on the primary axis in case we've referenced a - // nonexistent axis (as we do above if axRef is missing). - // This assumes the object defaults to data referenced, which - // is the case for shapes and annotations but not for images. - // The only thing this is used for is to determine whether to - // do a full `recalc`, so the only ill effect of this error is - // to waste some time. - ax = Axes.getFromId(gd, axLetter); - } - return (ax || {}).autorange; -} - /** * update: update trace and layout attributes of an existing plot * @@ -2145,7 +2096,7 @@ exports.update = function update(gd, traceUpdate, layoutUpdate, _traces) { var relayoutFlags = relayoutSpecs.flags; // clear calcdata and/or axis types if required - if(restyleFlags.clearCalc || relayoutFlags.calc) gd.calcdata = undefined; + if(restyleFlags.calc || relayoutFlags.calc) gd.calcdata = undefined; if(restyleFlags.clearAxisTypes) helpers.clearAxisTypes(gd, traces, layoutUpdate); // fill in redraw sequence @@ -2392,14 +2343,10 @@ function diffData(gd, oldFullData, newFullData, immutable) { if(seenUIDs[trace.uid]) continue; seenUIDs[trace.uid] = 1; - diffOpts.autoranged = trace.xaxis ? ( - Axes.getFromId(gd, trace.xaxis).autorange || - Axes.getFromId(gd, trace.yaxis).autorange - ) : false; getDiffFlags(oldFullData[i]._fullInput, trace, [], diffOpts); } - if(flags.calc || flags.plot || flags.calcIfAutorange) { + if(flags.calc || flags.plot) { flags.fullReplot = true; } @@ -2438,17 +2385,9 @@ function getDiffFlags(oldContainer, newContainer, outerparts, opts) { var immutable = opts.immutable; var inArray = opts.inArray; var arrayIndex = opts.arrayIndex; - var gd = opts.gd; - var autoranged = opts.autoranged; function changed() { var editType = valObject.editType; - if(editType.indexOf('calcIfAutorange') !== -1 && (autoranged || (autoranged === undefined && ( - refAutorange(gd, newContainer, 'x') || refAutorange(gd, newContainer, 'y') - )))) { - flags.calc = true; - return; - } if(inArray && editType.indexOf('arraydraw') !== -1) { Lib.pushUnique(flags.arrays[inArray], arrayIndex); return; From 1c409472e3b0478c19eb850eaf055470ea793522 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Tue, 17 Jul 2018 18:06:33 -0400 Subject: [PATCH 13/15] try ax autorange as 'axrange' edit type --- src/plot_api/plot_api.js | 1 + src/plots/cartesian/layout_attributes.js | 2 +- test/jasmine/tests/plot_api_test.js | 7 +++++-- test/jasmine/tests/scatter_test.js | 2 +- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/plot_api/plot_api.js b/src/plot_api/plot_api.js index 5d4ced820a8..1ee438d5f0b 100644 --- a/src/plot_api/plot_api.js +++ b/src/plot_api/plot_api.js @@ -1708,6 +1708,7 @@ function addAxRangeSequence(seq, rangesAltered) { subroutines.doTicksRelayout; seq.push( + subroutines.doAutoRangeAndConstraints, doTicks, subroutines.drawData, subroutines.finalDraw diff --git a/src/plots/cartesian/layout_attributes.js b/src/plots/cartesian/layout_attributes.js index 3265be8f9ce..361d624ea7f 100644 --- a/src/plots/cartesian/layout_attributes.js +++ b/src/plots/cartesian/layout_attributes.js @@ -78,7 +78,7 @@ module.exports = { values: [true, false, 'reversed'], dflt: true, role: 'info', - editType: 'plot', + editType: 'axrange', impliedEdits: {'range[0]': undefined, 'range[1]': undefined}, description: [ 'Determines whether or not the range of this axis is', diff --git a/test/jasmine/tests/plot_api_test.js b/test/jasmine/tests/plot_api_test.js index 0a2b609e751..eb1e735387c 100644 --- a/test/jasmine/tests/plot_api_test.js +++ b/test/jasmine/tests/plot_api_test.js @@ -634,7 +634,7 @@ describe('Test plot api', function() { }); it('should trigger minimal sequence for cartesian axis range updates', function() { - var seq = ['doTicksRelayout', 'drawData', 'finalDraw']; + var seq = ['doAutoRangeAndConstraints', 'doTicksRelayout', 'drawData', 'finalDraw']; function _assert(msg) { expect(gd.calcdata).toBeDefined(); @@ -650,7 +650,10 @@ describe('Test plot api', function() { ['relayout', ['xaxis.range[0]', 0]], ['relayout', ['xaxis.range[1]', 3]], ['relayout', ['xaxis.range', [-1, 5]]], - ['update', [{}, {'xaxis.range': [-1, 10]}]] + ['update', [{}, {'xaxis.range': [-1, 10]}]], + + ['relayout', ['xaxis.autorange', true]], + ['update', [{}, {'xaxis.autorange': true}]] ]; specs.forEach(function(s) { diff --git a/test/jasmine/tests/scatter_test.js b/test/jasmine/tests/scatter_test.js index be852cc3a48..7ea0fa9cad9 100644 --- a/test/jasmine/tests/scatter_test.js +++ b/test/jasmine/tests/scatter_test.js @@ -891,7 +891,7 @@ describe('end-to-end scatter tests', function() { expect(schema.traces.scatter.attributes.marker.size.editType) .toBe('calc', 'marker.size editType'); expect(schema.layout.layoutAttributes.xaxis.autorange.editType) - .toBe('plot', 'ax autorange editType'); + .toBe('axrange', 'ax autorange editType'); Plotly.plot(gd, [{ y: [1, 2, 1] }]) .then(function() { From 026cd531dd7266bc39f97ddda60ef0787d60ba7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 18 Jul 2018 13:57:47 -0400 Subject: [PATCH 14/15] lint / fix comment --- src/plots/cartesian/autorange.js | 1 - src/traces/scatter/calc.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/plots/cartesian/autorange.js b/src/plots/cartesian/autorange.js index a009e1da986..63017327ee0 100644 --- a/src/plots/cartesian/autorange.js +++ b/src/plots/cartesian/autorange.js @@ -232,7 +232,6 @@ function doAutoRange(ax) { * and make it a tight bound if possible */ function expand(ax, data, options) { - if(!ax._min) ax._min = []; if(!ax._max) ax._max = []; if(!options) options = {}; diff --git a/src/traces/scatter/calc.js b/src/traces/scatter/calc.js index 2ba49a0ab46..97c3f187bdb 100644 --- a/src/traces/scatter/calc.js +++ b/src/traces/scatter/calc.js @@ -96,7 +96,7 @@ function calcAxisExpansion(gd, trace, xa, ya, x, y, ppad) { yOptions.padded = false; } - // N.B. asymmetric splom traces call this with undefined xa or ya + // N.B. asymmetric splom traces call this with blank {} xa or ya if(xa._id) Axes.expand(xa, x, xOptions); if(ya._id) Axes.expand(ya, y, yOptions); } From 1cdc36bdfbaaed53195d712d018cbf9a76561a7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89tienne=20T=C3=A9treault-Pinard?= Date: Wed, 18 Jul 2018 14:16:59 -0400 Subject: [PATCH 15/15] :hocho: obsolete _rangesliderAutorange field ... this was used to not skip Axes.expand when ax autorange was false, but range slider range wasn't set. --- src/components/rangeslider/defaults.js | 3 +-- src/plots/cartesian/axis_defaults.js | 7 ------- test/jasmine/tests/axes_test.js | 12 ------------ 3 files changed, 1 insertion(+), 21 deletions(-) diff --git a/src/components/rangeslider/defaults.js b/src/components/rangeslider/defaults.js index b262b762a26..9cf208ae638 100644 --- a/src/components/rangeslider/defaults.js +++ b/src/components/rangeslider/defaults.js @@ -46,7 +46,7 @@ module.exports = function handleDefaults(layoutIn, layoutOut, axName) { coerce('borderwidth'); coerce('thickness'); - axOut._rangesliderAutorange = coerce('autorange', !axOut.isValidRange(containerIn.range)); + coerce('autorange', !axOut.isValidRange(containerIn.range)); coerce('range'); var subplots = layoutOut._subplots; @@ -76,7 +76,6 @@ module.exports = function handleDefaults(layoutIn, layoutOut, axName) { if(rangeMode !== 'match') { coerceRange('range', yAxOut.range.slice()); } - yAxOut._rangesliderAutorange = (rangeMode === 'auto'); } } diff --git a/src/plots/cartesian/axis_defaults.js b/src/plots/cartesian/axis_defaults.js index bdd58de1af7..dad861716d4 100644 --- a/src/plots/cartesian/axis_defaults.js +++ b/src/plots/cartesian/axis_defaults.js @@ -48,13 +48,6 @@ module.exports = function handleAxisDefaults(containerIn, containerOut, coerce, setConvert(containerOut, layoutOut); var autoRange = coerce('autorange', !containerOut.isValidRange(containerIn.range)); - - // both x and y axes may need autorange done just for the range slider's purposes - // the logic is complicated to figure this out later, particularly for y axes since - // the settings can be spread out in the x axes... so instead we'll collect them - // during supplyDefaults - containerOut._rangesliderAutorange = false; - if(autoRange) coerce('rangemode'); coerce('range'); diff --git a/test/jasmine/tests/axes_test.js b/test/jasmine/tests/axes_test.js index 30df3aedbfb..fcb899b839c 100644 --- a/test/jasmine/tests/axes_test.js +++ b/test/jasmine/tests/axes_test.js @@ -1874,18 +1874,6 @@ describe('Test axes', function() { expect(ax._min).toEqual([{val: 2, pad: 0, extrapad: false}]); expect(ax._max).toEqual([{val: 5, pad: 0, extrapad: false}]); }); - - it('should consider range slider `autorange`', function() { - ax = getDefaultAx(); - data = [2, 5]; - - ax.autorange = false; - ax._rangesliderAutorange = true; - - expand(ax, data, {}); - expect(ax._min).toEqual([{val: 2, pad: 0, extrapad: false}]); - expect(ax._max).toEqual([{val: 5, pad: 0, extrapad: false}]); - }); }); describe('calcTicks and tickText', function() {