Skip to content

feat(data-weaver): choropleth charts - #458

Open
nick-nlb wants to merge 6 commits into
datacommonsorg:mainfrom
nick-nlb:dw-choropleth-charts
Open

feat(data-weaver): choropleth charts#458
nick-nlb wants to merge 6 commits into
datacommonsorg:mainfrom
nick-nlb:dw-choropleth-charts

Conversation

@nick-nlb

@nick-nlb nick-nlb commented Sep 1, 2026

Copy link
Copy Markdown
Collaborator

Description

This PR adds choropleth map visualization for multi-entity geographic comparisons in Data Weaver.

This fills the use case where data for a large number of places, usually children, needs to be displayed on a single chart.

Implementation

  • Geographic Boundaries API & Service: Implemented /api/geo route to query GeoJSON from the Data Commons API.
  • LRU caching (via a library): In the same session, we often use the same maps (i.e. a map of Africa with different data on different cards). To avoid refetching, we use a byte-limited LRU (via the very widely used lru-cache library).
  • Map Projections & Bounds Fitting: Added regional projection heuristics (map_projection.ts) supporting curated regional projections (Lambert Azimuthal for Europe, rotated Equirectangular for Asia/Oceania, Albers USA for US states) and dynamic bounds fitting (map_bounds.ts). This is largely based on the Data Commons website implementation (with some additional landmass filtering).
  • Choropleth Component & Time Slider: Created DataChartChoropleth with SVG pan/zoom, interactive tooltip with value/unit formatting, color ramp legend, and SliderTime for stepping or auto-playing through temporal observations.
  • General Pipeline: Updated pipeline and store to handle parentPlaceId and chropleths, added styling for the new components, updated prompts to recognize map-appropriate queries.

Testing

  • Build & Type Validation: Verified zero errors across pnpm test, pnpm lint (TypeScript + Biome + Stylelint), and pnpm build.

Coming Functionality (all in Tracker)

  • Better height handling during loading (to avoid layout shift).
  • Clickable spawning of line charts from choropleth
  • A consolidated facet selector (i.e. no very tall cards like we see now).
  • Investigate GeoJSON simplification (the calls are very quick and responsive, but the GeoJSON is often much more detailed than needs to be given the card size, with the caveat that we do need to accommodate zooming).
  • More color schemes for the choropleths (currently a single monotonic choropleth color scheme available).

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new choropleth map chart style to the application, enabling geographic data visualization. Key additions include a new API route (/api/geo) to fetch and normalize GeoJSON coordinates, a DataChartChoropleth component featuring pan, zoom, and time-slider animation controls, and helper services for map projections, bounds fitting, and LRU caching. Feedback on the changes highlights a high-severity issue where calling e.preventDefault() inside React's synthetic onWheel handler will throw a console error and fail to prevent page scrolling. The reviewer recommends attaching the wheel event listener directly to the SVG element with { passive: false } via a useEffect hook.

@nick-nlb
nick-nlb marked this pull request as ready for review September 1, 2026 22:04

@beets beets left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for this! ideally, a lot of this logic needn't live in dataweaver but is something that is easily plugged in. so i'd keep that in mind as we add this here first, that we'd lift this out of the app soon.

Comment on lines +167 to +172
(n.types?.includes('Country') ||
n.types?.includes('State') ||
n.types?.includes('County') ||
n.types?.includes('Place') ||
n.types?.includes('AdministrativeArea1') ||
n.types?.includes('AdministrativeArea2'))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's pull these out as constants

would really prefer if this logic did not live in the app (but in our API / client library), so could you add a TODO

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

similarly, logic for pulling down the right resolution level, adding the unwinding logic at processing time, etc.

[chartSeries],
);

const [isGeoAvailable, setIsGeoAvailable] = useState<boolean | null>(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we pull out the choropleth logic into a subclass (as static helpers perhaps).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add documentation to the module

Comment on lines +595 to +598
hovered.y < 60 ? s['tooltip-align-bottom'] : '',
hovered.x < 80 ? s['tooltip-align-left'] : '',
hovered.containerWidth !== undefined &&
hovered.x > hovered.containerWidth - 80

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please pull out these values as constants. best is if they could be computed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants