-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
Electoral data vis upgrades #206
Conversation
|
||
// Get min and max counts | ||
let minCount = min(dataByBoundary.map((d) => d.count || 0)) || 0 | ||
let minCount = 0 // min(dataByBoundary.map((d) => d.count || 0)) || 0 |
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.
question: is this right?
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.
Think so. We could make it an option to 'go from 0 or go from min' but I think in general, data we're loading is things like population, votes, £, etc — all continuous data which benefits from starting at 0. Otherwise the choropleth becomes hypersensitive to small differences between 95 and 98 or something.
The toggle is probably the most versatile approach.
@@ -203,12 +235,12 @@ const PoliticalChoropleths: React.FC<PoliticalChoroplethsProps> = ({ | |||
maxzoom={tileset.maxZoom} | |||
/> | |||
|
|||
<Layer | |||
{/* <Layer |
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.
note: should this be uncommented?
// (boundary) => boundary.boundaryType === boundaryType | ||
// )?.tilesets | ||
|
||
// const activeTileset = useActiveTileset(boundaryType) |
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.
suggestion: remove this if not required
@@ -906,7 +1230,7 @@ function BigRecord({ | |||
} | |||
|
|||
const { primary, secondary } = getListValuesBasedOnDataType(item, dataType) | |||
const isActive = explorer.isValidEntity(explorer.state) | |||
// const isActive = explorer.isValidEntity(explorer.state) |
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.
suggestion: remove this if not required
@@ -699,65 +868,140 @@ function RelatedDataCarousel({ data }: { data: AreaLayerDataQuery['data'] }) { | |||
) | |||
} | |||
|
|||
// const ELECTION_RESULTS = gql` |
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.
suggestion: remove this if not required
), # Back to start to close polygon | ||
) | ||
bbox = Polygon(bbox_coords, srid=4326) | ||
# areas = models.Area.objects.filter(**area_type_filter.query_filter).filter( |
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.
question: should the filter by combined area be removed?
It was there because when "rolling up" data to larger areas, not all of the relevant points are in the map_bounds
box.
E.G. If have data at the output_area
level, and I'm browsing at the LSOA
level, not all the output_areas
for the LSOA
I'm looking at will be in the bounding box.
2588f26
to
c96f167
Compare
Suspect IssuesThis pull request was deployed and Sentry observed the following issues:
Did you find this useful? React with a 👍 or 👎 |
No description provided.