Merge pull request #1649 from tonio/geometrycollection_change

Make GeometryCollection propagate change events of its geometries.

src/ol/geom/geometrycollection.js

@@ -2,6 +2,8 @@ goog.provide('ol.geom.GeometryCollection');
 
 goog.require('goog.array');
 goog.require('goog.asserts');
+goog.require('goog.events');
+goog.require('goog.events.EventType');
 goog.require('goog.object');
 goog.require('ol.extent');
 goog.require('ol.geom.Geometry');
@@ -25,6 +27,7 @@ ol.geom.GeometryCollection = function(opt_geometries) {
    */
   this.geometries_ = goog.isDef(opt_geometries) ? opt_geometries : null;
 
+  this.listenGeometriesChange_();
 };
 goog.inherits(ol.geom.GeometryCollection, ol.geom.Geometry);
 
@@ -44,6 +47,38 @@ ol.geom.GeometryCollection.cloneGeometries_ = function(geometries) {
 };
 
 
+/**
+ * @private
+ */
+ol.geom.GeometryCollection.prototype.unlistenGeometriesChange_ = function() {
+  var i, ii;
+  if (goog.isNull(this.geometries_)) {
+    return;
+  }
+  for (i = 0, ii = this.geometries_.length; i < ii; ++i) {
+    goog.events.unlisten(
+        this.geometries_[i], goog.events.EventType.CHANGE,
+        this.dispatchChangeEvent, false, this);
+  }
+};
+
+
+/**
+ * @private
+ */
+ol.geom.GeometryCollection.prototype.listenGeometriesChange_ = function() {
+  var i, ii;
+  if (goog.isNull(this.geometries_)) {
+    return;
+  }
+  for (i = 0, ii = this.geometries_.length; i < ii; ++i) {
+    goog.events.listen(
+        this.geometries_[i], goog.events.EventType.CHANGE,
+        this.dispatchChangeEvent, false, this);
+  }
+};
+
+
 /**
  * @inheritDoc
  */
@@ -199,7 +234,9 @@ ol.geom.GeometryCollection.prototype.setGeometries = function(geometries) {
  * @param {Array.<ol.geom.Geometry>} geometries Geometries.
  */
 ol.geom.GeometryCollection.prototype.setGeometriesArray = function(geometries) {
+  this.unlistenGeometriesChange_();
   this.geometries_ = geometries;
+  this.listenGeometriesChange_();
   this.dispatchChangeEvent();
 };
 
@@ -215,3 +252,12 @@ ol.geom.GeometryCollection.prototype.transform = function(transformFn) {
   }
   this.dispatchChangeEvent();
 };
+
+
+/**
+ * @inheritDoc
+ */
+ol.geom.GeometryCollection.prototype.disposeInternal = function() {
+  this.unlistenGeometriesChange_();
+  goog.base(this, 'disposeInternal');
+};

test/spec/ol/geom/geometrycollection.test.js

@@ -9,15 +9,45 @@ describe('ol.geom.GeometryCollection', function() {
 
   describe('constructor', function() {
 
+    var line, multi, point, poly;
+    beforeEach(function() {
+      point = new ol.geom.Point([10, 20]);
+      line = new ol.geom.LineString([[10, 20], [30, 40]]);
+      poly = new ol.geom.Polygon([outer, inner1, inner2]);
+      multi = new ol.geom.GeometryCollection([point, line, poly]);
+    });
+
     it('creates a geometry collection from an array of geometries', function() {
-      var point = new ol.geom.Point([10, 20]);
-      var line = new ol.geom.LineString([[10, 20], [30, 40]]);
-      var poly = new ol.geom.Polygon([outer, inner1, inner2]);
-      var multi = new ol.geom.GeometryCollection([point, line, poly]);
       expect(multi).to.be.a(ol.geom.GeometryCollection);
       expect(multi).to.be.a(ol.geom.Geometry);
     });
 
+    it('fires a change event when one of its component changes',
+        function(done) {
+          multi.on('change', function() {
+            done();
+          });
+          point.setCoordinates([10, 10]);
+        }
+    );
+
+    it('deregister old components', function() {
+      multi.setGeometries([poly]);
+      multi.on('change', function() {
+        expect().fail();
+      });
+      point.setCoordinates([10, 10]);
+    });
+
+    it('register new components', function(done) {
+      var point2 = new ol.geom.Point([10, 20]);
+      multi.setGeometriesArray([point2]);
+      multi.on('change', function() {
+        done();
+      });
+      point2.setCoordinates([10, 10]);
+    });
+
   });
 
   describe('#getGeometries', function() {
@@ -97,8 +127,6 @@ describe('ol.geom.GeometryCollection', function() {
     it('fires a change event', function() {
       var listener = sinon.spy();
       multi.on('change', listener);
-      point.setCoordinates([15, 25]);
-      expect(listener.calledOnce).to.be(false);
       multi.setGeometries([point, line, poly]);
       expect(listener.calledOnce).to.be(true);
     });
@@ -106,8 +134,6 @@ describe('ol.geom.GeometryCollection', function() {
     it('updates the extent', function() {
       expect(multi.getExtent()).to.eql([0, 0, 30, 40]);
       line.setCoordinates([[10, 20], [300, 400]]);
-      expect(multi.getExtent()).to.eql([0, 0, 30, 40]);
-      multi.setGeometries([point, line, poly]);
       expect(multi.getExtent()).to.eql([0, 0, 300, 400]);
     });