From 14373987da4d86edfbe0cb401162504f8cd6dbca Mon Sep 17 00:00:00 2001 From: Sam Selikoff Date: Sat, 28 Jun 2014 18:40:43 -0400 Subject: [PATCH 1/3] Add remove method to layer #gh-63 --- examples/scripts/bar-chart.js | 3 +-- src/layer-extensions.js | 5 ++++- src/layer.js | 20 ++++++++++++++++--- test/tests/layer.js | 36 ++++++++++++++++++++++++++++++++--- 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/examples/scripts/bar-chart.js b/examples/scripts/bar-chart.js index 52e8230..f21f6ca 100644 --- a/examples/scripts/bar-chart.js +++ b/examples/scripts/bar-chart.js @@ -34,8 +34,7 @@ d3.chart("BarChart", { function onExitTrans() { this.duration(1000) - .attr("x", function(d, i) { return chart.x(i - 1) - .5; }) - .remove(); + .attr("x", function(d, i) { return chart.x(i - 1) - .5; }); } function dataBind(data) { diff --git a/src/layer-extensions.js b/src/layer-extensions.js index 2048f76..8c7bd0a 100644 --- a/src/layer-extensions.js +++ b/src/layer-extensions.js @@ -13,9 +13,12 @@ d3.selection.prototype.layer = function(options) { var layer = new Layer(this); var eventName; - // Set layer methods (required) + // Set layer methods (databind/insert required, remove optional) layer.dataBind = options.dataBind; layer.insert = options.insert; + if (options.remove) { + layer.remove = options.remove; + } // Bind events (optional) if ("events" in options) { diff --git a/src/layer.js b/src/layer.js index cfaa900..f3b6823 100644 --- a/src/layer.js +++ b/src/layer.js @@ -39,6 +39,15 @@ Layer.prototype.insert = function() { d3cAssert(false, "Layers must specify an `insert` method."); }; +/** + * Invoked by {@link Layer#draw} in order to remove exiting DOM nodes from + * this layer's `base`. This default implementation may be overridden by + * Layer instances. + */ +Layer.prototype.remove = function() { + this.remove(); +}; + /** * Subscribe a handler to a "lifecycle event". These events (and only these * events) are triggered when {@link Layer#draw} is invoked--see that method @@ -113,9 +122,12 @@ Layer.prototype.off = function(eventName, handler) { /** * Render the layer according to the input data: Bind the data to the layer - * (according to {@link Layer#dataBind}, insert new elements (according to - * {@link Layer#insert}, make lifecycle selections, and invoke all relevant - * handlers (as attached via {@link Layer#on}) with the lifecycle selections. + * (according to {@link Layer#dataBind}), insert new elements (according to + * {@link Layer#insert}), make lifecycle selections, invoke all relevant + * handlers (as attached via {@link Layer#on}) with the lifecycle selections, + * then remove exiting elements (according to {@link Layer#remove}). + * + * The lifecycle selections are: * * - update * - update:transition @@ -216,4 +228,6 @@ Layer.prototype.draw = function(data) { } } } + + this.remove.call(selection); }; diff --git a/test/tests/layer.js b/test/tests/layer.js index f59ac19..e0686ae 100644 --- a/test/tests/layer.js +++ b/test/tests/layer.js @@ -49,11 +49,18 @@ suite("d3.layer", function() { sinon.spy(entering, "transition"); return entering; }); + var remove = this.remove = sinon.spy(); var base = this.base = d3.select("#test").append("svg"); this.layer = base.layer({ dataBind: dataBind, - insert: insert + insert: insert, + }); + + this.layerWithRemove = this.base.append('g').layer({ + dataBind: dataBind, + insert: insert, + remove: remove }); }); @@ -82,7 +89,24 @@ suite("d3.layer", function() { this.layer.draw([]); assert(this.insert.calledOn(this.dataBind.returnValues[0].enter.returnValues[0])); }); - + test("by default removes exiting nodes from the DOM", function() { + this.layer.draw([1]); + assert.equal(this.layer.selectAll('g').size(), 1); + this.layer.draw([]); + assert.equal(this.layer.selectAll('g').size(), 0); + }); + test("invokes the provided `remove` method in the context of the layer's bound 'exit' selection", function() { + var updating, exiting; + + this.layerWithRemove.draw([1, 2, 3]); + this.layerWithRemove.draw([]); + + updating = this.dataBind.returnValues[1]; + exiting = updating.exit.returnValues[0]; + + assert(this.remove.calledOn(exiting)); + }); + suite("event triggering", function() { test("invokes event handlers with the correct selection", function() { var layer = this.base.append("g").layer({ @@ -137,19 +161,25 @@ suite("d3.layer", function() { }, insert: function() { return this.append("g"); - } + }, + remove: function() { + return this.remove(); + } }); var enterSpy = sinon.spy(); var updateSpy = sinon.spy(); + var exitSpy = sinon.spy(); layer.draw([1]); layer.on("enter", enterSpy); layer.on("update", updateSpy); + layer.on("exit", exitSpy); layer.draw([1]); sinon.assert.callCount(enterSpy, 0); sinon.assert.callCount(updateSpy, 1); + sinon.assert.callCount(exitSpy, 0); }); suite("Layer#off", function() { From 205178e2bb9d069adc6f35b3af794353032b178a Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Sun, 28 Jun 2015 19:09:38 -0400 Subject: [PATCH 2/3] Correct code style inconsistencies --- test/tests/layer.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/tests/layer.js b/test/tests/layer.js index e0686ae..fc3bdcb 100644 --- a/test/tests/layer.js +++ b/test/tests/layer.js @@ -54,10 +54,10 @@ suite("d3.layer", function() { this.layer = base.layer({ dataBind: dataBind, - insert: insert, + insert: insert }); - this.layerWithRemove = this.base.append('g').layer({ + this.layerWithRemove = this.base.append("g").layer({ dataBind: dataBind, insert: insert, remove: remove @@ -91,18 +91,18 @@ suite("d3.layer", function() { }); test("by default removes exiting nodes from the DOM", function() { this.layer.draw([1]); - assert.equal(this.layer.selectAll('g').size(), 1); + assert.equal(this.layer.selectAll("g").size(), 1); this.layer.draw([]); - assert.equal(this.layer.selectAll('g').size(), 0); + assert.equal(this.layer.selectAll("g").size(), 0); }); test("invokes the provided `remove` method in the context of the layer's bound 'exit' selection", function() { - var updating, exiting; + var updating, exiting; this.layerWithRemove.draw([1, 2, 3]); this.layerWithRemove.draw([]); - updating = this.dataBind.returnValues[1]; - exiting = updating.exit.returnValues[0]; + updating = this.dataBind.returnValues[1]; + exiting = updating.exit.returnValues[0]; assert(this.remove.calledOn(exiting)); }); @@ -162,9 +162,9 @@ suite("d3.layer", function() { insert: function() { return this.append("g"); }, - remove: function() { - return this.remove(); - } + remove: function() { + return this.remove(); + } }); var enterSpy = sinon.spy(); var updateSpy = sinon.spy(); From 3f3b0f370688f4dab28a8bd3af4f5013e34724f8 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Wed, 17 Jun 2015 20:11:07 -0400 Subject: [PATCH 3/3] Limit invocation of `remove` handler Do not invoke a layer instance's `remove` handler if the `exit` selection is empty. This completes the implementation of `Layer#remove`. --- src/layer.js | 4 +++- test/tests/layer.js | 6 ++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/src/layer.js b/src/layer.js index f3b6823..b2a56ca 100644 --- a/src/layer.js +++ b/src/layer.js @@ -229,5 +229,7 @@ Layer.prototype.draw = function(data) { } } - this.remove.call(selection); + if (!selection.empty()) { + this.remove.call(selection); + } }; diff --git a/test/tests/layer.js b/test/tests/layer.js index fc3bdcb..cf94d3e 100644 --- a/test/tests/layer.js +++ b/test/tests/layer.js @@ -106,6 +106,12 @@ suite("d3.layer", function() { assert(this.remove.calledOn(exiting)); }); + test("does not invoke `remove` method when `exit` selection is empty", function() { + this.layerWithRemove.draw([1, 2, 3]); + this.layerWithRemove.draw([1, 2, 3]); + + assert.equal(this.remove.callCount, 0); + }); suite("event triggering", function() { test("invokes event handlers with the correct selection", function() {