From a5a507274e6a58f70f8c9017d063210c8b78583b Mon Sep 17 00:00:00 2001 From: Klaus Weidner Date: Fri, 11 May 2012 14:29:43 -0700 Subject: [PATCH] Requests from Dan's code review: - rename the option to drawGapEdgePoints - fix point lookup for isolated points. The code was getting the overall points[] array and limits confused with the subset intended for the current series. --- auto_tests/tests/callback.js | 8 ++++---- dygraph-canvas.js | 26 +++++++++++++++----------- dygraph-options-reference.js | 2 +- tests/isolated-points.html | 2 +- 4 files changed, 21 insertions(+), 17 deletions(-) diff --git a/auto_tests/tests/callback.js b/auto_tests/tests/callback.js index e2f425f..6d3df1d 100644 --- a/auto_tests/tests/callback.js +++ b/auto_tests/tests/callback.js @@ -152,11 +152,11 @@ CallbackTestCase.prototype.testDrawPointCallback_isolated = function() { assertEquals(13, xvalues[0]); assertEquals(15, xvalues[1]); - // Test that isolated points + gap points get drawn when drawGapPoints is set. - // This should add one point at the right edge of the segment at x=11, but not - // at the graph edge at x=10. + // Test that isolated points + gap points get drawn when + // drawGapEdgePoints is set. This should add one point at the right + // edge of the segment at x=11, but not at the graph edge at x=10. xvalues = []; // Reset for new test - graphOpts.drawGapPoints = true; + graphOpts.drawGapEdgePoints = true; g = new Dygraph(graph, testdata, graphOpts); assertEquals(3, xvalues.length); assertEquals(11, xvalues[0]); diff --git a/dygraph-canvas.js b/dygraph-canvas.js index e5939d0..0c44e84 100644 --- a/dygraph-canvas.js +++ b/dygraph-canvas.js @@ -661,11 +661,12 @@ DygraphCanvasRenderer.prototype._renderAnnotations = function() { } }; -DygraphCanvasRenderer.makeNextPointStep_ = function(connect, points, end) { +DygraphCanvasRenderer.makeNextPointStep_ = function( + connect, points, start, end) { if (connect) { return function(j) { - while (++j < end) { - if (!(points[j].yval === null)) break; + while (++j + start < end) { + if (!(points[start + j].yval === null)) break; } return j; } @@ -688,18 +689,22 @@ DygraphCanvasRenderer.prototype._drawStyledLine = function( var points = this.layout.points; var prevX = null; var prevY = null; + var nextY = null; var pointsOnLine = []; // Array of [canvasx, canvasy] pairs. if (!Dygraph.isArrayLike(strokePattern)) { strokePattern = null; } - var drawGapPoints = this.dygraph_.attr_('drawGapPoints', setName); + var drawGapPoints = this.dygraph_.attr_('drawGapEdgePoints', setName); - var point; + var point, nextPoint; var next = DygraphCanvasRenderer.makeNextPointStep_( - this.attr_('connectSeparatedPoints'), points, afterLastIndexInSet); + this.attr_('connectSeparatedPoints'), points, firstIndexInSet, + afterLastIndexInSet); ctx.save(); - for (var j = firstIndexInSet; j < afterLastIndexInSet; j = next(j)) { - point = points[j]; + for (var j = 0; j < setLength; j = next(j)) { + point = points[firstIndexInSet + j]; + nextY = (next(j) < setLength) ? + points[firstIndexInSet + next(j)].canvasy : null; if (isNullOrNaN(point.canvasy)) { if (stepPlot && prevX !== null) { // Draw a horizontal line to the start of the missing data @@ -714,13 +719,12 @@ DygraphCanvasRenderer.prototype._drawStyledLine = function( } else { // A point is "isolated" if it is non-null but both the previous // and next points are null. - var isIsolated = (!prevX && (j == points.length - 1 || - isNullOrNaN(points[j+1].canvasy))); + var isIsolated = (!prevX && isNullOrNaN(nextY)); if (drawGapPoints) { // Also consider a point to be is "isolated" if it's adjacent to a // null point, excluding the graph edges. if ((j > 0 && !prevX) || - (j < points.length - 1 && isNullOrNaN(points[j+1].canvasy))) { + (next(j) < setLength && isNullOrNaN(nextY))) { isIsolated = true; } } diff --git a/dygraph-options-reference.js b/dygraph-options-reference.js index 3fdd4ea..97b410f 100644 --- a/dygraph-options-reference.js +++ b/dygraph-options-reference.js @@ -43,7 +43,7 @@ Dygraph.OPTIONS_REFERENCE = // "type": "boolean", "description": "Draw a small dot at each point, in addition to a line going through the point. This makes the individual data points easier to see, but can increase visual clutter in the chart. The small dot can be replaced with a custom rendering by supplying a drawPointCallback." }, - "drawGapPoints": { + "drawGapEdgePoints": { "default": "false", "labels": ["Data Line display"], "type": "boolean", diff --git a/tests/isolated-points.html b/tests/isolated-points.html index 85e8fd8..f507fda 100644 --- a/tests/isolated-points.html +++ b/tests/isolated-points.html @@ -30,7 +30,7 @@ { labels: ["X", "S1", "S2" ], S1: { - drawGapPoints: true + drawGapEdgePoints: true }, pointSize: 4, showRoller: true -- 2.7.4