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
Add dimensions in WFS GetFeature requests #2496
Add dimensions in WFS GetFeature requests #2496
Conversation
@fredj : Could you take a look here ? Note that I do not understand why to not dispatch all global dimensions to each node. In my project I will have to add FLOOR dimension on approximately all layers. |
Note that when I put one dimension on one layer (gmf admin), it is applied on layer parent group (ol.Layer object), can't-we get this applied on all layer tree nodes, regardless of the dimensions set in admin interface ? |
@@ -290,6 +290,7 @@ gmf.LayertreeController.prototype.updateLayerDimensions_ = function(layer, node) | |||
var value = this.dimensions[key]; | |||
if (value !== undefined) { | |||
dimensions[key] = value; | |||
node.dimensions[key] = value; | |||
} |
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.
This if looks strange for me, for me it should be like this:
var value = this.dimensions[key];
dimensions[key] = value !== undefined ? value : node.dimensions[key];
@fredj what do you think?
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.
node.dimensions
are the default dimension values for a node; it should not be modified
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.
Without this I cannot apply current dimensions on WFS GetFeature requests
src/services/query.js
Outdated
value = dimensions[key]; | ||
} | ||
if (value !== undefined) { | ||
url = goog.uri.utils.setParam(url, key, value); |
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.
Do not use goog
!!!
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.
Ok, can you explain, why do not use goog, what should I use instead ? Note that this comes from doGetFeatureInfoRequests_ existing code.
I see goog
everywhere in this file.
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.
We try to don't use goog anymore to be able to build the project without google library.
In the master branch almost all the goog (Except goog.[asserts, requires, provide, module]) will be removes, in this case there a method in openlayers that can be used.
src/services/query.js
Outdated
if (value === undefined) { | ||
// get the value from the layer default value | ||
value = dimensions[key]; | ||
} |
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.
Looks duplicate code from layer tree...
594cdd4
to
373199c
Compare
I've changed my desktop.js file with this : I've passed the parameters directly to $http, so I do not use goog anymore. What do you think about this updated version, is this good for merging. |
Note that I still have an issue on the server side, where I find really boring to add same dimensions on all layers. |
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.
Looks good to me @fredj can you also have a look?
For the server part you can write a script like this one: https://github.com/camptocamp/epfl_authgeoportail/blob/master/epfl_authgeoportail/scripts/manage_layers.py |
@fredj : Are you OK for merging this PR ? |
No description provided.