Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Removing camelize method. #64

Merged
merged 2 commits into from

4 participants

@tmcw

There's not much justification for the camelize method, which is only used in a handful of places and can be replaced by knowledge. This patch removes the method, the tests for the method, and corrects usage of CSS-in-javascript elsewhere.

@elemoine
Owner

@tmcw we have decided on the mailing list to move the deprecated code to deprecated.js. This happens in the main development branch (master), so in the context of 2.x. With this we won't break apps that rely on deprecated code. As long as deprecated.js is loaded things should work as before.

@tmcw

All right, moved over to deprecated.

@elemoine
Owner

I just realized that this function is an API function, so there may be applications relying on it. I think it's fine to have a release note mentioning that this function is deprecated and has been moved to deprecated.js. Can we just make it output a deprecating warning message? After that I think we can merge. Thanks @tmcw.

@tmcw

AFAIK, the changelog isn't managed in Git, so I can't really do much there. I think a release note is all that's required here, or it should be in 3 or something. Adding a stub function is just adding another thing to deprecate down the line. The future doesn't come without change.

@elemoine
Owner

I can deal with the release note if you want, no problem.

I'm not saying adding a stub function, but just make the moved function output a deprecation warning.

http://trac.osgeo.org/openlayers/wiki/Release/2.12/Notes

@elemoine
Owner

@ahocevar please merge this if you agree with the changes made by @tmcw. I think they address your comments. Once merged I'll go ahead and update the 2.12 notes. Thanks.

@ahocevar ahocevar merged commit c2b8f9c into from
@elemoine elemoine referenced this pull request from a commit
@elemoine elemoine indicate that String.camelize is deprecated in the 2.12 release notes…
…, refs #64, no functional change
acf6a8a
@jorix jorix commented on the diff
lib/OpenLayers/Handler/Box.js
((12 lines not shown))
var bottom = parseInt(OpenLayers.Element.getStyle(
- this.zoomBox, "border-bottom-width"));
+ this.zoomBox, "borderBottomWidth"));
@jorix
jorix added a note

I suspect that this "Pull" may have affected the behavior of ZoomBox, my copy of the February 27 still draws the box and changes the cursor.

Try on current http://www.openlayers.org/dev/examples/controls.html oops! lack the box and the cursor is inadequate.

(Before I put the comment in the wrong place)

@elemoine Owner
elemoine added a note

I don't see anything wrong in the Box handler. Are you sure this commit is responsible?

@jorix
jorix added a note

not, sorry. Var bottom is NaN but... At this moment I do not understand the problem, I'll wait to see something concrete.

But I'm sure that is later than fa656e7

@jorix
jorix added a note

@elemoine: Yes now I am sure. I reversed the two commits and it works, but still do not know why it fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elemoine elemoine commented on the diff
lib/OpenLayers/Control/OverviewMap.js
((6 lines not shown))
parseInt(OpenLayers.Element.getStyle(this.extentRectangle,
- 'border-right-width'));
+ 'borderRightWidth'));
this.wComp = (this.wComp) ? this.wComp : 2;
this.hComp = parseInt(OpenLayers.Element.getStyle(this.extentRectangle,
'border-top-width')) +
@elemoine Owner
elemoine added a note

This line isn't changed while it should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@ahocevar
Owner

Is anybody working on this? If we don't get a fix before the feature freeze later today, I'm going to revert the merge.

@elemoine
Owner

I'd like to take a look tonight.

