From e6c4fd973a1a4015b8b8eb93d4b53f6901bfeb3d Mon Sep 17 00:00:00 2001 From: Bart van den Eijnden Date: Mon, 4 Nov 2013 19:53:21 +0100 Subject: [PATCH 1/3] allow target to be specified as a string for controls, update the documentation to make more clear what element and target are for --- src/objectliterals.jsdoc | 7 +++++-- src/ol/control/control.js | 5 ++++- test/spec/ol/control/control.test.js | 24 ++++++++++++++++++++++++ 3 files changed, 33 insertions(+), 3 deletions(-) diff --git a/src/objectliterals.jsdoc b/src/objectliterals.jsdoc index 6c1c23e5f1..811a0bf6ce 100644 --- a/src/objectliterals.jsdoc +++ b/src/objectliterals.jsdoc @@ -170,8 +170,11 @@ /** * @typedef {Object} olx.control.ControlOptions - * @property {Element|undefined} element Element. - * @property {Element|undefined} target Target. + * @property {Element|undefined} element The element is the control's container + * element. This only needs to be specified if you're developing a custom + * control. + * @property {Element|string|undefined} target Specify a target if you want the + * control to be rendered outside of the map's viewport. * @todo stability experimental */ diff --git a/src/ol/control/control.js b/src/ol/control/control.js index d596af3ef7..f2912d0c0f 100644 --- a/src/ol/control/control.js +++ b/src/ol/control/control.js @@ -32,7 +32,10 @@ ol.control.Control = function(options) { * @private * @type {Element|undefined} */ - this.target_ = options.target; + this.target_ = goog.isDef(options.target) ? (goog.isString(options.target) ? + goog.dom.getElement(options.target) !== null ? + goog.dom.getElement(options.target) : undefined : options.target) : + undefined; /** * @private diff --git a/test/spec/ol/control/control.test.js b/test/spec/ol/control/control.test.js index 4b4f25bd00..2b988076d2 100644 --- a/test/spec/ol/control/control.test.js +++ b/test/spec/ol/control/control.test.js @@ -24,6 +24,30 @@ describe('ol.control.Control', function() { }); }); +describe('ol.control.Control\'s target', function() { + describe('target as string or element', function() { + it('transforms target from string to element', function() { + var target = goog.dom.createDom('div', {'id': 'mycontrol'}); + document.body.appendChild(target); + var ctrl = new ol.control.Control({target: 'mycontrol'}); + expect(ctrl.target_.id).to.equal('mycontrol'); + goog.dispose(ctrl); + }); + it('accepts element for target', function() { + var target = goog.dom.createDom('div', {'id': 'mycontrol'}); + document.body.appendChild(target); + var ctrl = new ol.control.Control({target: target}); + expect(ctrl.target_.id).to.equal('mycontrol'); + goog.dispose(ctrl); + }); + it('ignores non-existing target id', function() { + var ctrl = new ol.control.Control({target: 'doesnotexist'}); + expect(ctrl.target_).to.equal(undefined); + goog.dispose(ctrl); + }); + }); +}); + goog.require('goog.dispose'); goog.require('goog.dom'); goog.require('goog.dom.TagName'); From 2fc884a3d9ea5e9830db0e11d17481af60ce63de Mon Sep 17 00:00:00 2001 From: Bart van den Eijnden Date: Tue, 5 Nov 2013 16:52:24 +0100 Subject: [PATCH 2/3] simplify the assignment of this.target_ --- src/ol/control/control.js | 10 ++++------ test/spec/ol/control/control.test.js | 2 +- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/ol/control/control.js b/src/ol/control/control.js index f2912d0c0f..4ebab38345 100644 --- a/src/ol/control/control.js +++ b/src/ol/control/control.js @@ -30,12 +30,10 @@ ol.control.Control = function(options) { /** * @private - * @type {Element|undefined} + * @type {?Element} */ - this.target_ = goog.isDef(options.target) ? (goog.isString(options.target) ? - goog.dom.getElement(options.target) !== null ? - goog.dom.getElement(options.target) : undefined : options.target) : - undefined; + this.target_ = goog.isDef(options.target) ? + goog.dom.getElement(options.target) : null; /** * @private @@ -98,7 +96,7 @@ ol.control.Control.prototype.setMap = function(map) { } this.map_ = map; if (!goog.isNull(this.map_)) { - var target = goog.isDef(this.target_) ? + var target = !goog.isNull(this.target_) ? this.target_ : map.getOverlayContainerStopEvent(); goog.dom.appendChild(target, this.element); if (this.handleMapPostrender !== goog.nullFunction) { diff --git a/test/spec/ol/control/control.test.js b/test/spec/ol/control/control.test.js index 2b988076d2..22a555c134 100644 --- a/test/spec/ol/control/control.test.js +++ b/test/spec/ol/control/control.test.js @@ -42,7 +42,7 @@ describe('ol.control.Control\'s target', function() { }); it('ignores non-existing target id', function() { var ctrl = new ol.control.Control({target: 'doesnotexist'}); - expect(ctrl.target_).to.equal(undefined); + expect(ctrl.target_).to.equal(null); goog.dispose(ctrl); }); }); From 498d05a44cb941901fdfd33401ba6db7e697e018 Mon Sep 17 00:00:00 2001 From: Bart van den Eijnden Date: Tue, 5 Nov 2013 17:11:40 +0100 Subject: [PATCH 3/3] remove question mark since all object properties are nullable by default --- src/ol/control/control.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ol/control/control.js b/src/ol/control/control.js index 4ebab38345..ab61fc8c0b 100644 --- a/src/ol/control/control.js +++ b/src/ol/control/control.js @@ -30,7 +30,7 @@ ol.control.Control = function(options) { /** * @private - * @type {?Element} + * @type {Element} */ this.target_ = goog.isDef(options.target) ? goog.dom.getElement(options.target) : null;