-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
Support dynamic attribute based point icons #364
Conversation
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.
Very nice! Thank you for your contribution!
It would be awesome to also add the feature to geostyler-sld-parser |
Co-authored-by: Jan Suleiman <jansule@users.noreply.github.com>
If I understood correctly, nothing changes from geostyler-sld-parser point of view? Maybe just the docs. |
That is correct. I rather meant that it would be nice to also have this feature implemented for other parsers, e.g. geostyler-sld-parser. |
I created a new issue for that topic, as there is no need to discuss this here. geostyler/geostyler-sld-parser#387 Feel free to open a PR on geostyler-sld-parser if you also want to implement that feature there. Again, thank you for your contribution! |
name: 'OL Style Rule 0', | ||
symbolizers: [{ | ||
kind: 'Icon', | ||
image: '{{path}}', |
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.
why double brackets? we are trying to use this with GeoServer styles and they use $ brackets - ${path}
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.
The double brackets are only used within the parsing context. This is basically just a hint for the parser to not just write a static url, but to use the styling-format dependent way of providing dymanic icons. In the case of openlayers styles, the parser creates a style function instead of a style object.
So at the end, the double brackets should not be part of the parsed style
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.
Yes, I get the reason for the placeholders, I'm curious about the choice to use double brackets vs the ${value}
convention.
Maybe our use case is not very common, but this might make more sense if I explain... we want to take existing SLDs (from GeoServer) and use geostyler to conver them into OpenLayers styles in order to keep a consistent style between WMS rendered map images and client side renderings via WFS. So we convert from SLD -> geostyler style -> OL style.
When we do this we end up with placeholders using $ bracket notation that aren't understood as placeholders in the OL conversion.
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.
Alright, now I understand. The main problem here is that the geostyler-sld-parser does not yet support handling dynamic icon urls. So currently, the href
attribute of <OnlineResource>
will always be interpreted as a static url (see here), and thereby the parser passes the ${value}
notation to the other styles.
As soon as the geostyler-sld-parser supports this feature, it should replace the SLD specific notation (${value}
) with the geostyler-style specific notation ({{value}})
, so other parsers who support this feature will be able to interpret it.
Yep, that would work. Ref: geostyler/geostyler-sld-parser#387 |
Description
This PR allows deciding IconSymboilizers URL based on features attribute by supporting template literals. We can write
{{path}}
in symbolizer definitionand it would then take the attribute
path
from each feature and generate the URL to load the icon from.Related issues or pull requests
#363
Pull request type
Please check the type of change your PR introduces:
Do you introduce a breaking change?
Checklist
npm test
locally)