From 7f39b22161ad63ceb581dd925b2c0fde0a82a12b Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Mon, 23 Jan 2017 16:19:10 +0100 Subject: [PATCH 1/2] Add tests for ol.Overlay.setVisible --- test/spec/ol/overlay.test.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/test/spec/ol/overlay.test.js b/test/spec/ol/overlay.test.js index 4323b7ce58..cb94abc9b4 100644 --- a/test/spec/ol/overlay.test.js +++ b/test/spec/ol/overlay.test.js @@ -75,4 +75,29 @@ describe('ol.Overlay', function() { }); + describe('#setVisible()', function() { + var overlay, target; + + beforeEach(function() { + target = document.createElement('div'); + }); + afterEach(function() { + map.removeOverlay(overlay); + }); + + it('changes the CSS display value', function() { + overlay = new ol.Overlay({ + element: target, + position: [0, 0] + }); + map.addOverlay(overlay); + expect(overlay.element_.style.display).to.be('none'); + overlay.setVisible(true); + expect(overlay.element_.style.display).not.to.be('none'); + overlay.setVisible(false); + expect(overlay.element_.style.display).to.be('none'); + }); + + }); + }); From 6605669cb6e39fe096dc6eb9e771b85ceb38652e Mon Sep 17 00:00:00 2001 From: Frederic Junod Date: Mon, 23 Jan 2017 16:48:36 +0100 Subject: [PATCH 2/2] Be more tolerant of map and position value The `map` property type is `ol.Map|undefined` but `null` is set when removing the overlay from the map: https://github.com/openlayers/openlayers/blob/v3.20.1/src/ol/map.js#L453 This raise an exception with: ```js map.removeOverlay(overlay); overlay.setPosition(undefined); ``` Because `map === undefined` is used in `ol.Overlay.setPosition` --- src/ol/overlay.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ol/overlay.js b/src/ol/overlay.js index b858e66991..64c6fa3cff 100644 --- a/src/ol/overlay.js +++ b/src/ol/overlay.js @@ -271,7 +271,7 @@ ol.Overlay.prototype.handleOffsetChanged = function() { */ ol.Overlay.prototype.handlePositionChanged = function() { this.updatePixelPosition(); - if (this.get(ol.Overlay.Property_.POSITION) !== undefined && this.autoPan) { + if (this.get(ol.Overlay.Property_.POSITION) && this.autoPan) { this.panIntoView_(); } }; @@ -339,7 +339,7 @@ ol.Overlay.prototype.setPosition = function(position) { ol.Overlay.prototype.panIntoView_ = function() { var map = this.getMap(); - if (map === undefined || !map.getTargetElement()) { + if (!map || !map.getTargetElement()) { return; } @@ -442,7 +442,7 @@ ol.Overlay.prototype.setVisible = function(visible) { ol.Overlay.prototype.updatePixelPosition = function() { var map = this.getMap(); var position = this.getPosition(); - if (map === undefined || !map.isRendered() || position === undefined) { + if (!map || !map.isRendered() || !position) { this.setVisible(false); return; }