From be8c90012e70c01b0eba6ff46b18323a22edbcdd Mon Sep 17 00:00:00 2001 From: euzuro Date: Fri, 29 Aug 2008 16:28:20 +0000 Subject: [PATCH] Fixing the sizing issues with the popups -- all this time, the 'size' parameter we were passing in to init a popup has been the *content* size, not the actual size of the popup. This confusion has stemmed from the fact that we weren't ever differentiating between the two. Now we are. Rename that parameter 'contentSize' in all the constructors. NOTE that this does not *break* API, it is merely renaming a variable. As such, no one should see any difference... other than the fact that autosized popups now stay their correct size when opened and closed repeatedly. Big thanks to jpulles for finding this bug and filing an accurate, informative, and detailed bug report. Thanks to ahocevar and crschmidt for reviewing. (Closes #1586) git-svn-id: http://svn.openlayers.org/trunk/openlayers@7897 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf --- lib/OpenLayers/Popup.js | 35 +++++++++++++++----------- lib/OpenLayers/Popup/Anchored.js | 21 ++++++++++------ lib/OpenLayers/Popup/AnchoredBubble.js | 9 ++++--- lib/OpenLayers/Popup/Framed.js | 11 ++++---- lib/OpenLayers/Popup/FramedCloud.js | 4 +-- tests/Popup.html | 4 +-- 6 files changed, 49 insertions(+), 35 deletions(-) diff --git a/lib/OpenLayers/Popup.js b/lib/OpenLayers/Popup.js index b1ffab6353..da26f82b9f 100644 --- a/lib/OpenLayers/Popup.js +++ b/lib/OpenLayers/Popup.js @@ -47,6 +47,12 @@ OpenLayers.Popup = OpenLayers.Class({ */ div: null, + /** + * Property: contentSize + * {} the width and height of the content. + */ + contentSize: null, + /** * Property: size * {} the width and height of the popup. @@ -183,21 +189,22 @@ OpenLayers.Popup = OpenLayers.Class({ * an identifier will be automatically generated. * lonlat - {} The position on the map the popup will * be shown. - * size - {} The size of the popup. + * contentSize - {} The size of the content. * contentHTML - {String} An HTML string to display inside the * popup. * closeBox - {Boolean} Whether to display a close box inside * the popup. * closeBoxCallback - {Function} Function to be called on closeBox click. */ - initialize:function(id, lonlat, size, contentHTML, closeBox, closeBoxCallback) { + initialize:function(id, lonlat, contentSize, contentHTML, closeBox, closeBoxCallback) { if (id == null) { id = OpenLayers.Util.createUniqueID(this.CLASS_NAME + "_"); } this.id = id; this.lonlat = lonlat; - this.size = (size != null) ? size + + this.contentSize = (contentSize != null) ? contentSize : new OpenLayers.Size( OpenLayers.Popup.WIDTH, OpenLayers.Popup.HEIGHT); @@ -218,7 +225,7 @@ OpenLayers.Popup = OpenLayers.Class({ "hidden"); var id = this.div.id + "_contentDiv"; - this.contentDiv = OpenLayers.Util.createDiv(id, null, this.size.clone(), + this.contentDiv = OpenLayers.Util.createDiv(id, null, this.contentSize.clone(), null, "relative"); this.contentDiv.className = this.contentDisplayClass; this.groupDiv.appendChild(this.contentDiv); @@ -310,8 +317,8 @@ OpenLayers.Popup = OpenLayers.Class({ } this.moveTo(px); - if (!this.autoSize) { - this.setSize(this.size); + if (!this.autoSize && !this.size) { + this.setSize(this.contentSize); } this.setBackgroundColor(); this.setOpacity(); @@ -399,13 +406,11 @@ OpenLayers.Popup = OpenLayers.Class({ * Used to adjust the size of the popup. * * Parameters: - * size - {} the new size of the popup's contents div - * (in pixels). + * contentSize - {} the new size for the popup's + * contents div (in pixels). */ - setSize:function(size) { - this.size = size; - - var contentSize = this.size.clone(); + setSize:function(contentSize) { + this.size = contentSize.clone(); // if our contentDiv has a css 'padding' set on it by a stylesheet, we // must add that to the desired "size". @@ -433,8 +438,10 @@ OpenLayers.Popup = OpenLayers.Class({ // div itself bigger to take its own padding into effect. this makes // me want to shoot someone, but so it goes. if (OpenLayers.Util.getBrowserName() == "msie") { - contentSize.w += contentDivPadding.left + contentDivPadding.right; - contentSize.h += contentDivPadding.bottom + contentDivPadding.top; + this.contentSize.w += + contentDivPadding.left + contentDivPadding.right; + this.contentSize.h += + contentDivPadding.bottom + contentDivPadding.top; } if (this.div != null) { diff --git a/lib/OpenLayers/Popup/Anchored.js b/lib/OpenLayers/Popup/Anchored.js index a452b9dc27..f8585beb80 100644 --- a/lib/OpenLayers/Popup/Anchored.js +++ b/lib/OpenLayers/Popup/Anchored.js @@ -35,17 +35,18 @@ OpenLayers.Popup.Anchored = * Parameters: * id - {String} * lonlat - {} - * size - {} + * contentSize - {} * contentHTML - {String} * anchor - {Object} Object which must expose a 'size' * and 'offset' (generally an ). * closeBox - {Boolean} * closeBoxCallback - {Function} Function to be called on closeBox click. */ - initialize:function(id, lonlat, size, contentHTML, anchor, closeBox, + initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox, closeBoxCallback) { - var newArguments = new Array(id, lonlat, size, contentHTML, closeBox, - closeBoxCallback); + var newArguments = [ + id, lonlat, contentSize, contentHTML, closeBox, closeBoxCallback + ]; OpenLayers.Popup.prototype.initialize.apply(this, newArguments); this.anchor = (anchor != null) ? anchor @@ -108,9 +109,10 @@ OpenLayers.Popup.Anchored = * APIMethod: setSize * * Parameters: - * size - {} + * contentSize - {} the new size for the popup's + * contents div (in pixels). */ - setSize:function(size) { + setSize:function(contentSize) { OpenLayers.Popup.prototype.setSize.apply(this, arguments); if ((this.lonlat) && (this.map)) { @@ -164,12 +166,15 @@ OpenLayers.Popup.Anchored = */ calculateNewPx:function(px) { var newPx = px.offset(this.anchor.offset); + + //use contentSize if size is not already set + var size = this.size || this.contentSize; var top = (this.relativePosition.charAt(0) == 't'); - newPx.y += (top) ? -this.size.h : this.anchor.size.h; + newPx.y += (top) ? -size.h : this.anchor.size.h; var left = (this.relativePosition.charAt(1) == 'l'); - newPx.x += (left) ? -this.size.w : this.anchor.size.w; + newPx.x += (left) ? -size.w : this.anchor.size.w; return newPx; }, diff --git a/lib/OpenLayers/Popup/AnchoredBubble.js b/lib/OpenLayers/Popup/AnchoredBubble.js index 38c486281f..93e03f2f3c 100644 --- a/lib/OpenLayers/Popup/AnchoredBubble.js +++ b/lib/OpenLayers/Popup/AnchoredBubble.js @@ -28,7 +28,7 @@ OpenLayers.Popup.AnchoredBubble = * Parameters: * id - {String} * lonlat - {} - * size - {} + * contentSize - {} * contentHTML - {String} * anchor - {Object} Object to which we'll anchor the popup. Must expose * a 'size' () and 'offset' () @@ -36,7 +36,7 @@ OpenLayers.Popup.AnchoredBubble = * closeBox - {Boolean} * closeBoxCallback - {Function} Function to be called on closeBox click. */ - initialize:function(id, lonlat, size, contentHTML, anchor, closeBox, + initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox, closeBoxCallback) { this.padding = new OpenLayers.Bounds( @@ -81,9 +81,10 @@ OpenLayers.Popup.AnchoredBubble = * APIMethod: setSize * * Parameters: - * size - {} + * contentSize - {} the new size for the popup's + * contents div (in pixels). */ - setSize:function(size) { + setSize:function(contentSize) { OpenLayers.Popup.Anchored.prototype.setSize.apply(this, arguments); this.setRicoCorners(); diff --git a/lib/OpenLayers/Popup/Framed.js b/lib/OpenLayers/Popup/Framed.js index 1c1fcb22a6..f9e97cfa37 100644 --- a/lib/OpenLayers/Popup/Framed.js +++ b/lib/OpenLayers/Popup/Framed.js @@ -85,7 +85,7 @@ OpenLayers.Popup.Framed = * Parameters: * id - {String} * lonlat - {} - * size - {} + * contentSize - {} * contentHTML - {String} * anchor - {Object} Object to which we'll anchor the popup. Must expose * a 'size' () and 'offset' () @@ -93,7 +93,7 @@ OpenLayers.Popup.Framed = * closeBox - {Boolean} * closeBoxCallback - {Function} Function to be called on closeBox click. */ - initialize:function(id, lonlat, size, contentHTML, anchor, closeBox, + initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox, closeBoxCallback) { OpenLayers.Popup.Anchored.prototype.initialize.apply(this, arguments); @@ -188,9 +188,10 @@ OpenLayers.Popup.Framed = * of the popup has changed. * * Parameters: - * size - {} + * contentSize - {} the new size for the popup's + * contents div (in pixels). */ - setSize:function(size) { + setSize:function(contentSize) { OpenLayers.Popup.Anchored.prototype.setSize.apply(this, arguments); this.updateBlocks(); @@ -296,7 +297,7 @@ OpenLayers.Popup.Framed = this.createBlocks(); } - if (this.relativePosition) { + if (this.size && this.relativePosition) { var position = this.positionBlocks[this.relativePosition]; for (var i = 0; i < position.blocks.length; i++) { diff --git a/lib/OpenLayers/Popup/FramedCloud.js b/lib/OpenLayers/Popup/FramedCloud.js index 18697b217e..39f98a45d3 100644 --- a/lib/OpenLayers/Popup/FramedCloud.js +++ b/lib/OpenLayers/Popup/FramedCloud.js @@ -203,7 +203,7 @@ OpenLayers.Popup.FramedCloud = * Parameters: * id - {String} * lonlat - {} - * size - {} + * contentSize - {} * contentHTML - {String} * anchor - {Object} Object to which we'll anchor the popup. Must expose * a 'size' () and 'offset' () @@ -211,7 +211,7 @@ OpenLayers.Popup.FramedCloud = * closeBox - {Boolean} * closeBoxCallback - {Function} Function to be called on closeBox click. */ - initialize:function(id, lonlat, size, contentHTML, anchor, closeBox, + initialize:function(id, lonlat, contentSize, contentHTML, anchor, closeBox, closeBoxCallback) { this.imageSrc = OpenLayers.Util.getImagesLocation() + 'cloud-popup-relative.png'; diff --git a/tests/Popup.html b/tests/Popup.html index 7b322c54da..80f07fcdd2 100644 --- a/tests/Popup.html +++ b/tests/Popup.html @@ -16,7 +16,7 @@ t.ok(OpenLayers.String.startsWith(popup.id, "OpenLayers.Popup"), "valid default popupid"); var firstID = popup.id; - t.ok(popup.size.equals(size), "good default popup.size"); + t.ok(popup.contentSize.equals(size), "good default popup.size"); t.eq(popup.contentHTML, null, "good default popup.contentHTML"); t.eq(popup.backgroundColor, OpenLayers.Popup.COLOR, "good default popup.backgroundColor"); t.eq(popup.opacity, OpenLayers.Popup.OPACITY, "good default popup.opacity"); @@ -54,7 +54,7 @@ t.ok( popup instanceof OpenLayers.Popup, "new OpenLayers.Popup returns Popup object" ); t.eq(popup.id, id, "popup.id set correctly"); t.ok(popup.lonlat.equals(ll), "popup.lonlat set correctly"); - t.ok(popup.size.equals(sz), "popup.size set correctly"); + t.ok(popup.contentSize.equals(sz), "popup.size set correctly"); t.eq(popup.contentHTML, content, "contentHTML porpoerty of set correctly"); // test that a browser event is registered on click on popup closebox