-
Notifications
You must be signed in to change notification settings - Fork 135
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
Variable radius legend #135
Variable radius legend #135
Conversation
Add import so that example runs
* Update text-size to use expression based on viz.label_size property * Add new label properties to choropleth viz templates * Move label properties to base class (leverage inheritance to reduce repeated arguments) * Extract vector_color_map and numeric map (for height or line_width) to a VectorMixin class; fixes linestring bug with interpolation of color for certain color lookups with match-type * Extend CircleViz with VectorMixin (start GraduatedCircleViz, HeatmapViz, ClusteredCircleViz) * Extend vector data loading to base map and CircleMap -- datadriven styling for radius needs work * Refine color mapping in VectorMixin and update templates, viz.py * Update CircleViz template files with Jinja inheritance, establish {% block circle %} tag * Update Vector layer example for CircleViz; add geojson_file_to_dict utility and logic to viz.py to facilitate loading data from JSON object, list of Python dicts, GeoJSON filename * Refine function for parsing GeoJSON and JSON input (esp for vector visualizations) and add SourceDataError * Add support for GraduatedCircleViz template to use vector source data layer * Add support for HeatmapViz to use vector data source * Add and refine tests; update utility name for geojson_to_dict etc. * Change FileNotFoundError to IOError for Python2.7 support * Enable data from vector layers to be used for data-driven style (without using data-join technique) * Update docs for VectorMixin class, vector properties and label properties inherited from MapViz parent class * Update geojson_to_dict to geojson_to_dict_list and add docs; organize MapBox.create_html()
…for GraduatedCircleViz
53013ca
to
b387e50
Compare
7f068cb
to
aa3ef04
Compare
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.
Changes look good @akacarlyann - can you add a screenshot here for documentation and design 👀 showing what the new legend option looks like?
Another question - does this legend support a radius-and-color legend, for showing two dimensions using color and size for a graduated circle viz?
@ryanbaumann No multi-dimensional legends just yet, so the variable radius is using the |
Great. I'd anticipate that a user would expect both color and size options for a GraduatedCircleMap - I lean towards not merging this until we can incorporate both size and color into the legend. |
@ryanbaumann Makes sense to me to. Before this point only color legends were available for graduated circle visualizations. I think adding multiple sub-legends could parallel the multi-layer visualizations so I had expected that support to be wrapped in #129 but I'm happy to scratch out some ideas for this. |
@ryanbaumann is this more along the lines you were thinking? I still want to tackle compound/multi-variable legends in greater detail, but maybe this is a decent stand-in. Legends are taking up so much real-estate I want to look into adding a toggle to collapse them too. |
Yep, that looks great. I agree that legend real-estate is hard to manage - collapsable seems like a good option. |
Should I plan to merge this PR in, with a followup PR for collapsing the legends @akacarlyann ? |
@ryanbaumann I think I have a few commits locally to support the double graduated circle legend so I need to push those first. |
@ryanbaumann I'll see what I can do to clean up this PR this week :) |
…teLegendMargin functions in main.html to automatically place secondary legends
@ryanbaumann I added a couple functions to better automatically place a secondary legend, as in the case with the graduated circle default. I'm ready for review/merge when you are :) |
Still not seeing this functionality on GraduatedCircleViz, despite legends being displayed in the examples. Will this Merge resolve? |
@caerus-data it's ready for merge as of last week :) Glad it will be useful! @ryanbaumann the coveralls test looks like it stalled but I would hope this PR doesn't decrease coverage. |
Tightened up my issue. Will post as a new question |
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.
Looks good @akacarlyann
Adds support for a variable radius legend for the GraduatedCircleViz. Adds legend_function parameter for selecting between calcColorLegend and calcRadiusLegend in JavaScript. Addresses #80.