From 87f78fb2306da355f82c436ee204cacf7a06fe10 Mon Sep 17 00:00:00 2001 From: Dan Vanderkam Date: Tue, 27 Dec 2016 08:56:51 -0500 Subject: [PATCH] Make resetZoom consistent between x- and y-axes (#812) This also eliminates isZoomedIgnoreProgrammaticZoom and the zoomed_x, zoomed_y flags. --- auto_tests/tests/interaction_model.js | 45 ++--- scripts/run-tests.sh | 2 + src/dygraph-options-reference.js | 6 - src/dygraph-utils.js | 1 - src/dygraph.js | 248 ++++++++++---------------- tests/is-zoomed-ignore-programmatic-zoom.html | 167 ----------------- tests/is-zoomed.html | 25 +-- tests/zoom.html | 10 +- 8 files changed, 126 insertions(+), 378 deletions(-) delete mode 100644 tests/is-zoomed-ignore-programmatic-zoom.html diff --git a/auto_tests/tests/interaction_model.js b/auto_tests/tests/interaction_model.js index a846df5..2015e11 100644 --- a/auto_tests/tests/interaction_model.js +++ b/auto_tests/tests/interaction_model.js @@ -1,4 +1,4 @@ -/** +/** * @fileoverview Test cases for the interaction model. * * @author konigsberg@google.com (Robert Konigsbrg) @@ -344,7 +344,7 @@ it('testCorrectAxisValueRangeAfterUnzoom', function() { dateWindow: [1, 9], animatedZooms:false }); - + // Zoom x axis DygraphOps.dispatchMouseDown_Point(g, 100, 100); DygraphOps.dispatchMouseMove_Point(g, 130, 100); @@ -356,13 +356,13 @@ it('testCorrectAxisValueRangeAfterUnzoom', function() { DygraphOps.dispatchMouseUp_Point(g, 100, 130); var currentYAxisRange = g.yAxisRange(); var currentXAxisRange = g.xAxisRange(); - + //check that the range for the axis has changed assert.notEqual(1, currentXAxisRange[0]); assert.notEqual(10, currentXAxisRange[1]); assert.notEqual(1, currentYAxisRange[0]); assert.notEqual(50, currentYAxisRange[1]); - + // unzoom by doubleclick. This is really the order in which a browser // generates events, and we depend on it. DygraphOps.dispatchMouseDown_Point(g, 10, 10); @@ -370,13 +370,10 @@ it('testCorrectAxisValueRangeAfterUnzoom', function() { DygraphOps.dispatchMouseDown_Point(g, 10, 10); DygraphOps.dispatchMouseUp_Point(g, 10, 10); DygraphOps.dispatchDoubleClick(g, null); - - // check if range for y-axis was reset to original value - // TODO check if range for x-axis is correct. - // Currently not possible because dateRange is set to null and extremes are returned - var newYAxisRange = g.yAxisRange(); - assert.equal(1, newYAxisRange[0]); - assert.equal(50, newYAxisRange[1]); + + // check if the range for both axis was reset to show the full data. + assert.deepEqual(g.yAxisExtremes()[0], g.yAxisRange()); + assert.deepEqual(g.xAxisExtremes(), g.xAxisRange()); }); /** @@ -468,8 +465,9 @@ it('testCorrectAxisPaddingAfterUnzoom', function() { animatedZooms:false }); - var extremes = g.xAxisExtremes(); - + var xExtremes = g.xAxisExtremes(); + var [ yExtremes ] = g.yAxisExtremes(); + // Zoom x axis DygraphOps.dispatchMouseDown_Point(g, 100, 100); DygraphOps.dispatchMouseMove_Point(g, 130, 100); @@ -479,15 +477,11 @@ it('testCorrectAxisPaddingAfterUnzoom', function() { DygraphOps.dispatchMouseDown_Point(g, 100, 100); DygraphOps.dispatchMouseMove_Point(g, 100, 130); DygraphOps.dispatchMouseUp_Point(g, 100, 130); - var currentYAxisRange = g.yAxisRange(); - var currentXAxisRange = g.xAxisRange(); - + //check that the range for the axis has changed - assert.notEqual(1, currentXAxisRange[0]); - assert.notEqual(10, currentXAxisRange[1]); - assert.notEqual(1, currentYAxisRange[0]); - assert.notEqual(50, currentYAxisRange[1]); - + assert.notDeepEqual([1, 10], g.xAxisRange()); + assert.notDeepEqual([1, 50], g.yAxisRange()); + // unzoom by doubleclick. This is really the order in which a browser // generates events, and we depend on it. DygraphOps.dispatchMouseDown_Point(g, 10, 10); @@ -495,11 +489,10 @@ it('testCorrectAxisPaddingAfterUnzoom', function() { DygraphOps.dispatchMouseDown_Point(g, 10, 10); DygraphOps.dispatchMouseUp_Point(g, 10, 10); DygraphOps.dispatchDoubleClick(g, null); - - // check if range for x-axis was reset to original value - var newXAxisRange = g.xAxisRange(); - assert.equal(extremes[0], newXAxisRange[0]); - assert.equal(extremes[1], newXAxisRange[1]); + + // check if range for x-axis was reset to original value. + assert.deepEqual(xExtremes, g.xAxisRange()); + assert.deepEqual(yExtremes, g.yAxisRange()); }); }); diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index 4bc0718..5dd166f 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -1,6 +1,8 @@ #!/bin/bash # Starts the http-server and runs mocha-phantomjs-based tests # Note that you must run `npm run build` or `npm run watch` before running this. +# Additional arguments are passed to mocha-phantomjs, e.g. +# run-tests.sh --grep interaction-model set -o errexit # Run http-server and save its PID diff --git a/src/dygraph-options-reference.js b/src/dygraph-options-reference.js index dc9243b..d136a93 100644 --- a/src/dygraph-options-reference.js +++ b/src/dygraph-options-reference.js @@ -635,12 +635,6 @@ OPTIONS_REFERENCE = // "default": "18", "description": "Width of the div which contains the y-axis label. Since the y-axis label appears rotated 90 degrees, this actually affects the height of its div." }, - "isZoomedIgnoreProgrammaticZoom" : { - "default": "false", - "labels": ["Zooming"], - "type": "boolean", - "description" : "When this option is passed to updateOptions() along with either the dateWindow or valueRange options, the zoom flags are not changed to reflect a zoomed state. This is primarily useful for when the display area of a chart is changed programmatically and also where manual zooming is allowed and use is made of the isZoomed method to determine this." - }, "drawGrid": { "default": "true for x and y, false for y2", "labels": ["Grid"], diff --git a/src/dygraph-utils.js b/src/dygraph-utils.js index c319e49..a9e4b22 100644 --- a/src/dygraph-utils.js +++ b/src/dygraph-utils.js @@ -841,7 +841,6 @@ var pixelSafeOptions = { 'highlightCallback': true, 'highlightCircleSize': true, 'interactionModel': true, - 'isZoomedIgnoreProgrammaticZoom': true, 'labelsDiv': true, 'labelsKMB': true, 'labelsKMG2': true, diff --git a/src/dygraph.js b/src/dygraph.js index e0fcb58..13c6744 100644 --- a/src/dygraph.js +++ b/src/dygraph.js @@ -154,10 +154,6 @@ Dygraph.prototype.__init__ = function(div, file, attrs) { this.annotations_ = []; - // Zoomed indicators - These indicate when the graph has been zoomed and on what axis. - this.zoomed_x_ = false; - this.zoomed_y_ = false; - // Clear the div. This ensure that, if multiple dygraphs are passed the same // div, then only one will be drawn. div.innerHTML = ""; @@ -333,16 +329,20 @@ Dygraph.prototype.getPluginInstance_ = function(type) { * Axis is an optional parameter. Can be set to 'x' or 'y'. * * The zoomed status for an axis is set whenever a user zooms using the mouse - * or when the dateWindow or valueRange are updated (unless the - * isZoomedIgnoreProgrammaticZoom option is also specified). + * or when the dateWindow or valueRange are updated. Double-clicking or calling + * resetZoom() resets the zoom status for the chart. */ Dygraph.prototype.isZoomed = function(axis) { + const isZoomedX = !!this.dateWindow_; + if (axis === 'x') return isZoomedX; + + const isZoomedY = this.axes_.map(axis => !!axis.valueRange).indexOf(true) >= 0; if (axis === null || axis === undefined) { - return this.zoomed_x_ || this.zoomed_y_; + return isZoomedX || isZoomedY; } - if (axis === 'x') return this.zoomed_x_; - if (axis === 'y') return this.zoomed_y_; - throw "axis parameter is [" + axis + "] must be null, 'x' or 'y'."; + if (axis === 'y') return isZoomedY; + + throw new Error(`axis parameter is [${axis}] must be null, 'x' or 'y'.`); }; /** @@ -510,8 +510,7 @@ Dygraph.prototype.xAxisRange = function() { }; /** - * Returns the lower- and upper-bound x-axis values of the - * data set. + * Returns the lower- and upper-bound x-axis values of the data set. */ Dygraph.prototype.xAxisExtremes = function() { var pad = this.getNumericOption('xRangePad') / this.plotter_.area.w; @@ -530,6 +529,22 @@ Dygraph.prototype.xAxisExtremes = function() { }; /** + * Returns the lower- and upper-bound y-axis values for each axis. These are + * the ranges you'll get if you double-click to zoom out or call resetZoom(). + * The return value is an array of [low, high] tuples, one for each y-axis. + */ +Dygraph.prototype.yAxisExtremes = function() { + // TODO(danvk): this is pretty inefficient + const packed = this.gatherDatasets_(this.rolledSeries_, null); + const { extremes } = packed; + const saveAxes = this.axes_; + this.computeYAxisRanges_(extremes); + const newAxes = this.axes_; + this.axes_ = saveAxes; + return newAxes.map(axis => axis.extremeRange); +} + +/** * Returns the currently-visible y-range for an axis. This can be affected by * zooming, panning or a call to updateOptions. Axis indices are zero-based. If * called with no arguments, returns the range of the first axis. @@ -1264,12 +1279,10 @@ Dygraph.prototype.doZoomXDates_ = function(minDate, maxDate) { // between values, it can jerk around.) var old_window = this.xAxisRange(); var new_window = [minDate, maxDate]; - this.zoomed_x_ = true; - var that = this; - this.doAnimatedZoom(old_window, new_window, null, null, function() { - if (that.getFunctionOption("zoomCallback")) { - that.getFunctionOption("zoomCallback").call(that, - minDate, maxDate, that.yAxisRanges()); + const zoomCallback = this.getFunctionOption('zoomCallback'); + this.doAnimatedZoom(old_window, new_window, null, null, () => { + if (zoomCallback) { + zoomCallback.call(this, minDate, maxDate, this.yAxisRanges()); } }); }; @@ -1296,13 +1309,11 @@ Dygraph.prototype.doZoomY_ = function(lowY, highY) { newValueRanges.push([low, hi]); } - this.zoomed_y_ = true; - var that = this; - this.doAnimatedZoom(null, null, oldValueRanges, newValueRanges, function() { - if (that.getFunctionOption("zoomCallback")) { - var xRange = that.xAxisRange(); - that.getFunctionOption("zoomCallback").call(that, - xRange[0], xRange[1], that.yAxisRanges()); + const zoomCallback = this.getFunctionOption('zoomCallback'); + this.doAnimatedZoom(null, null, oldValueRanges, newValueRanges, () => { + if (zoomCallback) { + const [minX, maxX] = this.xAxisRange(); + zoomCallback.call(this, minX, maxX, this.yAxisRanges()); } }); }; @@ -1322,89 +1333,56 @@ Dygraph.zoomAnimationFunction = function(frame, numFrames) { * double-clicking on the graph. */ Dygraph.prototype.resetZoom = function() { - var dirty = false, dirtyX = false, dirtyY = false; - if (this.dateWindow_ !== null) { - dirty = true; - dirtyX = true; - } - - for (var i = 0; i < this.axes_.length; i++) { - if (typeof(this.axes_[i].valueWindow) !== 'undefined' && this.axes_[i].valueWindow !== null) { - dirty = true; - dirtyY = true; - } - } + const dirtyX = this.isZoomed('x'); + const dirtyY = this.isZoomed('y'); + const dirty = dirtyX || dirtyY; // Clear any selection, since it's likely to be drawn in the wrong place. this.clearSelection(); - if (dirty) { - this.zoomed_x_ = false; - this.zoomed_y_ = false; - - //calculate extremes to avoid lack of padding on reset. - var extremes = this.xAxisExtremes(); - var minDate = extremes[0], - maxDate = extremes[1]; - - // TODO(danvk): merge this block w/ the code below. - if (!this.getBooleanOption("animatedZooms")) { - this.dateWindow_ = null; - for (i = 0; i < this.axes_.length; i++) { - if (this.axes_[i].valueWindow !== null) { - delete this.axes_[i].valueWindow; - } - } - this.drawGraph_(); - if (this.getFunctionOption("zoomCallback")) { - this.getFunctionOption("zoomCallback").call(this, - minDate, maxDate, this.yAxisRanges()); - } - return; - } + if (!dirty) return; - var oldWindow=null, newWindow=null, oldValueRanges=null, newValueRanges=null; - if (dirtyX) { - oldWindow = this.xAxisRange(); - newWindow = [minDate, maxDate]; - } + // Calculate extremes to avoid lack of padding on reset. + const [minDate, maxDate] = this.xAxisExtremes(); - if (dirtyY) { - oldValueRanges = this.yAxisRanges(); - // TODO(danvk): this is pretty inefficient - var packed = this.gatherDatasets_(this.rolledSeries_, null); - var extremes = packed.extremes; + const animatedZooms = this.getBooleanOption('animatedZooms'); + const zoomCallback = this.getFunctionOption('zoomCallback'); - // this has the side-effect of modifying this.axes_. - // this doesn't make much sense in this context, but it's convenient (we - // need this.axes_[*].extremeValues) and not harmful since we'll be - // calling drawGraph_ shortly, which clobbers these values. - this.computeYAxisRanges_(extremes); + // TODO(danvk): merge this block w/ the code below. + if (!animatedZooms) { + this.dateWindow_ = null; + for (const axis of this.axes_) { + if (axis.valueRange) delete axis.valueRange; + } - newValueRanges = []; - for (i = 0; i < this.axes_.length; i++) { - var axis = this.axes_[i]; - newValueRanges.push((axis.valueRange !== null && - axis.valueRange !== undefined) ? - axis.valueRange : axis.extremeRange); - } + this.drawGraph_(); + if (zoomCallback) { + zoomCallback.call(this, minDate, maxDate, this.yAxisRanges()); } + return; + } - var that = this; - this.doAnimatedZoom(oldWindow, newWindow, oldValueRanges, newValueRanges, - function() { - that.dateWindow_ = null; - for (var i = 0; i < that.axes_.length; i++) { - if (that.axes_[i].valueWindow !== null) { - delete that.axes_[i].valueWindow; - } - } - if (that.getFunctionOption("zoomCallback")) { - that.getFunctionOption("zoomCallback").call(that, - minDate, maxDate, that.yAxisRanges()); - } - }); + var oldWindow=null, newWindow=null, oldValueRanges=null, newValueRanges=null; + if (dirtyX) { + oldWindow = this.xAxisRange(); + newWindow = [minDate, maxDate]; + } + + if (dirtyY) { + oldValueRanges = this.yAxisRanges(); + newValueRanges = this.yAxisExtremes(); } + + this.doAnimatedZoom(oldWindow, newWindow, oldValueRanges, newValueRanges, + () => { + this.dateWindow_ = null; + for (const axis of this.axes_) { + if (axis.valueRange) delete axis.valueRange; + } + if (zoomCallback) { + zoomCallback.call(this, minDate, maxDate, this.yAxisRanges()); + } + }); }; /** @@ -1440,18 +1418,17 @@ Dygraph.prototype.doAnimatedZoom = function(oldXRange, newXRange, oldYRanges, ne } } - var that = this; - utils.repeatAndCleanup(function(step) { + utils.repeatAndCleanup(step => { if (valueRanges.length) { - for (var i = 0; i < that.axes_.length; i++) { + for (var i = 0; i < this.axes_.length; i++) { var w = valueRanges[step][i]; - that.axes_[i].valueWindow = [w[0], w[1]]; + this.axes_[i].valueRange = [w[0], w[1]]; } } if (windows.length) { - that.dateWindow_ = windows[step]; + this.dateWindow_ = windows[step]; } - that.drawGraph_(); + this.drawGraph_(); }, steps, Dygraph.ANIMATION_DURATION / steps, callback); }; @@ -2319,10 +2296,7 @@ Dygraph.prototype.drawGraph_ = function() { this.addXTicks_(); - // Save the X axis zoomed status as the updateOptions call will tend to set it erroneously - var tmp_zoomed_x = this.zoomed_x_; // Tell PlotKit to use this new data and render itself - this.zoomed_x_ = tmp_zoomed_x; this.layout_.evaluate(); this.renderGraph_(is_initial_draw); @@ -2342,10 +2316,11 @@ Dygraph.prototype.renderGraph_ = function(is_initial_draw) { this.cascadeEvents_('clearChart'); this.plotter_.clear(); - if (this.getFunctionOption('underlayCallback')) { + const underlayCallback = this.getFunctionOption('underlayCallback'); + if (underlayCallback) { // NOTE: we pass the dygraph object to this callback twice to avoid breaking // users who expect a deprecated form of this callback. - this.getFunctionOption('underlayCallback').call(this, + underlayCallback.call(this, this.hidden_ctx_, this.layout_.getPlotArea(), this, this); } @@ -2362,8 +2337,9 @@ Dygraph.prototype.renderGraph_ = function(is_initial_draw) { // The interaction canvas should already be empty in that situation. this.canvas_.getContext('2d').clearRect(0, 0, this.width_, this.height_); - if (this.getFunctionOption("drawCallback") !== null) { - this.getFunctionOption("drawCallback").call(this, this, is_initial_draw); + const drawCallback = this.getFunctionOption("drawCallback"); + if (drawCallback !== null) { + drawCallback.call(this, this, is_initial_draw); } if (is_initial_draw) { this.readyFired_ = true; @@ -2385,15 +2361,7 @@ Dygraph.prototype.renderGraph_ = function(is_initial_draw) { * indices are into the axes_ array. */ Dygraph.prototype.computeYAxes_ = function() { - // Preserve valueWindow settings if they exist, and if the user hasn't - // specified a new valueRange. - var valueWindows, axis, index, opts, v; - if (this.axes_ !== undefined && this.user_attrs_.hasOwnProperty("valueRange") === false) { - valueWindows = []; - for (index = 0; index < this.axes_.length; index++) { - valueWindows.push(this.axes_[index].valueWindow); - } - } + var axis, index, opts, v; // this.axes_ doesn't match this.attributes_.axes_.options. It's used for // data computation as well as options storage. @@ -2407,26 +2375,6 @@ Dygraph.prototype.computeYAxes_ = function() { this.axes_[axis] = opts; } - - // Copy global valueRange option over to the first axis. - // NOTE(konigsberg): Are these two statements necessary? - // I tried removing it. The automated tests pass, and manually - // messing with tests/zoom.html showed no trouble. - v = this.attr_('valueRange'); - if (v) this.axes_[0].valueRange = v; - - if (valueWindows !== undefined) { - // Restore valueWindow settings. - - // When going from two axes back to one, we only restore - // one axis. - var idxCount = Math.min(valueWindows.length, this.axes_.length); - - for (index = 0; index < idxCount; index++) { - this.axes_[index].valueWindow = valueWindows[index]; - } - } - for (axis = 0; axis < this.axes_.length; axis++) { if (axis === 0) { opts = this.optionsViewForAxis_('y' + (axis ? '2' : '')); @@ -2501,10 +2449,11 @@ Dygraph.prototype.computeYAxisRanges_ = function(extremes) { // ypadCompat = true; ypad = 0.1; // add 10% - if (this.getNumericOption('yRangePad') !== null) { + const yRangePad = this.getNumericOption('yRangePad'); + if (yRangePad !== null) { ypadCompat = false; // Convert pixel padding to ratio - ypad = this.getNumericOption('yRangePad') / this.plotter_.area.h; + ypad = yRangePad / this.plotter_.area.h; } if (series.length === 0) { @@ -2572,12 +2521,7 @@ Dygraph.prototype.computeYAxisRanges_ = function(extremes) { } axis.extremeRange = [minAxisY, maxAxisY]; } - if (axis.valueWindow) { - // This is only set if the user has zoomed on the y-axis. It is never set - // by a user. It takes precedence over axis.valueRange because, if you set - // valueRange, you'd still expect to be able to pan. - axis.computedValueRange = [axis.valueWindow[0], axis.valueWindow[1]]; - } else if (axis.valueRange) { + if (axis.valueRange) { // This is a user-set value range for this axis. var y0 = isNullUndefinedOrNaN(axis.valueRange[0]) ? axis.extremeRange[0] : axis.valueRange[0]; var y1 = isNullUndefinedOrNaN(axis.valueRange[1]) ? axis.extremeRange[1] : axis.valueRange[1]; @@ -2585,7 +2529,7 @@ Dygraph.prototype.computeYAxisRanges_ = function(extremes) { } else { axis.computedValueRange = axis.extremeRange; } - if (!axis.valueWindow && !ypadCompat) { + if (!ypadCompat) { // When using yRangePad, adjust the upper/lower bounds to add // padding unless the user has zoomed/panned the Y axis range. if (logscale) { @@ -3145,12 +3089,6 @@ Dygraph.prototype.updateOptions = function(input_attrs, block_redraw) { } if ('dateWindow' in attrs) { this.dateWindow_ = attrs.dateWindow; - if (!('isZoomedIgnoreProgrammaticZoom' in attrs)) { - this.zoomed_x_ = (attrs.dateWindow !== null); - } - } - if ('valueRange' in attrs && !('isZoomedIgnoreProgrammaticZoom' in attrs)) { - this.zoomed_y_ = (attrs.valueRange !== null); } // TODO(danvk): validate per-series options. @@ -3275,7 +3213,7 @@ Dygraph.prototype.visibility = function() { * * @param {number|number[]|object} num the series index or an array of series indices * or a boolean array of visibility states by index - * or an object mapping series numbers, as keys, to + * or an object mapping series numbers, as keys, to * visibility state (boolean values) * @param {boolean} value the visibility state expressed as a boolean */ diff --git a/tests/is-zoomed-ignore-programmatic-zoom.html b/tests/is-zoomed-ignore-programmatic-zoom.html deleted file mode 100644 index cffc8be..0000000 --- a/tests/is-zoomed-ignore-programmatic-zoom.html +++ /dev/null @@ -1,167 +0,0 @@ - - - - isZoomedIgnoreProgrammaticZoom Flag - - - - - - - -

isZoomedIgnoreProgrammaticZoom Option

-

- By default, when the dateWindow or updateOptions - of a chart is changed programmatically by a call to updateOptions - the zoomed flags (isZoomed) are changed. This is the same - as manually zooming in using the mouse. -

-

- Sometimes it may be desirable to change the display of the chart by - manipulating the dateWindow and valueRange - options but without changing the zoomed flags, for example where manual - zooming is still required but where it is also desired that the zoomed - flags drive display elements, but only for manual zooming. -

-

- In this case isZoomedIgnoreProgrammaticZoom may be specified along with - either the dateWindow or valueRange values to - updateOptions and the zoomed flags will remain unaffected. -

-

- The chart below may be manipulated to change the updateOptions - using the Max and Min Y axis buttons and the dateWindow - by using the Max and Min X axis buttons. -

-

- Toggle the check box below to determine the difference in operation of the zoom flags - when the date and value windows of the chart are changed using the arrows underneath. -

-

Do not change zoom flags (isZoomedIgnoreProgrammaticZoom)

- -
-
-

- Max Y Axis: - - -

-

- Min Y Axis: - - -

-

- Min X Axis: - - -

-

- Max X Axis: - - -

-
-
-
- -
-
-
-

Zoomed Flags

-

Zoomed: False

-

Zoomed X: False

-

Zoomed Y: False

-

Window coordinates (in dates and values):

-
-
-
- - - - diff --git a/tests/is-zoomed.html b/tests/is-zoomed.html index 1becb2e..28c52cf 100644 --- a/tests/is-zoomed.html +++ b/tests/is-zoomed.html @@ -1,7 +1,7 @@ - isZoomedIgnoresProgrammaticZoom Flag + isZoomed method