-
-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
View2D properties #818
View2D properties #818
Conversation
Looks good to me. |
@@ -89,7 +89,8 @@ ol.View2D = function(opt_options) { | |||
values[ol.View2DProperty.RESOLUTION] = resolutionConstraint( | |||
this.maxResolution_, options.zoom); | |||
} | |||
values[ol.View2DProperty.ROTATION] = options.rotation; | |||
values[ol.View2DProperty.ROTATION] = | |||
goog.isDef(options.rotation) ? options.rotation : 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good to me. Though I don't understand the above change. I was thinking that sanitization only occured in getView2DState.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. This is for consistent behaviour with #690 regarding property values that are not explicitly set by the user.
Case for #690:
User creates a layer:
var layer = new ol.layer.Layer({
source: ...
});
If default values are not set, then you get strange behaviour: layer.getVisible()
returns undefined
(which is logically false) but the layer is visible by default. Without explicitly setting a default value, you'll get undesired behaviour if you try to toggle a layer's visibility with
layer.setVisible(!layer.getVisible());
The easy (and, I think, least bad) solution is to explicitly set the default values of properties.
This change applies the same logic to rotation
, so that, for example:
var view = new ol.View2D({});
view.setRotation(view.getRotation() + Math.PI / 4);
works as you'd expect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Convinced. Thanks a lot for explaining.
examples/vector-formats: Added "Encoded Polyline" format
This PR improves the handling of default and missing values in
ol.View2D
, as suggested by @elemoine in #690.