Skip to content
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

Merged
merged 3 commits into from
Aug 2, 2021

Conversation

raitisbe
Copy link
Contributor

@raitisbe raitisbe commented Jul 28, 2021

Description

This PR allows deciding IconSymboilizers URL based on features attribute by supporting template literals. We can write {{path}} in symbolizer definition

 symbolizers: [{
        kind: 'Icon',
        image: '{{path}}',
      }]

and 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:

  • Bugfix
  • Feature
  • Dependency updates
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe)
  • I am unsure (we'll look into it together)

Do you introduce a breaking change?

  • Yes
  • No
  • I am unsure (no worries, we'll find out)

Checklist

  • I understand and agree that the changes in this PR will be licensed under the BSD 2-Clause License
  • I have followed the guidelines for contributing
  • The proposed change fits to the content of the code of conduct
  • I have added or updated tests and documentation, and the test suite passes (run npm test locally)
  • I'm lost; why do I have to check so many boxes? Please help!

Copy link
Contributor

@jansule jansule left a 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!

src/OlStyleParser.spec.ts Outdated Show resolved Hide resolved
@jansule
Copy link
Contributor

jansule commented Aug 2, 2021

It would be awesome to also add the feature to geostyler-sld-parser

@raitisbe
Copy link
Contributor Author

raitisbe commented Aug 2, 2021

It would be awesome to also add the feature to geostyler-sld-parser

If I understood correctly, nothing changes from geostyler-sld-parser point of view? Maybe just the docs.

@jansule
Copy link
Contributor

jansule commented Aug 2, 2021

If I understood correctly, nothing changes from geostyler-sld-parser point of view?

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.

@jansule
Copy link
Contributor

jansule commented Aug 2, 2021

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}}',

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}

Copy link
Contributor

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

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.

Copy link
Contributor

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.

@jnewmoyer
Copy link

Yep, that would work. Ref: geostyler/geostyler-sld-parser#387

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants