From 2a02e5dde7f3727c1735761b84ca41c0f241dcaf Mon Sep 17 00:00:00 2001
From: Klaus Weidner <klausw@google.com>
Date: Mon, 27 Feb 2012 10:24:31 -0800
Subject: [PATCH] Changes based on danvk's review:

- turn on background fading by default, remove extra animation option.
- clarify docstrings for closest-point finding methods.
---
 auto_tests/tests/callback.js |  1 -
 dygraph-options-reference.js | 12 +++---------
 dygraph.js                   | 21 +++++++++++++++++----
 tests/series-highlight.html  |  6 +++---
 4 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/auto_tests/tests/callback.js b/auto_tests/tests/callback.js
index 1b851b5..aad0dad 100644
--- a/auto_tests/tests/callback.js
+++ b/auto_tests/tests/callback.js
@@ -168,7 +168,6 @@ var runClosestTest = function(isStacked, widthNormal, widthHighlighted) {
         strokeBorderWidth: 2,
         highlightCircleSize: widthNormal * 2,
         highlightSeriesBackgroundFade: 0.7,
-        highlightSeriesAnimate: true,
 
         highlightSeriesOpts: {
           strokeWidth: widthHighlighted,
diff --git a/dygraph-options-reference.js b/dygraph-options-reference.js
index 19becfd..bc5867e 100644
--- a/dygraph-options-reference.js
+++ b/dygraph-options-reference.js
@@ -146,16 +146,10 @@ Dygraph.OPTIONS_REFERENCE =  // <JSON>
     "description": "When set, the options from this object are applied to the timeseries closest to the mouse pointer for interactive highlighting. See also 'highlightCallback'. Example: highlightSeriesOpts: { strokeWidth: 3 }."
   },
   "highlightSeriesBackgroundFade": {
-    "default": "0",
+    "default": "0.5",
     "labels": ["Interactive Elements"],
-    "type": "number",
-    "description": "When nonzero, dim the background while highlighting series. 0=fully visible, 1=hidden"
-  },
-  "highlightSeriesAnimate": {
-    "default": "false",
-    "labels": ["Interactive Elements"],
-    "type": "boolean",
-    "description": "Animate the background dimming for nonzero highlightSeriesBackgroundFade."
+    "type": "float",
+    "description": "When nonzero, dim the background while highlighting series. 0=fully visible background, 1=hiddden background (show highlighted series only)."
   },
   "includeZero": {
     "default": "false",
diff --git a/dygraph.js b/dygraph.js
index 1d97bda..298f64b 100644
--- a/dygraph.js
+++ b/dygraph.js
@@ -187,8 +187,7 @@ Dygraph.dateAxisFormatter = function(date, granularity) {
 Dygraph.DEFAULT_ATTRS = {
   highlightCircleSize: 3,
   highlightSeriesOpts: null,
-  highlightSeriesBackgroundFade: 0,
-  highlightSeriesAnimated: false,
+  highlightSeriesBackgroundFade: 0.5,
 
   labelsDivWidth: 250,
   labelsDivStyles: {
@@ -1522,7 +1521,12 @@ Dygraph.prototype.findClosestRow = function(domX) {
 };
 
 /**
- * Given canvas X,Y coordinates, find the closest point
+ * Given canvas X,Y coordinates, find the closest point.
+ *
+ * This finds the individual data point across all visible series
+ * that's closest to the supplied DOM coordinates using the standard
+ * Euclidean X,Y distance.
+ *
  * @param {Number} domX graph-relative DOM X coordinate
  * @param {Number} domY graph-relative DOM Y coordinate
  * Returns: {row, seriesName, point}
@@ -1559,6 +1563,11 @@ Dygraph.prototype.findClosestPoint = function(domX, domY) {
 
 /**
  * Given canvas X,Y coordinates, find the touched area in a stacked graph.
+ *
+ * This first finds the X data point closest to the supplied DOM X coordinate,
+ * then finds the series which puts the Y coordinate on top of its filled area,
+ * using linear interpolation between adjacent point pairs.
+ *
  * @param {Number} domX graph-relative DOM X coordinate
  * @param {Number} domY graph-relative DOM Y coordinate
  * Returns: {row, seriesName, point}
@@ -1878,7 +1887,11 @@ Dygraph.prototype.updateSelection_ = function(opt_animFraction) {
     ctx.clearRect(0, 0, this.width_, this.height_);
     var alpha = this.attr_('highlightSeriesBackgroundFade');
     if (alpha) {
-      if (this.attr_('highlightSeriesAnimate')) {
+      // Activating background fade includes an animation effect for a gradual
+      // fade. TODO(klausw): make this independently configurable if it causes
+      // issues? Use a shared preference to control animations?
+      var animateBackgroundFade = true;
+      if (animateBackgroundFade) {
         if (opt_animFraction === undefined) {
           // start a new animation
           this.animateSelection_(1);
diff --git a/tests/series-highlight.html b/tests/series-highlight.html
index 9af12bd..94e09d6 100644
--- a/tests/series-highlight.html
+++ b/tests/series-highlight.html
@@ -60,13 +60,13 @@ var makeGraph = function(className, numSeries, numRows, isStacked) {
       {
         width: 480,
         height: 320,
-        stackedGraph: isStacked,
         labels: labels.slice(),
+        stackedGraph: isStacked,
+
         highlightCircleSize: 2,
         strokeWidth: 1,
         strokeBorderWidth: isStacked ? null : 1,
-        highlightSeriesBackgroundFade: 0.5,
-        highlightSeriesAnimate: true,
+
         highlightSeriesOpts: {
           strokeWidth: 3,
           strokeBorderWidth: 1,
-- 
2.7.4