From 9d3a4e3c6cad6a60b3e3ba2d292d0e49188ccd33 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Fri, 6 Dec 2013 10:54:09 -0700 Subject: [PATCH 1/8] Add beforechange event type and provide key with change events If you know ahead of time that you only want to listen for changes for a specific property, the foo:change type events can be useful. If you want to listen for changes on all properties, the change event becomes more useful if it provides information on what changed. And the beforechange event allows listeners to access values before they change. --- src/ol/object.js | 27 ++++++++++++++++++- test/spec/ol/object.test.js | 52 ++++++++++++++++++++++++++++++++++--- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/src/ol/object.js b/src/ol/object.js index 4bca4e75ca..cbef3af6c6 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -10,6 +10,7 @@ goog.provide('ol.ObjectEventType'); goog.require('goog.array'); goog.require('goog.events'); +goog.require('goog.events.Event'); goog.require('goog.functions'); goog.require('goog.object'); goog.require('ol.Observable'); @@ -19,11 +20,34 @@ goog.require('ol.Observable'); * @enum {string} */ ol.ObjectEventType = { + BEFORECHANGE: 'beforechange', CHANGE: 'change' }; +/** + * Object representing a property change event. + * + * @param {string} type The event type. + * @param {string} key The property name. + * @extends {goog.events.Event} + * @constructor + */ +ol.ObjectEvent = function(type, key) { + goog.base(this, type); + + /** + * The name of the property whose value is changing. + * @type {string} + */ + this.key = key; + +}; +goog.inherits(ol.ObjectEvent, goog.events.Event); + + + /** * @constructor * @param {ol.Object} target @@ -331,7 +355,7 @@ ol.Object.prototype.notify = function(key) { ol.Object.prototype.notifyInternal_ = function(key) { var eventType = ol.Object.getChangeEventType(key); this.dispatchEvent(eventType); - this.dispatchEvent(ol.ObjectEventType.CHANGE); + this.dispatchEvent(new ol.ObjectEvent(ol.ObjectEventType.CHANGE, key)); }; @@ -342,6 +366,7 @@ ol.Object.prototype.notifyInternal_ = function(key) { * @todo stability experimental */ ol.Object.prototype.set = function(key, value) { + this.dispatchEvent(new ol.ObjectEvent(ol.ObjectEventType.BEFORECHANGE, key)); var accessors = ol.Object.getAccessors(this); if (accessors.hasOwnProperty(key)) { var accessor = accessors[key]; diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index 1c59d6f273..9a53c6d40f 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -121,7 +121,11 @@ describe('ol.Object', function() { it('dispatches generic change events to bound objects', function() { o.notify('k'); - expect(listener2).to.be.called(); + expect(listener2.calledOnce).to.be(true); + var args = listener2.firstCall.args; + expect(args).to.have.length(1); + var event = args[0]; + expect(event.key).to.be('k'); }); it('dispatches events to bound objects', function() { @@ -157,7 +161,25 @@ describe('ol.Object', function() { it('dispatches generic change events to object', function() { o.set('k', 1); - expect(listener2).to.be.called(); + expect(listener2.calledOnce).to.be(true); + var args = listener2.firstCall.args; + expect(args).to.have.length(1); + var event = args[0]; + expect(event.key).to.be('k'); + }); + + it('dispatches beforechange events to object', function() { + o.set('k', 1); + + var oldValue; + var beforeListener = sinon.spy(function(event) { + oldValue = o2.get(event.key); + }); + o.on('beforechange', beforeListener); + + o.set('k', 2); + expect(beforeListener.calledOnce).to.be(true); + expect(oldValue).to.be(1); }); it('dispatches events to bound object', function() { @@ -175,8 +197,32 @@ describe('ol.Object', function() { it('dispatches generic change events to object bound to', function() { o2.set('k', 2); - expect(listener2).to.be.called(); + expect(listener2.calledOnce).to.be(true); + var args = listener2.firstCall.args; + expect(args).to.have.length(1); + var event = args[0]; + expect(event.key).to.be('k'); }); + + it('dispatches beforechange before changing bound objects', function() { + o2.set('k', 1); + + var oldValue; + var beforeListener = sinon.spy(function(event) { + oldValue = o2.get(event.key); + }); + o.on('beforechange', beforeListener); + + o2.set('k', 2); + expect(beforeListener.calledOnce).to.be(true); + var args = listener2.firstCall.args; + expect(args).to.have.length(1); + var event = args[0]; + expect(event.key).to.be('k'); + + expect(oldValue).to.be(1); + }); + }); describe('bind', function() { From 153cb307e0bd3fb45fdec7bb9402905d6572c186 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Mon, 9 Dec 2013 12:14:07 -0700 Subject: [PATCH 2/8] Handle beforechange events for bound properties --- src/ol/object.js | 47 ++++++++++++++++ test/spec/ol/object.test.js | 103 +++++++++++++++++++++++++++++++++++- 2 files changed, 149 insertions(+), 1 deletion(-) diff --git a/src/ol/object.js b/src/ol/object.js index cbef3af6c6..c8dc0febdf 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -118,6 +118,13 @@ ol.Object = function(opt_values) { */ this.values_ = {}; + /** + * Lookup of beforechange listener keys. + * @type {Object.} + * @private + */ + this.beforeChangeListeners_ = {}; + if (goog.isDef(opt_values)) { this.setValues(opt_values); } @@ -240,11 +247,20 @@ ol.Object.getSetterName = function(key) { ol.Object.prototype.bindTo = function(key, target, opt_targetKey) { var targetKey = opt_targetKey || key; this.unbind(key); + + // listen for change:targetkey events var eventType = ol.Object.getChangeEventType(targetKey); var listeners = ol.Object.getListeners(this); listeners[key] = goog.events.listen(target, eventType, function() { this.notifyInternal_(key); }, undefined, this); + + // listen for beforechange events and relay if key matches + this.beforeChangeListeners_[key] = goog.events.listen(target, + ol.ObjectEventType.BEFORECHANGE, + this.createBeforeChangeListener_(key, targetKey), + undefined, this); + var accessor = new ol.ObjectAccessor(target, targetKey); var accessors = ol.Object.getAccessors(this); accessors[key] = accessor; @@ -253,6 +269,30 @@ ol.Object.prototype.bindTo = function(key, target, opt_targetKey) { }; +/** + * Create a listener for beforechange events on a target object. This listener + * will relay events on this object if the event key matches the provided target + * key. + * @param {string} key The key on this object whose value will be changing. + * @param {string} targetKey The key on the target object. + * @return {function(this: ol.Object, ol.ObjectEvent)} Listener. + * @private + */ +ol.Object.prototype.createBeforeChangeListener_ = function(key, targetKey) { + /** + * Conditionally relay beforechange events if event key matches target key. + * @param {ol.ObjectEvent} event The beforechange event from the target. + * @this {ol.Object} + */ + return function(event) { + if (event.key === targetKey) { + this.dispatchEvent( + new ol.ObjectEvent(ol.ObjectEventType.BEFORECHANGE, key)); + } + }; +}; + + /** * Gets a value. * @param {string} key Key name. @@ -422,6 +462,13 @@ ol.Object.prototype.unbind = function(key) { delete accessors[key]; this.values_[key] = value; } + + // unregister any beforechange listener + var listenerKey = this.beforeChangeListeners_[key]; + if (listenerKey) { + goog.events.unlistenByKey(listenerKey); + delete this.beforeChangeListeners_[key]; + } }; diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index 9a53c6d40f..ff5a7660a1 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -215,7 +215,7 @@ describe('ol.Object', function() { o2.set('k', 2); expect(beforeListener.calledOnce).to.be(true); - var args = listener2.firstCall.args; + var args = beforeListener.firstCall.args; expect(args).to.have.length(1); var event = args[0]; expect(event.key).to.be('k'); @@ -223,6 +223,54 @@ describe('ol.Object', function() { expect(oldValue).to.be(1); }); + it('relays beforechange events from bound objects', function() { + var target = new ol.Object({ + foo: 'original value' + }); + var object = new ol.Object(); + object.bindTo('foo', target); + + var oldValue; + var beforeListener = sinon.spy(function(event) { + oldValue = object.get(event.key); + }); + object.on('beforechange', beforeListener); + + target.set('foo', 'new value'); + expect(beforeListener.calledOnce).to.be(true); + var args = beforeListener.firstCall.args; + expect(args).to.have.length(1); + var event = args[0]; + expect(event.key).to.be('foo'); + + expect(oldValue).to.be('original value'); + expect(object.get('foo')).to.be('new value'); + }); + + it('relays beforechange events when bound with a new key', function() { + var target = new ol.Object({ + foo: 'original value' + }); + var object = new ol.Object(); + object.bindTo('bar', target, 'foo'); + + var oldValue; + var beforeListener = sinon.spy(function(event) { + oldValue = object.get(event.key); + }); + object.on('beforechange', beforeListener); + + target.set('foo', 'new value'); + expect(beforeListener.calledOnce).to.be(true); + var args = beforeListener.firstCall.args; + expect(args).to.have.length(1); + var event = args[0]; + expect(event.key).to.be('bar'); + + expect(oldValue).to.be('original value'); + expect(object.get('bar')).to.be('new value'); + }); + }); describe('bind', function() { @@ -302,6 +350,59 @@ describe('ol.Object', function() { expect(o.get('k')).to.eql(1); expect(o2.get('k')).to.eql(2); }); + + it('stops relaying beforechange events', function() { + var target = new ol.Object({ + foo: 'original value' + }); + var object = new ol.Object(); + object.bindTo('foo', target); + + var listener = sinon.spy(); + object.on('beforechange', listener); + + target.set('foo', 'new value'); + expect(listener.calledOnce).to.be(true); + var call = listener.firstCall; + expect(call.args).to.have.length(1); + expect(call.args[0].key).to.be('foo'); + + object.unbind('foo'); + target.set('foo', 'another new value'); + expect(listener.calledOnce).to.be(true); + + expect(object.get('foo')).to.be('new value'); + }); + + it('selectively stops relaying beforechange events', function() { + var target = new ol.Object({ + foo: 'original foo', + bar: 'original bar' + }); + var object = new ol.Object(); + object.bindTo('foo', target); + object.bindTo('bar', target); + + var listener = sinon.spy(); + object.on('beforechange', listener); + + target.set('foo', 'new foo'); + expect(listener.calledOnce).to.be(true); + + target.set('bar', 'new bar'); + expect(listener.callCount).to.be(2); + + object.unbind('foo'); + target.set('foo', 'another new foo'); + expect(listener.callCount).to.be(2); + + target.set('bar', 'another new bar'); + expect(listener.callCount).to.be(3); + var lastCall = listener.getCall(2); + expect(lastCall.args).to.have.length(1); + expect(lastCall.args[0].key).to.be('bar'); + }); + }); describe('unbindAll', function() { From 178377697603f079c2826017f1a602680e47b71b Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 11:36:04 -0700 Subject: [PATCH 3/8] Getting explicit about which type --- test/spec/ol/object.test.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index ff5a7660a1..b18f84401a 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -106,7 +106,7 @@ describe('ol.Object', function() { goog.events.listen(o, 'change:k', listener1); listener2 = sinon.spy(); - goog.events.listen(o, 'change', listener2); + goog.events.listen(o, ol.ObjectEventType.CHANGE, listener2); var o2 = new ol.Object(); o2.bindTo('k', o); @@ -143,7 +143,7 @@ describe('ol.Object', function() { goog.events.listen(o, 'change:k', listener1); listener2 = sinon.spy(); - goog.events.listen(o, 'change', listener2); + goog.events.listen(o, ol.ObjectEventType.CHANGE, listener2); o2 = new ol.Object(); o2.bindTo('k', o); @@ -175,7 +175,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = o2.get(event.key); }); - o.on('beforechange', beforeListener); + o.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); o.set('k', 2); expect(beforeListener.calledOnce).to.be(true); @@ -211,7 +211,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = o2.get(event.key); }); - o.on('beforechange', beforeListener); + o.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); o2.set('k', 2); expect(beforeListener.calledOnce).to.be(true); @@ -234,7 +234,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = object.get(event.key); }); - object.on('beforechange', beforeListener); + object.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); target.set('foo', 'new value'); expect(beforeListener.calledOnce).to.be(true); @@ -258,7 +258,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = object.get(event.key); }); - object.on('beforechange', beforeListener); + object.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); target.set('foo', 'new value'); expect(beforeListener.calledOnce).to.be(true); @@ -359,7 +359,7 @@ describe('ol.Object', function() { object.bindTo('foo', target); var listener = sinon.spy(); - object.on('beforechange', listener); + object.on(ol.ObjectEventType.BEFORECHANGE, listener); target.set('foo', 'new value'); expect(listener.calledOnce).to.be(true); @@ -384,7 +384,7 @@ describe('ol.Object', function() { object.bindTo('bar', target); var listener = sinon.spy(); - object.on('beforechange', listener); + object.on(ol.ObjectEventType.BEFORECHANGE, listener); target.set('foo', 'new foo'); expect(listener.calledOnce).to.be(true); @@ -714,3 +714,4 @@ describe('ol.Object', function() { goog.require('goog.events'); goog.require('ol.Object'); +goog.require('ol.ObjectEventType'); From 17e91feb522456e5f9a0cb2c0b4babb69a57aabb Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 12:23:05 -0700 Subject: [PATCH 4/8] Listen for property changes in layer group This avoids a future bug when the ol.ObjectEventType.CHANGE value becomes something different than the goog.events.EventType.CHANGE value. --- src/ol/layer/layergroup.js | 6 ++-- test/spec/ol/layer/layergroup.test.js | 46 +++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index 9fd4b6c5b0..c2c0940786 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -3,13 +3,13 @@ goog.provide('ol.layer.Group'); goog.require('goog.array'); goog.require('goog.asserts'); goog.require('goog.events'); -goog.require('goog.events.EventType'); goog.require('goog.math'); goog.require('goog.object'); goog.require('ol.Collection'); goog.require('ol.CollectionEvent'); goog.require('ol.CollectionEventType'); goog.require('ol.Object'); +goog.require('ol.ObjectEventType'); goog.require('ol.layer.Base'); goog.require('ol.source.State'); @@ -104,7 +104,7 @@ ol.layer.Group.prototype.handleLayersChanged_ = function(event) { for (i = 0, ii = layersArray.length; i < ii; i++) { layer = layersArray[i]; this.listenerKeys_[goog.getUid(layer).toString()] = - goog.events.listen(layer, goog.events.EventType.CHANGE, + goog.events.listen(layer, ol.ObjectEventType.CHANGE, this.handleLayerChange_, false, this); } } @@ -120,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, ol.ObjectEventType.CHANGE, this.handleLayerChange_, false, this); this.dispatchChangeEvent(); }; diff --git a/test/spec/ol/layer/layergroup.test.js b/test/spec/ol/layer/layergroup.test.js index cefa283374..14263e7e7f 100644 --- a/test/spec/ol/layer/layergroup.test.js +++ b/test/spec/ol/layer/layergroup.test.js @@ -63,7 +63,7 @@ describe('ol.layer.Group', function() { }); - describe('change event', function() { + describe('generic change event', function() { var layer, group, listener; beforeEach(function() { @@ -84,14 +84,14 @@ describe('ol.layer.Group', function() { }); it('is dispatched by the group when layer opacity changes', function() { - group.on(ol.ObjectEventType.CHANGE, listener); + group.on(goog.events.EventType.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); + group.on(goog.events.EventType.CHANGE, listener); layer.setVisible(false); expect(listener.callCount).to.be(1); @@ -102,6 +102,45 @@ describe('ol.layer.Group', function() { }); + describe('property 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 group opacity changes', function() { + group.on(ol.ObjectEventType.CHANGE, listener); + + group.setOpacity(0.5); + expect(listener.calledOnce).to.be(true); + }); + + it('is dispatched by the group when group visibility changes', function() { + group.on(ol.ObjectEventType.CHANGE, listener); + + group.setVisible(false); + expect(listener.callCount).to.be(1); + + group.setVisible(true); + expect(listener.callCount).to.be(2); + }); + + }); + describe('constructor (options)', function() { it('accepts options', function() { @@ -352,6 +391,7 @@ describe('ol.layer.Group', function() { }); goog.require('goog.dispose'); +goog.require('goog.events.EventType'); goog.require('ol.ObjectEventType'); goog.require('ol.layer.Layer'); goog.require('ol.layer.Group'); From c8985b9906771eeb2193450d2f768f3e962730b9 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 16:12:05 -0700 Subject: [PATCH 5/8] Using unique event type values for distinct events Any event target can be used to dispatch generic goog.events.Event instances with an arbitrary type. In cases where we dispatch custom events, we should not use type values that collide with those used for generic events (at least internally). This allows listeners a better chance of knowing what kind of argument they will receive. As subsequent change will clean up the enumeration and add a bit more consistency. --- examples/device-orientation.js | 12 +++++++----- examples/geolocation.js | 2 +- src/ol/layer/layergroup.js | 8 +++++--- src/ol/map.js | 36 ++++++++++++++++++++++++++-------- src/ol/object.js | 3 ++- 5 files changed, 43 insertions(+), 18 deletions(-) diff --git a/examples/device-orientation.js b/examples/device-orientation.js index b464d2e958..e70f419630 100644 --- a/examples/device-orientation.js +++ b/examples/device-orientation.js @@ -25,11 +25,13 @@ var deviceOrientation = new ol.DeviceOrientation(); var track = new ol.dom.Input(document.getElementById('track')); track.bindTo('checked', deviceOrientation, 'tracking'); -deviceOrientation.on('change', function(event) { - document.getElementById('alpha').innerHTML = event.target.getAlpha(); - document.getElementById('beta').innerHTML = event.target.getBeta(); - document.getElementById('gamma').innerHTML = event.target.getGamma(); - document.getElementById('heading').innerHTML = event.target.getHeading(); +deviceOrientation.on('propertychange', function(event) { + // event.key is the changed property name + var key = event.key; + var element = document.getElementById(key); + if (element) { + element.innerHTML = deviceOrientation.get(key); + } }); // tilt the map diff --git a/examples/geolocation.js b/examples/geolocation.js index 7fb3fe83f2..fc0d2ac62e 100644 --- a/examples/geolocation.js +++ b/examples/geolocation.js @@ -29,7 +29,7 @@ geolocation.bindTo('projection', map.getView()); var track = new ol.dom.Input(document.getElementById('track')); track.bindTo('checked', geolocation, 'tracking'); -geolocation.on('change', function() { +geolocation.on('propertychange', function() { $('#accuracy').text(geolocation.getAccuracy() + ' [m]'); $('#altitude').text(geolocation.getAltitude() + ' [m]'); $('#altitudeAccuracy').text(geolocation.getAltitudeAccuracy() + ' [m]'); diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index c2c0940786..2afe59ca8e 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -3,6 +3,7 @@ goog.provide('ol.layer.Group'); goog.require('goog.array'); goog.require('goog.asserts'); goog.require('goog.events'); +goog.require('goog.events.EventType'); goog.require('goog.math'); goog.require('goog.object'); goog.require('ol.Collection'); @@ -104,7 +105,8 @@ ol.layer.Group.prototype.handleLayersChanged_ = function(event) { for (i = 0, ii = layersArray.length; i < ii; i++) { layer = layersArray[i]; this.listenerKeys_[goog.getUid(layer).toString()] = - goog.events.listen(layer, ol.ObjectEventType.CHANGE, + goog.events.listen(layer, + [ol.ObjectEventType.CHANGE, goog.events.EventType.CHANGE], this.handleLayerChange_, false, this); } } @@ -120,8 +122,8 @@ 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, ol.ObjectEventType.CHANGE, this.handleLayerChange_, false, - this); + layer, [ol.ObjectEventType.CHANGE, goog.events.EventType.CHANGE], + this.handleLayerChange_, false, this); this.dispatchChangeEvent(); }; diff --git a/src/ol/map.js b/src/ol/map.js index c6d2f2fcf8..d8ced68de7 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -39,6 +39,7 @@ goog.require('ol.MapBrowserEventHandler'); goog.require('ol.MapEvent'); goog.require('ol.MapEventType'); goog.require('ol.Object'); +goog.require('ol.ObjectEvent'); goog.require('ol.ObjectEventType'); goog.require('ol.Pixel'); goog.require('ol.PostRenderFunction'); @@ -208,9 +209,9 @@ ol.Map = function(options) { /** * @private - * @type {goog.events.Key} + * @type {Array.} */ - this.layerGroupPropertyListenerKey_ = null; + this.layerGroupPropertyListenerKeys_ = null; /** * @private @@ -899,7 +900,18 @@ ol.Map.prototype.handleViewChanged_ = function() { * @param {goog.events.Event} event Event. * @private */ +ol.Map.prototype.handleLayerGroupMemberChanged_ = function(event) { + goog.asserts.assertInstanceof(event, goog.events.Event); + this.render(); +}; + + +/** + * @param {ol.ObjectEvent} event Event. + * @private + */ ol.Map.prototype.handleLayerGroupPropertyChanged_ = function(event) { + goog.asserts.assertInstanceof(event, ol.ObjectEvent); this.render(); }; @@ -908,15 +920,23 @@ ol.Map.prototype.handleLayerGroupPropertyChanged_ = function(event) { * @private */ ol.Map.prototype.handleLayerGroupChanged_ = function() { - if (!goog.isNull(this.layerGroupPropertyListenerKey_)) { - goog.events.unlistenByKey(this.layerGroupPropertyListenerKey_); - this.layerGroupPropertyListenerKey_ = null; + if (!goog.isNull(this.layerGroupPropertyListenerKeys_)) { + var length = this.layerGroupPropertyListenerKeys_.length; + for (var i = 0; i < length; ++i) { + goog.events.unlistenByKey(this.layerGroupPropertyListenerKeys_[i]); + } + this.layerGroupPropertyListenerKeys_ = null; } var layerGroup = this.getLayerGroup(); if (goog.isDefAndNotNull(layerGroup)) { - this.layerGroupPropertyListenerKey_ = goog.events.listen( - layerGroup, ol.ObjectEventType.CHANGE, - this.handleLayerGroupPropertyChanged_, false, this); + this.layerGroupPropertyListenerKeys_ = [ + goog.events.listen( + layerGroup, ol.ObjectEventType.CHANGE, + this.handleLayerGroupPropertyChanged_, false, this), + goog.events.listen( + layerGroup, goog.events.EventType.CHANGE, + this.handleLayerGroupMemberChanged_, false, this) + ]; } this.render(); }; diff --git a/src/ol/object.js b/src/ol/object.js index c8dc0febdf..40e1fd65b3 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -6,6 +6,7 @@ */ goog.provide('ol.Object'); +goog.provide('ol.ObjectEvent'); goog.provide('ol.ObjectEventType'); goog.require('goog.array'); @@ -21,7 +22,7 @@ goog.require('ol.Observable'); */ ol.ObjectEventType = { BEFORECHANGE: 'beforechange', - CHANGE: 'change' + CHANGE: 'propertychange' }; From 625007f3644e3d00982ea8ada1c4571d0511c679 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 16:19:59 -0700 Subject: [PATCH 6/8] Make enum property name like its value Where an enum value is used as an event type, it should be alllowercase (to follow DOM events). Property names should be ALLUPPERCASE in this case (just as camelCase and PascalCase are converted to CONSTANT_CASE). --- src/ol/layer/layergroup.js | 4 ++-- src/ol/map.js | 4 ++-- src/ol/object.js | 5 +++-- test/spec/ol/layer/layer.test.js | 12 ++++++------ test/spec/ol/layer/layergroup.test.js | 4 ++-- test/spec/ol/object.test.js | 4 ++-- 6 files changed, 17 insertions(+), 16 deletions(-) diff --git a/src/ol/layer/layergroup.js b/src/ol/layer/layergroup.js index 2afe59ca8e..6f154543ac 100644 --- a/src/ol/layer/layergroup.js +++ b/src/ol/layer/layergroup.js @@ -106,7 +106,7 @@ ol.layer.Group.prototype.handleLayersChanged_ = function(event) { layer = layersArray[i]; this.listenerKeys_[goog.getUid(layer).toString()] = goog.events.listen(layer, - [ol.ObjectEventType.CHANGE, goog.events.EventType.CHANGE], + [ol.ObjectEventType.PROPERTYCHANGE, goog.events.EventType.CHANGE], this.handleLayerChange_, false, this); } } @@ -122,7 +122,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, [ol.ObjectEventType.CHANGE, goog.events.EventType.CHANGE], + layer, [ol.ObjectEventType.PROPERTYCHANGE, goog.events.EventType.CHANGE], this.handleLayerChange_, false, this); this.dispatchChangeEvent(); }; diff --git a/src/ol/map.js b/src/ol/map.js index d8ced68de7..31e50d9da8 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -889,7 +889,7 @@ ol.Map.prototype.handleViewChanged_ = function() { var view = this.getView(); if (goog.isDefAndNotNull(view)) { this.viewPropertyListenerKey_ = goog.events.listen( - view, ol.ObjectEventType.CHANGE, + view, ol.ObjectEventType.PROPERTYCHANGE, this.handleViewPropertyChanged_, false, this); } this.render(); @@ -931,7 +931,7 @@ ol.Map.prototype.handleLayerGroupChanged_ = function() { if (goog.isDefAndNotNull(layerGroup)) { this.layerGroupPropertyListenerKeys_ = [ goog.events.listen( - layerGroup, ol.ObjectEventType.CHANGE, + layerGroup, ol.ObjectEventType.PROPERTYCHANGE, this.handleLayerGroupPropertyChanged_, false, this), goog.events.listen( layerGroup, goog.events.EventType.CHANGE, diff --git a/src/ol/object.js b/src/ol/object.js index 40e1fd65b3..859e9ad6e8 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -22,7 +22,7 @@ goog.require('ol.Observable'); */ ol.ObjectEventType = { BEFORECHANGE: 'beforechange', - CHANGE: 'propertychange' + PROPERTYCHANGE: 'propertychange' }; @@ -396,7 +396,8 @@ ol.Object.prototype.notify = function(key) { ol.Object.prototype.notifyInternal_ = function(key) { var eventType = ol.Object.getChangeEventType(key); this.dispatchEvent(eventType); - this.dispatchEvent(new ol.ObjectEvent(ol.ObjectEventType.CHANGE, key)); + this.dispatchEvent( + new ol.ObjectEvent(ol.ObjectEventType.PROPERTYCHANGE, key)); }; diff --git a/test/spec/ol/layer/layer.test.js b/test/spec/ol/layer/layer.test.js index 8fd23001e3..4d8f1b516c 100644 --- a/test/spec/ol/layer/layer.test.js +++ b/test/spec/ol/layer/layer.test.js @@ -212,7 +212,7 @@ describe('ol.layer.Layer', function() { it('triggers a change event', function() { var listener = sinon.spy(); - layer.on(ol.ObjectEventType.CHANGE, listener); + layer.on(ol.ObjectEventType.PROPERTYCHANGE, listener); layer.setBrightness(0.5); expect(listener.calledOnce).to.be(true); }); @@ -247,7 +247,7 @@ describe('ol.layer.Layer', function() { it('triggers a change event', function() { var listener = sinon.spy(); - layer.on(ol.ObjectEventType.CHANGE, listener); + layer.on(ol.ObjectEventType.PROPERTYCHANGE, listener); layer.setContrast(43); expect(listener.calledOnce).to.be(true); }); @@ -293,7 +293,7 @@ describe('ol.layer.Layer', function() { it('triggers a change event', function() { var listener = sinon.spy(); - layer.on(ol.ObjectEventType.CHANGE, listener); + layer.on(ol.ObjectEventType.PROPERTYCHANGE, listener); layer.setHue(0.5); expect(listener.calledOnce).to.be(true); }); @@ -324,7 +324,7 @@ describe('ol.layer.Layer', function() { it('triggers a change event', function() { var listener = sinon.spy(); - layer.on(ol.ObjectEventType.CHANGE, listener); + layer.on(ol.ObjectEventType.PROPERTYCHANGE, listener); layer.setOpacity(0.4); expect(listener.calledOnce).to.be(true); }); @@ -360,7 +360,7 @@ describe('ol.layer.Layer', function() { it('triggers a change event', function() { var listener = sinon.spy(); - layer.on(ol.ObjectEventType.CHANGE, listener); + layer.on(ol.ObjectEventType.PROPERTYCHANGE, listener); layer.setSaturation(42); expect(listener.calledOnce).to.be(true); }); @@ -393,7 +393,7 @@ describe('ol.layer.Layer', function() { it('fires a change event', function() { var listener = sinon.spy(); - layer.on(ol.ObjectEventType.CHANGE, listener); + layer.on(ol.ObjectEventType.PROPERTYCHANGE, listener); layer.setVisible(false); expect(listener.callCount).to.be(1); diff --git a/test/spec/ol/layer/layergroup.test.js b/test/spec/ol/layer/layergroup.test.js index 14263e7e7f..d3a29cfd15 100644 --- a/test/spec/ol/layer/layergroup.test.js +++ b/test/spec/ol/layer/layergroup.test.js @@ -123,14 +123,14 @@ describe('ol.layer.Group', function() { }); it('is dispatched by the group when group opacity changes', function() { - group.on(ol.ObjectEventType.CHANGE, listener); + group.on(ol.ObjectEventType.PROPERTYCHANGE, listener); group.setOpacity(0.5); expect(listener.calledOnce).to.be(true); }); it('is dispatched by the group when group visibility changes', function() { - group.on(ol.ObjectEventType.CHANGE, listener); + group.on(ol.ObjectEventType.PROPERTYCHANGE, listener); group.setVisible(false); expect(listener.callCount).to.be(1); diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index b18f84401a..794a7f0d43 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -106,7 +106,7 @@ describe('ol.Object', function() { goog.events.listen(o, 'change:k', listener1); listener2 = sinon.spy(); - goog.events.listen(o, ol.ObjectEventType.CHANGE, listener2); + goog.events.listen(o, ol.ObjectEventType.PROPERTYCHANGE, listener2); var o2 = new ol.Object(); o2.bindTo('k', o); @@ -143,7 +143,7 @@ describe('ol.Object', function() { goog.events.listen(o, 'change:k', listener1); listener2 = sinon.spy(); - goog.events.listen(o, ol.ObjectEventType.CHANGE, listener2); + goog.events.listen(o, ol.ObjectEventType.PROPERTYCHANGE, listener2); o2 = new ol.Object(); o2.bindTo('k', o); From 69385f4ff0fdadac3a641e8d1aae5918fc1f0f6a Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Wed, 11 Dec 2013 16:26:33 -0700 Subject: [PATCH 7/8] More consistent event types --- src/ol/object.js | 9 +++++---- test/spec/ol/object.test.js | 12 ++++++------ 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/src/ol/object.js b/src/ol/object.js index 859e9ad6e8..96ef0bcb5c 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -21,7 +21,7 @@ goog.require('ol.Observable'); * @enum {string} */ ol.ObjectEventType = { - BEFORECHANGE: 'beforechange', + BEFOREPROPERTYCHANGE: 'beforepropertychange', PROPERTYCHANGE: 'propertychange' }; @@ -258,7 +258,7 @@ ol.Object.prototype.bindTo = function(key, target, opt_targetKey) { // listen for beforechange events and relay if key matches this.beforeChangeListeners_[key] = goog.events.listen(target, - ol.ObjectEventType.BEFORECHANGE, + ol.ObjectEventType.BEFOREPROPERTYCHANGE, this.createBeforeChangeListener_(key, targetKey), undefined, this); @@ -288,7 +288,7 @@ ol.Object.prototype.createBeforeChangeListener_ = function(key, targetKey) { return function(event) { if (event.key === targetKey) { this.dispatchEvent( - new ol.ObjectEvent(ol.ObjectEventType.BEFORECHANGE, key)); + new ol.ObjectEvent(ol.ObjectEventType.BEFOREPROPERTYCHANGE, key)); } }; }; @@ -408,7 +408,8 @@ ol.Object.prototype.notifyInternal_ = function(key) { * @todo stability experimental */ ol.Object.prototype.set = function(key, value) { - this.dispatchEvent(new ol.ObjectEvent(ol.ObjectEventType.BEFORECHANGE, key)); + this.dispatchEvent( + new ol.ObjectEvent(ol.ObjectEventType.BEFOREPROPERTYCHANGE, key)); var accessors = ol.Object.getAccessors(this); if (accessors.hasOwnProperty(key)) { var accessor = accessors[key]; diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index 794a7f0d43..78938d8bbc 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -175,7 +175,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = o2.get(event.key); }); - o.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); + o.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); o.set('k', 2); expect(beforeListener.calledOnce).to.be(true); @@ -211,7 +211,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = o2.get(event.key); }); - o.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); + o.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); o2.set('k', 2); expect(beforeListener.calledOnce).to.be(true); @@ -234,7 +234,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = object.get(event.key); }); - object.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); + object.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); target.set('foo', 'new value'); expect(beforeListener.calledOnce).to.be(true); @@ -258,7 +258,7 @@ describe('ol.Object', function() { var beforeListener = sinon.spy(function(event) { oldValue = object.get(event.key); }); - object.on(ol.ObjectEventType.BEFORECHANGE, beforeListener); + object.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); target.set('foo', 'new value'); expect(beforeListener.calledOnce).to.be(true); @@ -359,7 +359,7 @@ describe('ol.Object', function() { object.bindTo('foo', target); var listener = sinon.spy(); - object.on(ol.ObjectEventType.BEFORECHANGE, listener); + object.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, listener); target.set('foo', 'new value'); expect(listener.calledOnce).to.be(true); @@ -384,7 +384,7 @@ describe('ol.Object', function() { object.bindTo('bar', target); var listener = sinon.spy(); - object.on(ol.ObjectEventType.BEFORECHANGE, listener); + object.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, listener); target.set('foo', 'new foo'); expect(listener.calledOnce).to.be(true); From 20d74810ab511e9ab61039f3c42818606d500480 Mon Sep 17 00:00:00 2001 From: Tim Schaub Date: Thu, 12 Dec 2013 15:08:09 -0700 Subject: [PATCH 8/8] Export a getKey method on ol.ObjectEvent --- examples/device-orientation.js | 4 ++-- src/ol/object.exports | 3 +++ src/ol/object.js | 14 ++++++++++++-- test/spec/ol/object.test.js | 24 ++++++++++++------------ 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/examples/device-orientation.js b/examples/device-orientation.js index e70f419630..318dc6468d 100644 --- a/examples/device-orientation.js +++ b/examples/device-orientation.js @@ -26,8 +26,8 @@ var track = new ol.dom.Input(document.getElementById('track')); track.bindTo('checked', deviceOrientation, 'tracking'); deviceOrientation.on('propertychange', function(event) { - // event.key is the changed property name - var key = event.key; + // event.getKey() is the changed property name + var key = event.getKey(); var element = document.getElementById(key); if (element) { element.innerHTML = deviceOrientation.get(key); diff --git a/src/ol/object.exports b/src/ol/object.exports index 89426170a5..3b9b0d2900 100644 --- a/src/ol/object.exports +++ b/src/ol/object.exports @@ -7,3 +7,6 @@ @exportProperty ol.Object.prototype.setValues @exportProperty ol.Object.prototype.unbind @exportProperty ol.Object.prototype.unbindAll + +@exportSymbol ol.ObjectEvent +@exportProperty ol.ObjectEvent.prototype.getKey diff --git a/src/ol/object.js b/src/ol/object.js index 96ef0bcb5c..a918f8df06 100644 --- a/src/ol/object.js +++ b/src/ol/object.js @@ -41,13 +41,23 @@ ol.ObjectEvent = function(type, key) { /** * The name of the property whose value is changing. * @type {string} + * @private */ - this.key = key; + this.key_ = key; }; goog.inherits(ol.ObjectEvent, goog.events.Event); +/** + * Get the name of the property associated with this event. + * @return {string} Object property name. + */ +ol.ObjectEvent.prototype.getKey = function() { + return this.key_; +}; + + /** * @constructor @@ -286,7 +296,7 @@ ol.Object.prototype.createBeforeChangeListener_ = function(key, targetKey) { * @this {ol.Object} */ return function(event) { - if (event.key === targetKey) { + if (event.getKey() === targetKey) { this.dispatchEvent( new ol.ObjectEvent(ol.ObjectEventType.BEFOREPROPERTYCHANGE, key)); } diff --git a/test/spec/ol/object.test.js b/test/spec/ol/object.test.js index 78938d8bbc..f82bb37a9e 100644 --- a/test/spec/ol/object.test.js +++ b/test/spec/ol/object.test.js @@ -125,7 +125,7 @@ describe('ol.Object', function() { var args = listener2.firstCall.args; expect(args).to.have.length(1); var event = args[0]; - expect(event.key).to.be('k'); + expect(event.getKey()).to.be('k'); }); it('dispatches events to bound objects', function() { @@ -165,7 +165,7 @@ describe('ol.Object', function() { var args = listener2.firstCall.args; expect(args).to.have.length(1); var event = args[0]; - expect(event.key).to.be('k'); + expect(event.getKey()).to.be('k'); }); it('dispatches beforechange events to object', function() { @@ -173,7 +173,7 @@ describe('ol.Object', function() { var oldValue; var beforeListener = sinon.spy(function(event) { - oldValue = o2.get(event.key); + oldValue = o2.get(event.getKey()); }); o.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); @@ -201,7 +201,7 @@ describe('ol.Object', function() { var args = listener2.firstCall.args; expect(args).to.have.length(1); var event = args[0]; - expect(event.key).to.be('k'); + expect(event.getKey()).to.be('k'); }); it('dispatches beforechange before changing bound objects', function() { @@ -209,7 +209,7 @@ describe('ol.Object', function() { var oldValue; var beforeListener = sinon.spy(function(event) { - oldValue = o2.get(event.key); + oldValue = o2.get(event.getKey()); }); o.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); @@ -218,7 +218,7 @@ describe('ol.Object', function() { var args = beforeListener.firstCall.args; expect(args).to.have.length(1); var event = args[0]; - expect(event.key).to.be('k'); + expect(event.getKey()).to.be('k'); expect(oldValue).to.be(1); }); @@ -232,7 +232,7 @@ describe('ol.Object', function() { var oldValue; var beforeListener = sinon.spy(function(event) { - oldValue = object.get(event.key); + oldValue = object.get(event.getKey()); }); object.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); @@ -241,7 +241,7 @@ describe('ol.Object', function() { var args = beforeListener.firstCall.args; expect(args).to.have.length(1); var event = args[0]; - expect(event.key).to.be('foo'); + expect(event.getKey()).to.be('foo'); expect(oldValue).to.be('original value'); expect(object.get('foo')).to.be('new value'); @@ -256,7 +256,7 @@ describe('ol.Object', function() { var oldValue; var beforeListener = sinon.spy(function(event) { - oldValue = object.get(event.key); + oldValue = object.get(event.getKey()); }); object.on(ol.ObjectEventType.BEFOREPROPERTYCHANGE, beforeListener); @@ -265,7 +265,7 @@ describe('ol.Object', function() { var args = beforeListener.firstCall.args; expect(args).to.have.length(1); var event = args[0]; - expect(event.key).to.be('bar'); + expect(event.getKey()).to.be('bar'); expect(oldValue).to.be('original value'); expect(object.get('bar')).to.be('new value'); @@ -365,7 +365,7 @@ describe('ol.Object', function() { expect(listener.calledOnce).to.be(true); var call = listener.firstCall; expect(call.args).to.have.length(1); - expect(call.args[0].key).to.be('foo'); + expect(call.args[0].getKey()).to.be('foo'); object.unbind('foo'); target.set('foo', 'another new value'); @@ -400,7 +400,7 @@ describe('ol.Object', function() { expect(listener.callCount).to.be(3); var lastCall = listener.getCall(2); expect(lastCall.args).to.have.length(1); - expect(lastCall.args[0].key).to.be('bar'); + expect(lastCall.args[0].getKey()).to.be('bar'); }); });