Merge pull request #1349 from tschaub/fewer-change-events

Clean up layer change event handling.

src/ol/layer/layerbase.js

@@ -71,22 +71,6 @@ ol.layer.Base = function(options) {
       values.minResolution : 0;
 
   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);
-
 };
 goog.inherits(ol.layer.Base, ol.Object);
 
@@ -251,26 +235,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
- */
-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

src/ol/layer/layergroup.js

@@ -70,23 +70,15 @@ 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();
   }
 };
 
 
-/**
- * @inheritDoc
- */
-ol.layer.Group.prototype.handleLayerVisibleChange = function() {
-  this.dispatchChangeEvent();
-};
-
-
 /**
  * @param {goog.events.Event} event Event.
  * @private
@@ -113,7 +105,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 +120,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();
 };

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,25 +358,48 @@ 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);
+    });
+
   });
 
 
   describe('#setVisible', function() {
 
-    it('sets visible property', function() {
+    var layer;
-      var layer = new ol.layer.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);
     });
 
   });
@@ -356,6 +407,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');

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');