-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add 3-d option for choropleth extrusion; support pitch and bearing arguments for MapViz #90
Conversation
a46358b
to
b11698d
Compare
…guments for MapViz
b11698d
to
564008a
Compare
Hehe, this is awesome @akacarlyann. Nice work! Notes:
Unrelated note on performance:
|
@akacarlyann no rush here yet, but wondering if you've had a chance for to take a pass on any of the feedback above or are ready for a final PR review. |
@ryanbaumann funny timing - I'm heading to the coffee shop to work on this today! It's my first chance in a couple weeks. For the SF population graphic, there is an updating scale 3d height in the legend. Is there any public source code you can think of that might implement something similar? Otherwise I'll just handle it similarly to the way we do color v radius for the graduated circle viz. |
@akacarlyann the legend in www.mapbox.com/bites/00273 is pretty custom CSS and HTML - I'd have to search around to see if there are any modularized components for size extruded / 3D data visualizations |
6b1bf53
to
51e021d
Compare
…roperty; add note to legend title if two properties used for data-driven styling
@ryanbaumann Got it. Removed duplicate add to tooltip if height_property equals color_property and added note to legend title if height based on different property from color. Ready for you to take a look. |
Nice work @akacarlyann. A few updates to tests for coverage of the new utils function |
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.
Hi guys, really cool feature! One thing though, you currently dont check the height_default
parameter to set self.extrude
to true.
Thanks @lucasvw - could you open a quick issue ticket or PR for the change or bug? This PR is old. Thank you! |
Addresses #4, #15; closes #83