From 3007b236210dc6a6bf39c81fd3f534aa87b5fc47 Mon Sep 17 00:00:00 2001 From: euzuro Date: Thu, 28 Aug 2008 20:04:17 +0000 Subject: [PATCH] Bubble out the code that handles the re-sizing of the popup into an APIMethod: updateSize(). Other change is that now instead of using the 'contentHTML' string property to autosize, we will now use the actual contentDiv's 'innerHTML' property. This is possible because in the setContentHTML() function, we switch around the order and set the conentDiv.innerHTML *before* calling updateSize(). Seemingly minor, this change actually does wonders towards distancing us from the horrendous idea that the 'contentHTML' property was in the first place. Now, if users go in and fudge with the contentDiv DOMElement directly, they can still benefit from the auto-sizing. In fact, 'contentHTML' can be totally ignored altogether. joy. All tests pass, including an eyeballing of the acceptance test examples/popupMatrix.html in ff2 and ie7. Mil gracias to cr5 for the speedy review (Closes #1708) git-svn-id: http://svn.openlayers.org/trunk/openlayers@7886 dc9f47b5-9b13-0410-9fdd-eb0c1a62fdaf --- lib/OpenLayers/Popup.js | 153 +++++++++++++++++++++----------------- tests/Popup.html | 2 +- tests/Popup/Anchored.html | 2 +- 3 files changed, 85 insertions(+), 72 deletions(-) diff --git a/lib/OpenLayers/Popup.js b/lib/OpenLayers/Popup.js index c1d390842c..5e920b706d 100644 --- a/lib/OpenLayers/Popup.js +++ b/lib/OpenLayers/Popup.js @@ -55,9 +55,9 @@ OpenLayers.Popup = OpenLayers.Class({ /** * Property: contentHTML - * {String} The HTML that this popup displays. + * {String} An HTML string for this popup to display. */ - contentHTML: "", + contentHTML: null, /** * Property: backgroundColor @@ -184,7 +184,7 @@ OpenLayers.Popup = OpenLayers.Class({ * lonlat - {} The position on the map the popup will * be shown. * size - {} The size of the popup. - * contentHTML - {String} The HTML content to display inside the + * contentHTML - {String} An HTML string to display inside the * popup. * closeBox - {Boolean} Whether to display a close box inside * the popup. @@ -447,6 +447,78 @@ OpenLayers.Popup = OpenLayers.Class({ } }, + /** + * APIMethod: updateSize + * Auto size the popup so that it precisely fits its contents (as + * determined by this.contentDiv.innerHTML). Popup size will, of + * course, be limited by the available space on the current map + */ + updateSize: function() { + + // determine actual render dimensions of the contents by putting its + // contents into a fake contentDiv (for the CSS) and then measuring it + var preparedHTML = "
" + + this.contentDiv.innerHTML + + "
"; + var realSize = OpenLayers.Util.getRenderedDimensions( + preparedHTML, null, { displayClass: this.displayClass } + ); + + // is the "real" size of the div is safe to display in our map? + var safeSize = this.getSafeContentSize(realSize); + + var newSize = null; + if (safeSize.equals(realSize)) { + //real size of content is small enough to fit on the map, + // so we use real size. + newSize = realSize; + + } else { + + //make a new OL.Size object with the clipped dimensions + // set or null if not clipped. + var fixedSize = new OpenLayers.Size(); + fixedSize.w = (safeSize.w < realSize.w) ? safeSize.w : null; + fixedSize.h = (safeSize.h < realSize.h) ? safeSize.h : null; + + if (fixedSize.w && fixedSize.h) { + //content is too big in both directions, so we will use + // max popup size (safeSize), knowing well that it will + // overflow both ways. + newSize = safeSize; + } else { + //content is clipped in only one direction, so we need to + // run getRenderedDimensions() again with a fixed dimension + var clippedSize = OpenLayers.Util.getRenderedDimensions( + preparedHTML, fixedSize, + { displayClass: this.contentDisplayClass } + ); + + //if the clipped size is still the same as the safeSize, + // that means that our content must be fixed in the + // offending direction. If overflow is 'auto', this means + // we are going to have a scrollbar for sure, so we must + // adjust for that. + // + var currentOverflow = OpenLayers.Element.getStyle( + this.contentDiv, "overflow" + ); + if ( (currentOverflow != "hidden") && + (clippedSize.equals(safeSize)) ) { + var scrollBar = OpenLayers.Util.getScrollbarWidth(); + if (fixedSize.w) { + clippedSize.h += scrollBar; + } else { + clippedSize.w += scrollBar; + } + } + + newSize = this.getSafeContentSize(clippedSize); + } + } + this.setSize(newSize); + }, + /** * Method: setBackgroundColor * Sets the background color of the popup. @@ -511,79 +583,20 @@ OpenLayers.Popup = OpenLayers.Class({ */ setContentHTML:function(contentHTML) { - var preparedHTML; if (contentHTML != null) { this.contentHTML = contentHTML; } - if (this.autoSize) { - //fake the contentDiv for the CSS context - preparedHTML = "
" + this.contentHTML + "
"; - - // determine actual render dimensions of the contents - var realSize = - OpenLayers.Util.getRenderedDimensions(preparedHTML, null, - { displayClass: this.displayClass }); - - // is the "real" size of the div is safe to display in our map? - var safeSize = this.getSafeContentSize(realSize); - - var newSize = null; - - if (safeSize.equals(realSize)) { - //real size of content is small enough to fit on the map, - // so we use real size. - newSize = realSize; - - } else { - - //make a new OL.Size object with the clipped dimensions - // set or null if not clipped. - var fixedSize = new OpenLayers.Size(); - fixedSize.w = (safeSize.w < realSize.w) ? safeSize.w : null; - fixedSize.h = (safeSize.h < realSize.h) ? safeSize.h : null; - - if (fixedSize.w && fixedSize.h) { - //content is too big in both directions, so we will use - // max popup size (safeSize), knowing well that it will - // overflow both ways. - newSize = safeSize; - } else { - //content is clipped in only one direction, so we need to - // run getRenderedDimensions() again with a fixed dimension - var clippedSize = OpenLayers.Util.getRenderedDimensions( - preparedHTML, fixedSize, - { displayClass: this.contentDisplayClass } - ); - - //if the clipped size is still the same as the safeSize, - // that means that our content must be fixed in the - // offending direction. If overflow is 'auto', this means - // we are going to have a scrollbar for sure, so we must - // adjust for that. - // - var currentOverflow = OpenLayers.Element.getStyle( - this.contentDiv, "overflow" - ); - if ( (currentOverflow != "hidden") && - (clippedSize.equals(safeSize)) ) { - var scrollBar = OpenLayers.Util.getScrollbarWidth(); - if (fixedSize.w) { - clippedSize.h += scrollBar; - } else { - clippedSize.w += scrollBar; - } - } - - newSize = this.getSafeContentSize(clippedSize); - } - } - this.setSize(newSize); - } - - if (this.contentDiv != null) { + if ((this.contentDiv != null) && + (this.contentHTML != null) && + (this.contentHTML != this.contentDiv.innerHTML)) { this.contentDiv.innerHTML = this.contentHTML; } + + if (this.autoSize) { + this.updateSize(); + } + }, diff --git a/tests/Popup.html b/tests/Popup.html index 8f24b4715f..7b322c54da 100644 --- a/tests/Popup.html +++ b/tests/Popup.html @@ -17,7 +17,7 @@ "valid default popupid"); var firstID = popup.id; t.ok(popup.size.equals(size), "good default popup.size"); - t.eq(popup.contentHTML, "", "good default popup.contentHTML"); + 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"); t.eq(popup.border, OpenLayers.Popup.BORDER, "good default popup.border"); diff --git a/tests/Popup/Anchored.html b/tests/Popup/Anchored.html index 0869047326..be22abc706 100644 --- a/tests/Popup/Anchored.html +++ b/tests/Popup/Anchored.html @@ -13,7 +13,7 @@ t.ok( popup instanceof OpenLayers.Popup.Anchored, "new OpenLayers.Popup.Anchored returns Popup.Anchored object" ); t.ok(popup.id.startsWith("OpenLayers.Popup.Anchored"), "valid default popupid"); var firstID = popup.id; - t.eq(popup.contentHTML, "", "good default popup.contentHTML"); + t.eq(popup.contentHTML, null, "good default popup.contentHTML"); popup = new OpenLayers.Popup.Anchored();