search

yaph / d3-geomap

A library for creating geographical maps based on D3.js
https://d3-geomap.github.io/
MIT License
132 stars 45 forks source link

feat: substitute colorbrewer for optional d3-scale-chromatic #52

Closed dpraul closed 5 years ago

dpraul commented 5 years ago

The initial request was to substitute colorbrewer for d3-scale-chromatic, as mentioned in https://github.com/yaph/d3-geomap/pull/51#issuecomment-491236997.

Looking at how colorbrewer (and the subsequent replacement, d3-scale-chromatic) is used, it doesn't look like much of it is strictly needed - i.e. it feels like unnecessary bulk to include the whole d3-scale-chromatic package when a user might not need more than a single array from it.

With that in mind, I've taken the approach of making it optional. The default color scheme (schemeOrRd[9]) is included, but d3-scale-chromatic can be installed and used for more advanced functionality, as seen in examples/world-choropleth.html.

Let me know your thoughts

dpraul commented 5 years ago

Additionally, I've made this PR for merging into the webpack branch (as you can see). For a cleaner git history, I think it would make more sense to squash webpack to master, and then re-base this to master and squash this PR after, but as it is your project and I don't know your deploy process, you can do as you see appropriate.

yaph commented 5 years ago

Many thanks Dylan! I think you took a very reasonable approach with regards to not making d3-scale-chromatic a dependency. The color values in the schemeOrRd scheme are not going to change anyway.

With regards to git, I more or less follow the approach outlined in A successful Git branching model, because I'm used to it and understand it. Thanks for your suggestion though.

I want to migrate an existing production project to the updated d3-geomap codebase and update the documentation site as needed, before creating a new release hopefully early next week.

I'm very grateful for your help. Who knows if there would have been a new major release without your help. Assuming you did this during your work time please say thanks to your employer for letting you work on this.