diff --git a/externs/olx.js b/externs/olx.js index aa31c2c854..9d36e803f0 100644 --- a/externs/olx.js +++ b/externs/olx.js @@ -309,7 +309,8 @@ olx.MapOptions.prototype.view; /** * Object literal with config options for the overlay. - * @typedef {{element: (Element|undefined), + * @typedef {{id: (number|string|undefined), + * element: (Element|undefined), * offset: (Array.|undefined), * position: (ol.Coordinate|undefined), * positioning: (ol.OverlayPositioning|string|undefined), @@ -323,6 +324,15 @@ olx.MapOptions.prototype.view; olx.OverlayOptions; +/** + * Set the overlay id. The overlay id can be used with the + * {@link ol.Map#getOverlayById} method. + * @type {number|string|undefined} + * @api + */ +olx.OverlayOptions.prototype.id; + + /** * The overlay element. * @type {Element|undefined} diff --git a/src/ol/map.js b/src/ol/map.js index aec1dd1230..10bdaaf288 100644 --- a/src/ol/map.js +++ b/src/ol/map.js @@ -557,9 +557,6 @@ ol.Map.prototype.addOverlayInternal = function(overlay) { this.overlayIdIndex_[id.toString()] = overlay; } overlay.setMap(this); - goog.events.listen( - overlay, ol.Object.getChangeEventType(overlay.getOverlayIdProperty()), - this.handleOverlayIdChange_, false, this); }; @@ -1103,28 +1100,6 @@ ol.Map.prototype.handleTileChange_ = function() { }; -/** - * @param {goog.events.Event} event Event. - * @private - */ -ol.Map.prototype.handleOverlayIdChange_ = function(event) { - var overlay = /** @type {ol.Overlay} */ (event.target); - var id = overlay.getId().toString(); - var oldId = event.oldValue; - if (oldId && oldId != id) { - delete this.overlayIdIndex_[oldId]; - } - if (id in this.overlayIdIndex_) { - if (this.overlayIdIndex_[id] !== overlay) { - delete this.overlayIdIndex_[id]; - this.overlayIdIndex_[id] = overlay; - } - } else { - this.overlayIdIndex_[id] = overlay; - } -}; - - /** * @private */ diff --git a/src/ol/overlay.js b/src/ol/overlay.js index 079be33a5d..3829db1ec4 100644 --- a/src/ol/overlay.js +++ b/src/ol/overlay.js @@ -19,7 +19,6 @@ goog.require('ol.extent'); * @enum {string} */ ol.OverlayProperty = { - ID: 'id', ELEMENT: 'element', MAP: 'map', OFFSET: 'offset', @@ -78,7 +77,7 @@ ol.Overlay = function(options) { * @private * @type {number|string|undefined} */ - this.id_ = undefined; + this.id_ = options.id; /** * @private @@ -195,8 +194,7 @@ ol.Overlay.prototype.getElement = function() { /** - * Get the feature identifier. This is an identifier for the overlay and - * is set explicitly by calling {@link ol.Overlay#setId}. + * Get the feature identifier which is set on constructor. * @return {number|string|undefined} Id. * @api */ @@ -229,15 +227,6 @@ ol.Overlay.prototype.getOffset = function() { }; -/** - * Workaround to overcome circular dependency. - * @return {ol.OverlayProperty} - */ -ol.Overlay.prototype.getOverlayIdProperty = function() { - return ol.OverlayProperty.ID; -}; - - /** * Get the current position of this overlay. * @return {ol.Coordinate|undefined} The spatial point that the overlay is @@ -348,22 +337,6 @@ ol.Overlay.prototype.setElement = function(element) { }; -/** - * Set the feature id. The feature id can be used with the - * {@link ol.Map#getOverlayById} method. - * @param {number|string} id The feature id. - * @observable - * @api - */ -ol.Overlay.prototype.setId = function(id) { - goog.asserts.assert(id !== undefined, 'overlay id should be defined'); - if (id != this.id_) { - this.id_ = id; - this.set(ol.OverlayProperty.ID, id); - } -}; - - /** * Set the map to be associated with this overlay. * @param {ol.Map|undefined} map The map that the overlay is part of. diff --git a/test/spec/ol/map.test.js b/test/spec/ol/map.test.js index cf2cb4bea4..78ca851136 100644 --- a/test/spec/ol/map.test.js +++ b/test/spec/ol/map.test.js @@ -346,80 +346,36 @@ describe('ol.Map', function() { it('returns an overlay by id', function() { overlay = new ol.Overlay({ - element: overlay_target, - position: [0, 0] - }); - overlay.setId('foo'); - map.addOverlay(overlay); - expect(map.getOverlayById('foo')).to.be(overlay); - }); - - it('returns an overlay by id (set after add)', function() { - overlay = new ol.Overlay({ + id: 'foo', element: overlay_target, position: [0, 0] }); map.addOverlay(overlay); - expect(map.getOverlayById('foo')).to.be(null); - overlay.setId('foo'); expect(map.getOverlayById('foo')).to.be(overlay); }); it('returns null when no overlay is found', function() { overlay = new ol.Overlay({ + id: 'foo', element: overlay_target, position: [0, 0] }); - overlay.setId('foo'); map.addOverlay(overlay); expect(map.getOverlayById('bar')).to.be(null); }); it('returns null after removing overlay', function() { overlay = new ol.Overlay({ + id: 'foo', element: overlay_target, position: [0, 0] }); - overlay.setId('foo'); map.addOverlay(overlay); expect(map.getOverlayById('foo')).to.be(overlay); map.removeOverlay(overlay); expect(map.getOverlayById('foo')).to.be(null); }); - it('returns correct overlay after add/remove/add', function() { - expect(map.getOverlayById('foo')).to.be(null); - var first = new ol.Overlay({ - element: overlay_target, - position: [0, 0] - }); - first.setId('foo'); - map.addOverlay(first); - expect(map.getOverlayById('foo')).to.be(first); - map.removeOverlay(first); - expect(map.getOverlayById('foo')).to.be(null); - var second = new ol.Overlay({ - element: overlay_target, - position: [0, 0] - }); - second.setId('foo'); - map.addOverlay(second); - expect(map.getOverlayById('foo')).to.be(second); - }); - - it('returns correct overlay after add/change', function() { - expect(map.getOverlayById('foo')).to.be(null); - overlay = new ol.Overlay({ - element: overlay_target, - position: [0, 0] - }); - overlay.setId('foo'); - map.addOverlay(overlay); - expect(map.getOverlayById('foo')).to.be(overlay); - overlay.setId('bar'); - expect(map.getOverlayById('foo')).to.be(null); - expect(map.getOverlayById('bar')).to.be(overlay); - }); }); }); diff --git a/test/spec/ol/overlay.test.js b/test/spec/ol/overlay.test.js index 45f49e5046..468e58d1da 100644 --- a/test/spec/ol/overlay.test.js +++ b/test/spec/ol/overlay.test.js @@ -41,34 +41,33 @@ describe('ol.Overlay', function() { }); - describe('#setId()', function() { + describe('#getId()', function() { var overlay, target; beforeEach(function() { target = document.createElement('div'); - overlay = new ol.Overlay({ - element: target, - position: [0, 0] - }); - map.addOverlay(overlay); }); afterEach(function() { map.removeOverlay(overlay); }); - it('sets the overlay identifier', function() { + it('returns the overlay identifier', function() { + overlay = new ol.Overlay({ + element: target, + position: [0, 0] + }); + map.addOverlay(overlay); expect(overlay.getId()).to.be(undefined); - overlay.setId('foo'); + map.removeOverlay(overlay); + overlay = new ol.Overlay({ + id: 'foo', + element: target, + position: [0, 0] + }); + map.addOverlay(overlay); expect(overlay.getId()).to.be('foo'); }); - it('accepts a string or number', function() { - overlay.setId('foo'); - expect(overlay.getId()).to.be('foo'); - overlay.setId(2); - expect(overlay.getId()).to.be(2); - }); - }); });