-
Notifications
You must be signed in to change notification settings - Fork 639
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
Update to use mapbox-gl and d3 v7. #330
Conversation
…x for a few functions. add max latitude clipping for 4 maps. disable chicago-parks for now, d3 cannot process it.
Hi @fnogatz - Happy New Year! Any thoughts about this one? |
Hi @epaulson, thank you very much for all the effort you put into this! I reviewed your changes. Would you mind to take into account these suggestions? |
There are some legacy code fragments in |
I would like to have this repository self-contained, so we can reproduce how to create the custom bundle in the future. What about adding browserify and the turf modules as dependencies to |
I have updated the PR to have local copies of d3 and mapbox-gl. I did also notice that there was some unused code in js/lib, and I thought we'd leave that for a future PR to remove it. I'd also rather put the new turf.js build into a future PR, mostly because I don't really know how to use the JS build tools yet! But yeah, we should do a real browserify step in the build rather than leaving everything static. For the record, this is what I'm using for browserify to build the turf bundle module.exports = {
bboxClip: require('@turf/bbox-clip').default,
lineString: require('@turf/helpers').lineString,
featureCollection: require('@turf/helpers').featureCollection,
featureEach: require('@turf/meta').featureEach
}; |
Aaaaand it's live: https://click-that-hood.com/ Thank you very much, @epaulson! 👏 |
This is a big update that switches to mapbox-gl instead of mapbox 0.6.7, which is nearly 9 years out of date. This restores map tiles to the background.
This also updates to d3 version 7, instead of d3 version 3.
I use a bit of turf.js for some calculations. This includes a smaller bundle of 4 functions created with browersify instead of including the full 500K library. (Using a custom bundle is recommended by turf.js)
This includes a new 'maxLat' setting that can be used by metadata.json - if it's present, the geojson is clipped to that latitude, which helps reduce distortion from the mercator projection. The europe-1914 and europe-1938 maps now use it, along with north-america and canada.
chicago-parks is disabled from now. For whatever, d3 version 6 and 7 cannot correctly display that geojson - d3 v5 handles it just fine, as does mapshaper.org I can reproduce the problem with a super-basic d3 only example, so I will open an issue with d3 and figure out if there's a bug in d3 or something wrong with the chicago-parks geojson
The main remaining problem is that when the aspect ratio gets too out of bounds - say you resize your browser window to be tall but narrow - the geojson svg is no longer properly placed relative to the map tiles. In looking at it, I don't see how this bug wasn't also present in the existing codebase, if map tiles were still working- the SVG container is set to fit the entire map, but because of the logo on the side and the vertical mercator distortion, the map that we draw to fill up the entire background simply can't fit right with the smaller SVG. It is most noticeable on the russia and asia maps, which are very wide by also go very far north. Because I'm pretty sure the bug was there before, I'm opening a PR now, and we can figure out how to fix it later - set some minimum size and turn on scrollbars if necessary, or only draw the map under the SVG, or clip the geojson, or let a map disable tiles in its metadata.json - russia and asia are quite playable without the tiles.