CODAP-296: auto-connect new graph to dataset with visible table/card#2417
CODAP-296: auto-connect new graph to dataset with visible table/card#2417
Conversation
…when multiple datasets exist Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
codap-v3
|
||||||||||||||||||||||||||||
| Project |
codap-v3
|
| Branch Review |
main
|
| Run status |
|
| Run duration | 02m 11s |
| Commit |
|
| Committer | eireland |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
4
|
| View all changes introduced in this branch ↗︎ | |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2417 +/- ##
==========================================
- Coverage 85.53% 83.53% -2.01%
==========================================
Files 756 756
Lines 41958 41975 +17
Branches 10293 10388 +95
==========================================
- Hits 35888 35062 -826
- Misses 6055 6896 +841
- Partials 15 17 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…orting appState singleton Adds IDocumentContentLike interface to tile-content-info.ts and threads documentContent through createTileSnapshotOfType/createTileOfType so that defaultContent callbacks can access document state without global imports. Moves isTileHidden views block earlier in DocumentContentModel to avoid needing a cast. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates graph tile creation to auto-connect to the most relevant dataset when multiple datasets exist, based on which dataset has a visible case table/card, while preserving the existing single-dataset auto-connect behavior.
Changes:
- Adds a lightweight
documentContentinterface to default tile creation options to support tile-visibility-based decisions without introducing circular dependencies. - Threads
documentContentthrough tile creation helpers so tiledefaultContentcan consider current document visibility state. - Updates graph tile
defaultContentto auto-connect to a dataset only when exactly one dataset has a visible case table/card (or when only one dataset exists total).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| v3/src/models/tiles/tile-content-info.ts | Adds IDocumentContentLike and extends default content options to include documentContent. |
| v3/src/models/document/document-content.ts | Exposes isTileHidden earlier and passes document content into tile snapshot creation. |
| v3/src/models/codap/create-tile.ts | Threads documentContent into defaultContent creation path by extending helper function signatures. |
| v3/src/models/codap/add-default-content.ts | Passes appState.document.content into tile creation so default tiles can inspect visibility. |
| v3/src/components/graph/graph-registration.ts | Implements dataset auto-connect selection logic based on visible table/card when multiple datasets exist. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
kswenson
left a comment
There was a problem hiding this comment.
👍 Looks good -- the following review was developed in conjunction with Claude Code 🤖.
PR Review Summary
Changes: 6 files, +41 / -28 lines (after reviewer commits)
What it does
When a user creates a new graph in a document with multiple datasets, the graph currently stays empty (no auto-connection). This PR adds logic so that if exactly one of those datasets has a visible (non-hidden) case table or case card tile, the new graph auto-connects to that dataset. The existing single-dataset auto-connect behavior is preserved.
The approach
The core change is in graph-registration.ts in the defaultContent callback. The original code checked sharedDataSets.length === 1 and used that dataset directly. The new code introduces a branching structure:
- Single dataset: Same behavior as before — auto-connect.
- Multiple datasets: Filter to datasets whose
DataSetMetadatahas acaseTableTileIdorcaseCardTileIdthat corresponds to a tile that exists and is not hidden. If exactly one dataset passes this filter, auto-connect to it; otherwise leave the graph empty.
Document content is accessed via options.documentContent (an IDocumentContentLike interface added to IDefaultContentOptions), threaded through the tile creation call chain from DocumentContentModel.createTile().
Assessment
The approach is sound and well-scoped. Edge cases are handled correctly (0, 1, 2+ qualifying datasets all behave sensibly).
Changes made during review
-
Threaded
documentContentthroughIDefaultContentOptionsinstead of importingappStatesingleton directly in graph-registration.ts. AddedIDocumentContentLikeinterface totile-content-info.ts, threaded throughcreateTileSnapshotOfType/createTileOfType, and updated callers indocument-content.tsandadd-default-content.ts. -
Moved
isTileHiddenviews block earlier inDocumentContentModel(before thecreateTileactions block) soselfnaturally has the view at the call site without needing a cast. -
Added consistent optional chaining for
content?.isTileHidden()calls to matchcontent?.getTile()usage.
Issues
No concerns. This is a clean, focused, well-tested fix.
Summary
Test plan
Fixes CODAP-296
🤖 Generated with Claude Code