Skip to content

feat(dataweaver): Refactor and improve chart/data/store infrastructure and implement spawnable charts - #470

Open
nick-nlb wants to merge 12 commits into
datacommonsorg:mainfrom
nick-nlb:dw-chart-spawn
Open

nick-nlb wants to merge 12 commits into
datacommonsorg:mainfrom
nick-nlb:dw-chart-spawn

Conversation

@nick-nlb

Copy link
Copy Markdown
Collaborator

Description

The original goal of this PR was to add in the ability to generate time series charts by clicking entities in the choropleth charts (functionality seen in the demo).

However, work on this task highlighted the need to improve the overall store, chart and data architecture underlying the appllication, to make it cleaner, efficient and extensible. This became the primary focus of the PR.

As part of that effort, this PR:

  • Combines the chart derivation paths (deriveChartContent and deriveChartContentForVariable) into a single cohesive path that covers all views (multi-entity, single-entity and child-scoped)
  • Moves facets onto the series they belong to, removing the parallel map and the standalone facets prop. Every chart now renders from the one series array.
  • Fixes to assumptions that may break in edge cases (such as assuming index 0 holds data). Variables and facet defaults are resolved to the first entry that actually contains observations. No longer register cards when charts cannot be derived.
  • Centralizes shape id construction and into its own importable function.
  • Cleaned up relationship between store key and tldraw id
  • Bumps state version to 2 (now with the id scheme having changed, old imports will no longer work).
  • Improves comments for stretches of complicated functionality.

In addition, the PR implements the card spawning itself (leaving accessibility as a follow-up). When a choropleth map entity is hovered, it is also clickable. Clicking the entity will produce a new card with the full time series associated with that entity. These individual charts can then be combined and manipulated like normal charts.

Issues

Address tracker items 48 and 49.

Upcoming Issues

A follow-up to this will be the TODO added into this PR, to make the functionality associated with the spawning keyboard, etc, accessible

Screenshots

image

Unify chart card architecture by standardizing all charts onto uniform `ChartSeries` arrays, and moving observation facets directly into series models, general series functionality consolidation.
…re `focusTarget` receives the parent shape ID for camera framing, and prevent double-prefixing during canvas card creation.
…t facet resolution, conversion of inline strings to constants, and various cleanup items. Add comments

@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 refactors the chart card state management and rendering logic in the Atlas workspace. Key changes include simplifying the facet data structure by embedding facets directly within chart series, introducing a canonical buildCardShapeId helper to prevent duplicate cards, and adding support for child-place scoped cards (via resultPlaceDcid) which allows drilling down into specific entities (e.g., from a choropleth map click). It also bumps the serialized state version to 2 and introduces comprehensive unit tests for facet resolution, shape ID generation, and store synchronization. I have no feedback to provide as there are no review comments to assess.

@nick-nlb
nick-nlb marked this pull request as ready for review September 11, 2026 20:41
@nick-nlb nick-nlb changed the title feat(data-weaver): Refactor and improve chart/data/store infrastructure and implement spawnable charts feat(dataweaver): Refactor and improve chart/data/store infrastructure and implement spawnable charts Sep 11, 2026
@nick-nlb
nick-nlb requested a review from juliawu as a code owner September 15, 2026 04:24
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