Update MDV integration roadmap - #30
Merged
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughThe MDV integration documentation is expanded from a brief checklist into a comprehensive multi-phase roadmap and compatibility strategy. It details package exports, a headless SpatialCanvas-backed MDV chart API, adapter patterns, feature-aware styling requirements, backend support discussions, and updated acceptance criteria. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
docs/docs/vis/mdv-integration.mdx (3)
55-56: ⚡ Quick winTimestamp volatile dependency-version statements.
These version assertions will age quickly; add “as of YYYY-MM-DD” so readers don’t treat them as evergreen facts.
Proposed doc adjustment
-- [ ] Align dependency versions before testing in MDV. This repo currently uses Viv `0.20.x` and deck.gl `9.1.x`; MDV currently declares Viv `0.19.x`. +- [ ] Align dependency versions before testing in MDV. As of 2026-04-30, this repo uses Viv `0.20.x` and deck.gl `9.1.x`; MDV declares Viv `0.19.x`. @@ -- Vitessce currently uses older Viv/deck/luma versions than this repo, with Viv `0.16.x`, deck.gl `8.8.x`, and luma.gl `8.5.x` in its workspace lockfile. +- As of 2026-04-30, Vitessce uses older Viv/deck/luma versions than this repo, with Viv `0.16.x`, deck.gl `8.8.x`, and luma.gl `8.5.x` in its workspace lockfile. @@ -- **Viv/deck version skew:** MDV uses Viv `0.19.x`; this repo uses Viv `0.20.x`. +- **Viv/deck version skew:** As of 2026-04-30, MDV uses Viv `0.19.x`; this repo uses Viv `0.20.x`.Also applies to: 180-181, 367-368
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/vis/mdv-integration.mdx` around lines 55 - 56, Update the dependency-version statements in the MDV integration notes to include an explicit snapshot date (e.g., “as of YYYY‑MM‑DD”) so they are not read as evergreen; modify the two bullet points that mention Viv `0.20.x`/`0.19.x`, deck.gl `9.1.x`, and the clean `pnpm build` instruction (and the repeated occurrences at the other noted locations) to append a timestamp phrase like “(as of 2026-04-30)”; ensure the phrasing is consistent across the three places referenced so readers know these versions are time-sensitive.
73-75: ⚡ Quick winPick one canonical prop name for injected deck.gl layers.
The draft alternates between
deckLayers(Line 73) andextraLayers(Line 85). Standardizing now prevents adapter churn and avoids public API ambiguity.Proposed doc adjustment
- deckLayers={mdvDeckLayers} + deckLayers={mdvDeckLayers} @@ -- [ ] Add `extraLayers` / `deckLayers` / `deckProps` so MDV can pass scatter points, gates, selection overlays, contours, ROI JSON, and custom tooltips. +- [ ] Add `deckLayers` and `deckProps` so MDV can pass scatter points, gates, selection overlays, contours, ROI JSON, and custom tooltips.Also applies to: 85-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/vis/mdv-integration.mdx` around lines 73 - 75, The docs currently use two different prop names for injected deck.gl layers—deckLayers and extraLayers—causing API ambiguity; pick one canonical name (e.g., deckLayers) and update this file so all occurrences and examples use that single symbol: replace extraLayers with deckLayers in the MDX content and any code examples (including references like mdvDeckLayers), and ensure matching prop name is used in adjacent examples/props (deckProps, renderTooltip) and any cross-references in the docs so the public API is consistent.
61-84: ⚡ Quick winReframe this phase as “stabilize/expose headless API,” not “add from scratch.”
Line 61 implies controlled embedding is missing, but the repo already has core controlled-state building blocks (
SpatialCanvasProviderexternal store +createSpatialCanvasStore). Adjust wording so this phase targets missing public surface/UI decoupling rather than re-implementing existing primitives.Proposed doc adjustment
- Add a controlled/headless rendering API to `@spatialdata/vis` so MDV can embed the viewer without this repo's UI. + Stabilize and expose a controlled/headless rendering API in `@spatialdata/vis` so MDV can embed the viewer without this repo's UI.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/vis/mdv-integration.mdx` around lines 61 - 84, Update the doc text to reframe this phase as “stabilize/expose headless API” instead of “add from scratch”: mention that controlled embedding already has core building blocks (SpatialCanvasProvider and createSpatialCanvasStore) and change the proposed work to focus on decoupling UI from viewer by exporting a stable headless surface (e.g., SpatialCanvasViewer or SpatialCanvasCore) that supports controlled coordinateSystem, layers, layerOrder, and viewState, while keeping the current zustand-driven UI as SpatialCanvas or SpatialCanvasEditor; replace wording that implies reimplementation with wording that emphasizes exposing and stabilizing the existing primitives and public surface.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/vis/mdv-integration.mdx`:
- Line 400: Update the user-facing text in the documentation entry that mentions
VivScatterComponent / scatter_state so the adjective is hyphenated: change "Long
term, scatter props should accept..." to "Long-term, scatter props should
accept..."; ensure the sentence referencing getVivId and the `@spatialdata/core`
TableElement stays unchanged except for the hyphenation.
---
Nitpick comments:
In `@docs/docs/vis/mdv-integration.mdx`:
- Around line 55-56: Update the dependency-version statements in the MDV
integration notes to include an explicit snapshot date (e.g., “as of
YYYY‑MM‑DD”) so they are not read as evergreen; modify the two bullet points
that mention Viv `0.20.x`/`0.19.x`, deck.gl `9.1.x`, and the clean `pnpm build`
instruction (and the repeated occurrences at the other noted locations) to
append a timestamp phrase like “(as of 2026-04-30)”; ensure the phrasing is
consistent across the three places referenced so readers know these versions are
time-sensitive.
- Around line 73-75: The docs currently use two different prop names for
injected deck.gl layers—deckLayers and extraLayers—causing API ambiguity; pick
one canonical name (e.g., deckLayers) and update this file so all occurrences
and examples use that single symbol: replace extraLayers with deckLayers in the
MDX content and any code examples (including references like mdvDeckLayers), and
ensure matching prop name is used in adjacent examples/props (deckProps,
renderTooltip) and any cross-references in the docs so the public API is
consistent.
- Around line 61-84: Update the doc text to reframe this phase as
“stabilize/expose headless API” instead of “add from scratch”: mention that
controlled embedding already has core building blocks (SpatialCanvasProvider and
createSpatialCanvasStore) and change the proposed work to focus on decoupling UI
from viewer by exporting a stable headless surface (e.g., SpatialCanvasViewer or
SpatialCanvasCore) that supports controlled coordinateSystem, layers,
layerOrder, and viewState, while keeping the current zustand-driven UI as
SpatialCanvas or SpatialCanvasEditor; replace wording that implies
reimplementation with wording that emphasizes exposing and stabilizing the
existing primitives and public surface.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40575463-11ab-4305-a395-9d55139b605d
📒 Files selected for processing (1)
docs/docs/vis/mdv-integration.mdx
This was referenced May 18, 2026
Merged
This was referenced Jun 5, 2026
This was referenced Jun 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by CodeRabbit