-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add color option to ol.style.Icon #4457
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
Conversation
|
@ahocevar, I ended up multiplying pixel values instead of replacing white with the colour because the latter left undesired artifacts in the majority of cases (e.g. anything with antialiasing) |
12e2a14 to
afe11b3
Compare
|
Thanks for this pull request @alexbrault. Great enhancement! I'll review it within the next few days. |
src/ol/style/iconstyle.js
Outdated
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.
Applying the color will only work if the image is not tainted.
It should be moved after determineTainting().
06a5230 to
28633e4
Compare
externs/olx.js
Outdated
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.
Would be nice to also accept a string here.
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.
I forget what my reasoning for omitting this was, but yes, it should
28633e4 to
c80ff96
Compare
c80ff96 to
3f16b34
Compare
|
This looks great @alexbrault. It looks like we do not have a CLA on file from you yet. Can you please submit one so we can merge this? |
Thanks @alexbrault for openlayers/openlayers#4457 and any future contributions you can make.
Add color option to ol.style.Icon
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.
I think it would be nicer to have a dedicated example that shows what this option should be used for. We have a bad tendency to continue modifying examples that started out simple until they become overly complex (or just ugly). And I think this particular example looked nicer before the color option was added (same goes for opacity).
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 point. I'll submit a PR to fork the example

This PR adds a color option to tint an ol.style.Icon.
See https://groups.google.com/forum/#!topic/ol3-dev/vPK4ygQHaqo