@elemoine elemoine commented on the diff
lib/OpenLayers/BaseTypes/Element.js
@@ -171,7 +171,7 @@ OpenLayers.Element = {
var css = document.defaultView.getComputedStyle(element, null);
value = css ? css.getPropertyValue(style) : null;
@elemoine Owner
elemoine added a note

We used to pass a dashed string to getPropertyValue. We now pass a camelized string. This is certainly an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@elemoine
Owner

@ahocevar for the reason mentioned in my last comment I think we can revert acf6a8a, 5875b50, and 43ef092. Do you agree?

@elemoine elemoine referenced this pull request from a commit in elemoine/openlayers
@elemoine elemoine Revert "indicate that String.camelize is deprecated in the 2.12 relea…
…se notes, refs #64, no functional change"

This reverts commit acf6a8a.
526ff5f
@elemoine elemoine referenced this pull request
Merged

Revert camelize removal #278

@mosesonline mosesonline referenced this pull request from a commit
Commit has since been removed from the repository and is no longer available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Nov 21, 2011
  1. @tmcw

    Removing camelize method.

    tmcw authored
  2. @tmcw
This page is out of date. Refresh to see the latest.
View
22 lib/OpenLayers/BaseTypes.js
@@ -61,28 +61,6 @@ OpenLayers.String = {
},
/**
- * APIFunction: camelize
- * Camel-case a hyphenated string.
- * Ex. "chicken-head" becomes "chickenHead", and
- * "-chicken-head" becomes "ChickenHead".
- *
- * Parameters:
- * str - {String} The string to be camelized. The original is not modified.
- *
- * Returns:
- * {String} The string, camelized
- */
- camelize: function(str) {
- var oStringList = str.split('-');
- var camelizedString = oStringList[0];
- for (var i=1, len=oStringList.length; i<len; i++) {
- var s = oStringList[i];
- camelizedString += s.charAt(0).toUpperCase() + s.substring(1);
- }
- return camelizedString;
- },
-
- /**
* APIFunction: format
* Given a string with tokens in the form ${token}, return a string
* with tokens replaced with properties from the given context
View
4 lib/OpenLayers/BaseTypes/Element.js
@@ -163,7 +163,7 @@ OpenLayers.Element = {
var value = null;
if (element && element.style) {
- value = element.style[OpenLayers.String.camelize(style)];
+ value = element.style[style];
if (!value) {
if (document.defaultView &&
document.defaultView.getComputedStyle) {
@@ -171,7 +171,7 @@ OpenLayers.Element = {
var css = document.defaultView.getComputedStyle(element, null);
value = css ? css.getPropertyValue(style) : null;
@elemoine Owner
elemoine added a note

We used to pass a dashed string to getPropertyValue. We now pass a camelized string. This is certainly an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
} else if (element.currentStyle) {
- value = element.currentStyle[OpenLayers.String.camelize(style)];
+ value = element.currentStyle[style];
}
}
View
6 lib/OpenLayers/Control/OverviewMap.js
@@ -499,14 +499,14 @@ OpenLayers.Control.OverviewMap = OpenLayers.Class(OpenLayers.Control, {
this.ovmap.zoomToMaxExtent();
// check extent rectangle border width
this.wComp = parseInt(OpenLayers.Element.getStyle(this.extentRectangle,
- 'border-left-width')) +
+ 'borderLeftWidth')) +
parseInt(OpenLayers.Element.getStyle(this.extentRectangle,
- 'border-right-width'));
+ 'borderRightWidth'));
this.wComp = (this.wComp) ? this.wComp : 2;
this.hComp = parseInt(OpenLayers.Element.getStyle(this.extentRectangle,
'border-top-width')) +
@elemoine Owner
elemoine added a note

This line isn't changed while it should be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
parseInt(OpenLayers.Element.getStyle(this.extentRectangle,
- 'border-bottom-width'));
+ 'borderBottomWidth'));
this.hComp = (this.hComp) ? this.hComp : 2;
this.handlers.drag = new OpenLayers.Handler.Drag(
View
8 lib/OpenLayers/Handler/Box.js
@@ -220,13 +220,13 @@ OpenLayers.Handler.Box = OpenLayers.Class(OpenLayers.Handler, {
document.body.removeChild(testDiv);
var left = parseInt(OpenLayers.Element.getStyle(this.zoomBox,
- "border-left-width"));
+ "borderLeftWidth"));
var right = parseInt(OpenLayers.Element.getStyle(
- this.zoomBox, "border-right-width"));
+ this.zoomBox, "borderRightWidth"));
var top = parseInt(OpenLayers.Element.getStyle(this.zoomBox,
- "border-top-width"));
+ "borderTopWidth"));
var bottom = parseInt(OpenLayers.Element.getStyle(
- this.zoomBox, "border-bottom-width"));
+ this.zoomBox, "borderBottomWidth"));
@jorix
jorix added a note

I suspect that this "Pull" may have affected the behavior of ZoomBox, my copy of the February 27 still draws the box and changes the cursor.

Try on current http://www.openlayers.org/dev/examples/controls.html oops! lack the box and the cursor is inadequate.

(Before I put the comment in the wrong place)

@elemoine Owner
elemoine added a note

I don't see anything wrong in the Box handler. Are you sure this commit is responsible?

@jorix
jorix added a note

not, sorry. Var bottom is NaN but... At this moment I do not understand the problem, I'll wait to see something concrete.

But I'm sure that is later than fa656e7

@jorix
jorix added a note

@elemoine: Yes now I am sure. I reversed the two commits and it works, but still do not know why it fails.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
this.boxOffsets = {
left: left,
right: right,
View
8 lib/OpenLayers/Popup.js
@@ -844,10 +844,10 @@ OpenLayers.Popup = OpenLayers.Class({
//read the padding settings from css, put them in an OL.Bounds
contentDivPadding = new OpenLayers.Bounds(
- OpenLayers.Element.getStyle(this.contentDiv, "padding-left"),
- OpenLayers.Element.getStyle(this.contentDiv, "padding-bottom"),
- OpenLayers.Element.getStyle(this.contentDiv, "padding-right"),
- OpenLayers.Element.getStyle(this.contentDiv, "padding-top")
+ OpenLayers.Element.getStyle(this.contentDiv, "paddingLeft"),
+ OpenLayers.Element.getStyle(this.contentDiv, "paddingBottom"),
+ OpenLayers.Element.getStyle(this.contentDiv, "paddingRight"),
+ OpenLayers.Element.getStyle(this.contentDiv, "paddingTop")
);
//cache the value
View
24 lib/deprecated.js
@@ -85,6 +85,30 @@ OpenLayers.Util.clearArray = function(array) {
array.length = 0;
};
+
+/**
+ * APIFunction: camelize
+ * Camel-case a hyphenated string.
+ * Ex. "chicken-head" becomes "chickenHead", and
+ * "-chicken-head" becomes "ChickenHead".
+ *
+ * Parameters:
+ * str - {String} The string to be camelized. The original is not modified.
+ *
+ * Returns:
+ * {String} The string, camelized
+ */
+
+OpenLayers.String.camelize = function(str) {
+ var oStringList = str.split('-');
+ var camelizedString = oStringList[0];
+ for (var i=1, len=oStringList.length; i<len; i++) {
+ var s = oStringList[i];
+ camelizedString += s.charAt(0).toUpperCase() + s.substring(1);
+ }
+ return camelizedString;
+};
+
/**
* Function: setOpacity
* *Deprecated*. This function has been deprecated. Instead, please use
View
27 tests/BaseTypes.html
@@ -59,34 +59,7 @@
str = " ";
t.eq(OpenLayers.String.trim(str), "", "whitespace string is trimmed correctly");
}
-
- function test_String_camelize(t) {
- t.plan(7);
-
- var str = "chickenhead";
- t.eq(OpenLayers.String.camelize(str), "chickenhead", "string with no hyphens is left alone");
-
- str = "chicken-head";
- t.eq(OpenLayers.String.camelize(str), "chickenHead", "string with one middle hyphen is camelized correctly");
-
- str = "chicken-head-man";
- t.eq(OpenLayers.String.camelize(str), "chickenHeadMan", "string with multiple middle hyphens is camelized correctly");
-
- str = "-chickenhead";
- t.eq(OpenLayers.String.camelize(str), "Chickenhead", "string with starting hyphen is camelized correctly (capitalized)");
-
- str = "-chicken-head-man";
- t.eq(OpenLayers.String.camelize(str), "ChickenHeadMan", "string with starting hypen and multiple middle hyphens is camelized correctly");
- str = "chicken-";
- t.eq(OpenLayers.String.camelize(str), "chicken", "string ending in hyphen is camelized correctly (hyphen dropped)");
-
- str = "chicken-head-man-";
- t.eq(OpenLayers.String.camelize(str), "chickenHeadMan", "string with multiple middle hyphens and end hyphen is camelized correctly (end hyphen dropped)");
-
-
- }
-
function test_String_format(t) {
var unchanged = [
"", "${ ", "${", " ${", "${${", "${}", "${${}}", " ${ ${",
View
2  tests/Handler/Box.html
@@ -41,7 +41,7 @@
// Chromium 10: left is 0
var testdiv = OpenLayers.Util.createDiv('testdiv', new OpenLayers.Pixel(5, 5));
map.div.appendChild(testdiv);
- var left = parseInt(OpenLayers.Element.getStyle(testdiv, 'border-left-width'));
+ var left = parseInt(OpenLayers.Element.getStyle(testdiv, 'borderLeftWidth'));
map.div.removeChild(testdiv);
var testAll = !isNaN(left);
View
40 tests/deprecated/BaseTypes.html
@@ -0,0 +1,40 @@
+<html>
+<head>
+ <script src="../../OLLoader.js"></script>
+ <script src="../../../lib/deprecated.js"></script>
+
+ <script type="text/javascript">
+
+ function test_String_camelize(t) {
+ t.plan(7);
+
+ var str = "chickenhead";
+ t.eq(OpenLayers.String.camelize(str), "chickenhead", "string with no hyphens is left alone");
+
+ str = "chicken-head";
+ t.eq(OpenLayers.String.camelize(str), "chickenHead", "string with one middle hyphen is camelized correctly");
+
+ str = "chicken-head-man";
+ t.eq(OpenLayers.String.camelize(str), "chickenHeadMan", "string with multiple middle hyphens is camelized correctly");
+
+ str = "-chickenhead";
+ t.eq(OpenLayers.String.camelize(str), "Chickenhead", "string with starting hyphen is camelized correctly (capitalized)");
+
+ str = "-chicken-head-man";
+ t.eq(OpenLayers.String.camelize(str), "ChickenHeadMan", "string with starting hypen and multiple middle hyphens is camelized correctly");
+
+ str = "chicken-";
+ t.eq(OpenLayers.String.camelize(str), "chicken", "string ending in hyphen is camelized correctly (hyphen dropped)");
+
+ str = "chicken-head-man-";
+ t.eq(OpenLayers.String.camelize(str), "chickenHeadMan", "string with multiple middle hyphens and end hyphen is camelized correctly (end hyphen dropped)");
+
+
+ }
+
+ </script>
+</head>
+<body>
+</body>
+</html>
+
View
40 tests/deprecated/BaseTypes/String.html
@@ -0,0 +1,40 @@
+<html>
+<head>
+ <script src="../../OLLoader.js"></script>
+ <script src="../../../lib/deprecated.js"></script>
+
+ <script type="text/javascript">
+
+ function test_String_camelize(t) {
+ t.plan(7);
+
+ var str = "chickenhead";
+ t.eq(OpenLayers.String.camelize(str), "chickenhead", "string with no hyphens is left alone");
+
+ str = "chicken-head";
+ t.eq(OpenLayers.String.camelize(str), "chickenHead", "string with one middle hyphen is camelized correctly");
+
+ str = "chicken-head-man";
+ t.eq(OpenLayers.String.camelize(str), "chickenHeadMan", "string with multiple middle hyphens is camelized correctly");
+
+ str = "-chickenhead";
+ t.eq(OpenLayers.String.camelize(str), "Chickenhead", "string with starting hyphen is camelized correctly (capitalized)");
+
+ str = "-chicken-head-man";
+ t.eq(OpenLayers.String.camelize(str), "ChickenHeadMan", "string with starting hypen and multiple middle hyphens is camelized correctly");
+
+ str = "chicken-";
+ t.eq(OpenLayers.String.camelize(str), "chicken", "string ending in hyphen is camelized correctly (hyphen dropped)");
+
+ str = "chicken-head-man-";
+ t.eq(OpenLayers.String.camelize(str), "chickenHeadMan", "string with multiple middle hyphens and end hyphen is camelized correctly (end hyphen dropped)");
+
+
+ }
+
+ </script>
+</head>
+<body>
+</body>
+</html>
+
View
1  tests/list-tests.html
@@ -226,6 +226,7 @@
<li>deprecated/Ajax.html</li>
<li>deprecated/BaseTypes/Class.html</li>
<li>deprecated/BaseTypes/Element.html</li>
+ <li>deprecated/BaseTypes/String.html</li>
<li>deprecated/Control/MouseToolbar.html</li>
<li>deprecated/Layer/MapServer/Untiled.html</li>
<li>deprecated/Layer/MultiMap.html</li>
Something went wrong with that request. Please try again.