-
-
Notifications
You must be signed in to change notification settings - Fork 23
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 custom colors #200
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.
Thanks for working on this @peacog! This is a good idea!
README.md
Outdated
Custom colors in hex or rgb can be used by providing a `color` option with the | ||
desired color value. rgb, rgba, and hex values with or without alpha channel are all supported. |
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.
Once that custom parsing code is removed, this can provide a few examples, then just point at the color-parse docs.
This is great @peacog! Thanks for the contribution! +1 And cool to learn that OpenLayers can handle some of that parsing logic too @symbioquine. :-) |
bc47eab
to
eab2e89
Compare
Thanks very much @symbioquine! Your suggestion is a much better solution and easier to understand. I had to assign to a new variable to avoid a |
How would we feel about doing a squash and merge here? I know we usually try and keep clean commit histories but especially for something like this when it is a simple change doesn't need multiple commits to explain the changes there's something nice about preserving the history from this PR here and squashing into a single commit on merge. I hope this could somewhat lower the barrier to contributions/maintenance, too |
I've updated the changelog and squashed the commits. |
@paul121 I feel fine about using the squash and merge functionality, but in this case I could see there wasn't a changelog entry so somebody is going to have to author 1 more commit adding that. At the point that's happening, it seems better for the original requester to do the work of consolidating the commit messages and getting the change ready-to-merge. I could also have just hit "approve" and let the workflow for the changelog check fail, but that feels less personal - almost passive aggressive compared to asking the PR author for what needs to happen... |
Thanks again @peacog!!! 🎉 |
Thanks for the contribution to farmOS-map @peacog! If you need this functionality in farmOS, the next step is a release of farmOS-map and a change like this one bumping the version of farmOS-map that ships with farmOS: farmOS/farmOS@42fe4bc |
Great! thanks very much to everyone for helping with this and accepting it so quickly :) |
Hi @symbioquine. Is created a new release something I can do? What are the steps? |
There's not much to it. I think the next version should be something like 2.3.1. There needs to be a commit like this one and a corresponding tag (https://github.com/farmOS/farmOS-map/releases/tag/v2.2.2) pushed. In theory you could create another PR for the commit, but @paul121, @mstenta, or myself need to push the tag so it's probably easier for one of us to do both the commit and the tagging. I can do it by Tuesday morning at the latest - possibly sooner, but no promises since I'm going to be traveling... |
I'm in no rush @symbioquine Whenever you find time to do it is fine by me. Thanks! |
I made the v2.3.0 release of farmOS-map that includes this change and opened a pull request to bump the version used by farmOS: farmOS/farmOS#841 |
thanks @symbioquine ! |
If we want to use stroke colors other than the 8 predefined named colors it's necessary to write a style function. It would be great if we could simply pass the hex or rgb value in the color option. This PR does that. The color option can be one of the predefined color names, a hex value with or without alpha channel, or an rgb() or rgba() value.