From fffad740bfa216ffd89e0359bc098e884cb768b0 Mon Sep 17 00:00:00 2001 From: Dan Vanderkam Date: Tue, 26 Jul 2011 17:00:47 -0400 Subject: [PATCH] fix bug with invisible dygraphs and add a regression test --- auto_tests/tests/css.js | 58 +++++++++++++++++++++++++++++++++++++++++++++++++ dygraph.js | 13 ++++------- 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/auto_tests/tests/css.js b/auto_tests/tests/css.js index 79b8910..b2b0010 100644 --- 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 = + '
' + + '
' + + '
'; + 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 = + '
' + + '
' + + '
'; + 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 = + ''; + 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 --- 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 []; } }; -- 2.7.4