fix bug with invisible dygraphs and add a regression test

diff --git a/auto_tests/tests/css.js b/auto_tests/tests/css.js
index 79b8910..b2b0010 100644 (file)
--- a/auto_tests/tests/css.js
+++ b/auto_tests/tests/css.js
@@ -105,6 +105,64 @@ CssTestCase.prototype.testClassPixelSize = function() {
   assertEquals({width: 456, height: 345}, g.size());
 };
 
+// An invisible chart div shouldn't produce an error.
+CssTestCase.prototype.testInvisibleChart = function() {
+  document.body.innerHTML =
+      '<div style="display:none;">' +
+      '<div id="graph" style="width: 640px; height: 480px;"></div>' +
+      '</div>';
+  var graph = document.getElementById("graph");
+  g = new Dygraph(graph, CssTestCase.data, {});
+};
+
+// An invisible chart div shouldn't produce an error.
+CssTestCase.prototype.testInvisibleChartDate = function() {
+  document.body.innerHTML =
+      '<div style="display:none;">' +
+      '<div id="graph" style="width: 640px; height: 480px;"></div>' +
+      '</div>';
+  var graph = document.getElementById("graph");
+  g = new Dygraph(graph,
+                  "Date,Y\n" +
+                  "2010/01/01,100\n" +
+                  "2010/02/01,200\n" +
+                  "2010/03/01,300\n" +
+                  "2010/04/01,400\n" +
+                  "2010/05/01,300\n" +
+                  "2010/06/01,100\n"
+                  , {});
+};
+
+// An invisible chart div that becomes visible.
+CssTestCase.prototype.testInvisibleThenVisibleChart = function() {
+  document.body.innerHTML =
+      '<div id="x" style="display:none;">' +
+      '<div id="graph" style="width: 640px; height: 480px;"></div>' +
+      '</div>';
+  var graph = document.getElementById("graph");
+  g = new Dygraph(graph,
+                  "Date,Y\n" +
+                  "2010/01/01,100\n" +
+                  "2010/02/01,200\n" +
+                  "2010/03/01,300\n" +
+                  "2010/04/01,400\n" +
+                  "2010/05/01,300\n" +
+                  "2010/06/01,100\n"
+                  , {});
+
+  // g.size() is undefined here (probably 0x0)
+  document.getElementById("x").style.display = "";
+
+  // This resize() call is annoying but essential.
+  // There are no DOM events to inform the dygraph that its div has changed size
+  // or visibility so we need to let it know ourselves.
+  g.resize();
+
+  assertEquals(640, graph.offsetWidth);
+  assertEquals(480, graph.offsetHeight);
+  assertEquals({width: 640, height: 480}, g.size());
+};
+
 // Verifies that a div resize gets picked up.
 /*
   this one isn't quite ready yet.

diff --git a/dygraph.js b/dygraph.js
index a1f8c1c..f004c4e 100644 (file)
--- a/dygraph.js
+++ b/dygraph.js
@@ -235,22 +235,16 @@ Dygraph.prototype.__init__ = function(div, file, attrs) {
   if (div.style.height == '' && attrs.height) {
     div.style.height = attrs.height + "px";
   }
-  if (div.offsetHeight == 0) {
+  if (div.style.height == '' && div.offsetHeight == 0) {
     div.style.height = Dygraph.DEFAULT_HEIGHT + "px";
     if (div.style.width == '') {
       div.style.width = Dygraph.DEFAULT_WIDTH + "px";
     }
   }
+  // these will be zero if the dygraph's div is hidden.
   this.width_ = div.offsetWidth;
   this.height_ = div.offsetHeight;
 
-  if (this.width_ == 0) {
-    this.error("dygraph has zero width. Please specify a width in pixels.");
-  }
-  if (this.height_ == 0) {
-    this.error("dygraph has zero height. Please specify a height in pixels.");
-  }
-
   // TODO(danvk): set fillGraph to be part of attrs_ here, not user_attrs_.
   if (attrs['stackedGraph']) {
     attrs['fillGraph'] = true;
@@ -1661,7 +1655,8 @@ Dygraph.dateTicker = function(startDate, endDate, self) {
   if (chosen >= 0) {
     return self.GetXAxis(startDate, endDate, chosen);
   } else {
-    // TODO(danvk): signal error.
+    // this can happen if self.width_ is zero.
+    return [];
   }
 };