CODAP-166: graph responds dynamically to attribute type changes

[bfinzer]

Summary

• When a user changes an attribute's type (e.g., numeric to categorical) via the case table or card view, the graph now dynamically reconfigures its axes and plot type
• Fixes stale D3 selection refs that caused crashes when transitioning between axis types
• Fixes inherited SVG attributes (e.g., fill="none") that made categorical labels invisible after switching from a numeric axis
• Caches axis place to prevent dead MST model access during undo

Fixes CODAP-166

Test plan

• Create a dataset with a numeric attribute and graph it on an axis
• Change the attribute type to "categorical" via the case table attribute properties — verify the graph reconfigures to a categorical axis
• Change back to "none" — verify it returns to numeric
• Test undo/redo of the type change
• Test with date type changes

🤖 Generated with Claude Code

[cypress]

codap-v3    Run #10347

Run Properties:   Passed #10347  •  a0d517a893: CODAP-166: graph responds dynamically to attribute type changes (#2415)

Project	codap-v3
Branch Review	main
Run status	Passed #10347
Run duration	05m 14s
Commit	a0d517a893: CODAP-166: graph responds dynamically to attribute type changes (#2415)
Committer	William Finzer
View all properties for this run ↗︎

---

Test results
Failures	0
Flaky	0
Pending	73
Skipped	0
Passing	300
View all changes introduced in this branch ↗︎

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.49%. Comparing base (0c10790) to head (a59d3e6).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2415      +/-   ##
==========================================
- Coverage   85.50%   85.49%   -0.01%     
==========================================
  Files         755      755              
  Lines       41908    41914       +6     
  Branches     9974    10368     +394     
==========================================
+ Hits        35832    35834       +2     
- Misses       6064     6068       +4     
  Partials       12       12              

Flag	Coverage Δ
cypress	69.21% <100.00%> (-0.02%)	⬇️
jest	57.03% <60.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[kswenson]

👍 Looks good -- the following review was developed in conjunction with Claude Code 🤖.

PR Review Summary

Changes: 3 files, +21 / -6 lines

What it does

When a user changes an attribute's type (e.g., numeric to categorical) via the case table or card view, the graph now dynamically reconfigures its axes and plot type. Previously, the graph would not respond to attribute type changes — only to attribute assignment changes. This also fixes three crash/display bugs that surfaced when transitioning between axis types: stale D3 selection refs, inherited SVG attributes making categorical labels invisible, and dead MST model access during undo.

The approach

Three targeted fixes across three files:

1. use-plot.ts (line 207-208): The existing mstReaction that watches attribute assignments now also watches attributeType() for each role. When the type changes, syncModelWithAttributeConfiguration reconfigures the axes and plot type. This is the core fix — it makes the graph reactive to type changes, not just assignment changes.

2. axis-helper.ts (lines 28, 35, 39-43, 47): Caches axisModel.place in a private _axisPlace field at construction time. The axisPlace getter now reads from the cache instead of the live MST model. This prevents crashes during undo when the axis model may already be removed from the MST tree. Additionally, the constructor now clears inherited SVG attributes (fill, font-size, font-family, text-anchor) from the axis <g> element. D3's numeric axis sets fill="none" on the group, and when switching to a categorical axis, the text labels inherit that fill and become invisible.

3. use-sub-axis.ts (lines 287-288): Clears categoriesSelectionRef and subAxisSelectionRef before constructing a new CategoricalAxisHelper. The AxisHelper constructor removes all SVG children, which detaches DOM elements. Without clearing these refs, other MobX reactions that fire synchronously could crash on detached DOM nodes.

Assessment

This is a well-targeted fix. Each of the three changes addresses a distinct, clearly described problem. The approach is sound:

• Extending the existing mstReaction to include attributeType is the minimal, correct way to make the graph responsive to type changes. The reaction already calls syncModelWithAttributeConfiguration, which handles the full axis/plot reconfiguration.
• Caching axisPlace is safe because axis place never changes after construction — it's determined by position in the document layout.
• Clearing SVG attributes and stale refs are defensive fixes for real crash paths.

Issues

Fixed during review:

• use-plot.ts: The attribute assignment/type reaction was missing equals: comparer.structural. The accessor returns a new array of [id, type] tuples on every call, so without structural comparison the reaction fires on every MobX transaction regardless of whether values actually changed. (This was a pre-existing issue — the original code also lacked it — but the PR made it slightly worse by nesting arrays.) Added equals: comparer.structural to the reaction options.

No other concerns.

[Copilot]

Pull request overview

This PR enables graphs to dynamically reconfigure when a user changes an attribute's type (e.g., from numeric to categorical) via the case table or card view. The changes fix crashes and rendering issues that occurred during these transitions.

Changes:

• Added tracking of attribute type changes in addition to attribute assignment changes, triggering graph reconfiguration when types change
• Fixed stale D3 selection references that caused crashes when transitioning between axis types by clearing refs before creating categorical axis helpers
• Fixed inherited SVG attributes and prevented dead MST model access during undo by caching axis place and clearing D3-set attributes

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
v3/src/components/graph/hooks/use-plot.ts	Added attribute type to the reaction that monitors attribute changes, enabling dynamic response when users change attribute types
v3/src/components/axis/hooks/use-sub-axis.ts	Clears stale D3 selection refs before creating CategoricalAxisHelper to prevent crashes from detached DOM nodes
v3/src/components/axis/helper-models/axis-helper.ts	Caches axis place to prevent dead MST model access during undo and clears inherited SVG attributes to fix rendering issues

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.