From 8fbfac52d4a9daeee106c55b239e0c1054daa772 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 01:06:04 -0700 Subject: [PATCH 1/5] Avoid firing duplicate change events in ol.layer.Layer --- src/ol/layer/layerbase.js | 11 ---------- test/spec/ol/layer/layer.test.js | 36 ++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/src/ol/layer/layerbase.js b/src/ol/layer/layerbase.js index a6f590de07..214f84de28 100644 --- a/src/ol/layer/layerbase.js +++ b/src/ol/layer/layerbase.js @@ -72,17 +72,6 @@ ol.layer.Base = function(options) { this.setValues(values); - goog.events.listen(this, [ - ol.Object.getChangeEventType(ol.layer.LayerProperty.BRIGHTNESS), - ol.Object.getChangeEventType(ol.layer.LayerProperty.CONTRAST), - ol.Object.getChangeEventType(ol.layer.LayerProperty.HUE), - ol.Object.getChangeEventType(ol.layer.LayerProperty.OPACITY), - ol.Object.getChangeEventType(ol.layer.LayerProperty.SATURATION), - ol.Object.getChangeEventType(ol.layer.LayerProperty.MAX_RESOLUTION), - ol.Object.getChangeEventType(ol.layer.LayerProperty.MIN_RESOLUTION) - ], - this.handleLayerChange, false, this); - goog.events.listen(this, ol.Object.getChangeEventType(ol.layer.LayerProperty.VISIBLE), this.handleLayerVisibleChange, false, this); diff --git a/test/spec/ol/layer/layer.test.js b/test/spec/ol/layer/layer.test.js index bcb52a8be7..36d93a6e6d 100644 --- a/test/spec/ol/layer/layer.test.js +++ b/test/spec/ol/layer/layer.test.js @@ -210,6 +210,13 @@ describe('ol.layer.Layer', function() { expect(layer.getBrightness()).to.be(-0.7); }); + it('triggers a change event', function() { + var listener = sinon.spy(); + layer.on(ol.ObjectEventType.CHANGE, listener); + layer.setBrightness(0.5); + expect(listener.calledOnce).to.be(true); + }); + }); describe('#setContrast', function() { @@ -238,6 +245,13 @@ describe('ol.layer.Layer', function() { expect(layer.getContrast()).to.be(42); }); + it('triggers a change event', function() { + var listener = sinon.spy(); + layer.on(ol.ObjectEventType.CHANGE, listener); + layer.setContrast(43); + expect(listener.calledOnce).to.be(true); + }); + }); @@ -277,6 +291,13 @@ describe('ol.layer.Layer', function() { expect(layer.getHue()).to.be(-100); }); + it('triggers a change event', function() { + var listener = sinon.spy(); + layer.on(ol.ObjectEventType.CHANGE, listener); + layer.setHue(0.5); + expect(listener.calledOnce).to.be(true); + }); + }); @@ -301,6 +322,13 @@ describe('ol.layer.Layer', function() { expect(layer.getOpacity()).to.be(0.3); }); + it('triggers a change event', function() { + var listener = sinon.spy(); + layer.on(ol.ObjectEventType.CHANGE, listener); + layer.setOpacity(0.4); + expect(listener.calledOnce).to.be(true); + }); + }); @@ -330,6 +358,13 @@ describe('ol.layer.Layer', function() { expect(layer.getSaturation()).to.be(42); }); + it('triggers a change event', function() { + var listener = sinon.spy(); + layer.on(ol.ObjectEventType.CHANGE, listener); + layer.setSaturation(42); + expect(listener.calledOnce).to.be(true); + }); + }); @@ -356,6 +391,7 @@ describe('ol.layer.Layer', function() { }); goog.require('goog.dispose'); +goog.require('ol.ObjectEventType'); goog.require('ol.layer.Layer'); goog.require('ol.proj'); goog.require('ol.source.Source'); From 858fe141632dea66972cc378125551fbbfad7d4e Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 01:23:51 -0700 Subject: [PATCH 2/5] No need for handleLayerChange in base layer --- src/ol/layer/layerbase.js | 10 ---------- src/ol/layer/layergroup.js | 8 ++++---- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/src/ol/layer/layerbase.js b/src/ol/layer/layerbase.js index 214f84de28..3d28f84a90 100644 --- a/src/ol/layer/layerbase.js +++ b/src/ol/layer/layerbase.js @@ -240,16 +240,6 @@ goog.exportProperty( ol.layer.Base.prototype.getVisible); -/** - * @protected - */ -ol.layer.Base.prototype.handleLayerChange = function() { - if (this.getVisible() && this.getSourceState() == ol.source.State.READY) { - this.dispatchChangeEvent(); - } -}; - - /** * @protected */ diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index df7ce69dff..0a48218943 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -70,9 +70,9 @@ goog.inherits(ol.layer.Group, ol.layer.Base); /** - * @inheritDoc + * @private */ -ol.layer.Group.prototype.handleLayerChange = function() { +ol.layer.Group.prototype.handleLayerChange_ = function() { if (this.getVisible()) { this.dispatchChangeEvent(); } @@ -113,7 +113,7 @@ ol.layer.Group.prototype.handleLayersChanged_ = function(event) { layer = layersArray[i]; this.listenerKeys_[goog.getUid(layer).toString()] = goog.events.listen(layer, goog.events.EventType.CHANGE, - this.handleLayerChange, false, this); + this.handleLayerChange_, false, this); } } @@ -128,7 +128,7 @@ ol.layer.Group.prototype.handleLayersChanged_ = function(event) { ol.layer.Group.prototype.handleLayersAdd_ = function(collectionEvent) { var layer = /** @type {ol.layer.Base} */ (collectionEvent.getElement()); this.listenerKeys_[goog.getUid(layer).toString()] = goog.events.listen( - layer, goog.events.EventType.CHANGE, this.handleLayerChange, false, + layer, goog.events.EventType.CHANGE, this.handleLayerChange_, false, this); this.dispatchChangeEvent(); }; From a792a224f6a006dca0d2fbb712a61bf4ba9526b8 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 01:28:55 -0700 Subject: [PATCH 3/5] Avoid duplicate change event on visibility change --- src/ol/layer/layerbase.js | 5 ----- test/spec/ol/layer/layer.test.js | 22 +++++++++++++++++++--- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/ol/layer/layerbase.js b/src/ol/layer/layerbase.js index 3d28f84a90..341a18e592 100644 --- a/src/ol/layer/layerbase.js +++ b/src/ol/layer/layerbase.js @@ -71,11 +71,6 @@ ol.layer.Base = function(options) { values.minResolution : 0; this.setValues(values); - - goog.events.listen(this, - ol.Object.getChangeEventType(ol.layer.LayerProperty.VISIBLE), - this.handleLayerVisibleChange, false, this); - }; goog.inherits(ol.layer.Base, ol.Object); diff --git a/test/spec/ol/layer/layer.test.js b/test/spec/ol/layer/layer.test.js index 36d93a6e6d..8fd23001e3 100644 --- a/test/spec/ol/layer/layer.test.js +++ b/test/spec/ol/layer/layer.test.js @@ -370,20 +370,36 @@ describe('ol.layer.Layer', function() { describe('#setVisible', function() { - it('sets visible property', function() { - var layer = new ol.layer.Layer({ + var layer; + beforeEach(function() { + layer = new ol.layer.Layer({ source: new ol.source.Source({ projection: ol.proj.get('EPSG:4326') }) }); + }); + afterEach(function() { + goog.dispose(layer); + }); + + it('sets visible property', function() { layer.setVisible(false); expect(layer.getVisible()).to.be(false); layer.setVisible(true); expect(layer.getVisible()).to.be(true); + }); - goog.dispose(layer); + it('fires a change event', function() { + var listener = sinon.spy(); + layer.on(ol.ObjectEventType.CHANGE, listener); + + layer.setVisible(false); + expect(listener.callCount).to.be(1); + + layer.setVisible(true); + expect(listener.callCount).to.be(2); }); }); From 5a5d1dec40611d816fd006b79d0a540e34333294 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 01:38:44 -0700 Subject: [PATCH 4/5] Remove unused handleLayerVisibleChange --- src/ol/layer/layerbase.js | 10 ---------- src/ol/layer/layergroup.js | 8 -------- 2 files changed, 18 deletions(-) diff --git a/src/ol/layer/layerbase.js b/src/ol/layer/layerbase.js index 341a18e592..5c1a1d24db 100644 --- a/src/ol/layer/layerbase.js +++ b/src/ol/layer/layerbase.js @@ -235,16 +235,6 @@ goog.exportProperty( ol.layer.Base.prototype.getVisible); -/** - * @protected - */ -ol.layer.Base.prototype.handleLayerVisibleChange = function() { - if (this.getSourceState() == ol.source.State.READY) { - this.dispatchChangeEvent(); - } -}; - - /** * Adjust the layer brightness. A value of -1 will render the layer completely * black. A value of 0 will leave the brightness unchanged. A value of 1 will diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index 0a48218943..9fd4b6c5b0 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -79,14 +79,6 @@ ol.layer.Group.prototype.handleLayerChange_ = function() { }; -/** - * @inheritDoc - */ -ol.layer.Group.prototype.handleLayerVisibleChange = function() { - this.dispatchChangeEvent(); -}; - - /** * @param {goog.events.Event} event Event. * @private From 8b8563f0fdb044a17472d2c3810cb96cfba1f029 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 01:46:23 -0700 Subject: [PATCH 5/5] Test change events on layer groups --- test/spec/ol/layer/layergroup.test.js | 40 +++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/test/spec/ol/layer/layergroup.test.js b/test/spec/ol/layer/layergroup.test.js index 5946ec1d20..cefa283374 100644 --- a/test/spec/ol/layer/layergroup.test.js +++ b/test/spec/ol/layer/layergroup.test.js @@ -63,6 +63,45 @@ describe('ol.layer.Group', function() { }); + describe('change event', function() { + + var layer, group, listener; + beforeEach(function() { + layer = new ol.layer.Layer({ + source: new ol.source.Source({ + projection: 'EPSG:4326' + }) + }); + group = new ol.layer.Group({ + layers: [layer] + }); + listener = sinon.spy(); + }); + + afterEach(function() { + goog.dispose(group); + goog.dispose(layer); + }); + + it('is dispatched by the group when layer opacity changes', function() { + group.on(ol.ObjectEventType.CHANGE, listener); + + layer.setOpacity(0.5); + expect(listener.calledOnce).to.be(true); + }); + + it('is dispatched by the group when layer visibility changes', function() { + group.on(ol.ObjectEventType.CHANGE, listener); + + layer.setVisible(false); + expect(listener.callCount).to.be(1); + + layer.setVisible(true); + expect(listener.callCount).to.be(2); + }); + + }); + describe('constructor (options)', function() { it('accepts options', function() { @@ -313,6 +352,7 @@ describe('ol.layer.Group', function() { }); goog.require('goog.dispose'); +goog.require('ol.ObjectEventType'); goog.require('ol.layer.Layer'); goog.require('ol.layer.Group'); goog.require('ol.source.Source');