Skip to content

Fix image layer resolution for controlled overlays - #38

Merged
xinaesthete merged 3 commits into
mainfrom
codex/audit-layer-prop-flow-design
May 27, 2026
Merged

Fix image layer resolution for controlled overlays#38
xinaesthete merged 3 commits into
mainfrom
codex/audit-layer-prop-flow-design

Conversation

@xinaesthete

@xinaesthete xinaesthete commented May 27, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Resolve SpatialCanvas layers through LayerConfig.elementKey instead of assuming the generated type:key layer id always matches the loaded element.
  • Keep the generated id lookup as a fallback so existing paths still work.
  • Add regression coverage for controlled layer ids and for multiple Viv image layers staying distinct in one viewport.
  • Document the points/GeoArrow migration path more explicitly in the vis docs.
  • Avoid issues with element bounds being re-computed when unrelated props change.

Testing

  • pnpm --filter @spatialdata/vis test -- spatialCanvasUtils.spec.ts vivSpatialViewer.spec.ts
  • pnpm --filter @spatialdata/vis build
  • pnpm --filter docs build
  • git diff --check

Summary by CodeRabbit

  • New Features

    • Configurable global layer ordering so image and non-image layers render in the configured sequence; viewer preserves distinct image layers and interleaves deck layers per order.
    • Improved single scale bar handling when multiple image layers are present.
  • Documentation

    • Added detailed points contract, migration roadmap, MDV/Parquet/Arrow guidance, and updated image layer prop-flow and SpatialCanvas limitations and checklist.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Refactors element lookup with exported resolveLayerElement, documents a points/core vs layers contract and MDV Arrow/Parquet guidance, formalizes Viv image adapter identity/prop-flow and audit checklist, and adds layerOrder-driven VivSpatialViewer composition plus tests.

Changes

Layer Element Resolution & Adapter Contracts

Layer / File(s) Summary
Points contract and roadmap
docs/docs/layers/overview.mdx, docs/docs/vis/mdv-integration.mdx, docs/docs/vis/spatial-canvas-status.mdx
Documents the split between @spatialdata/core (stable point ids, coordinate columns, row-index alignment, Arrow-ish batches) and @spatialdata/layers (rendering/filtering/backends), specifies initial ScatterplotLayer backend while keeping config representation-agnostic, and outlines incremental migration/testing for multipart Parquet and GeoArrow adoption.
Image layer prop flow and Viv adapter boundary
docs/docs/vis/layer-prop-flow.mdx
Declares Viv image layers as the adapter boundary with [loader, selections] as getTileData structural triggers, formalizes identity-stable wiring and viewer-local cache constraints, and provides an audit checklist focusing on tile-loading ownership and tile-read behavior tests.
Element resolution helper and useLayerData refactor
packages/vis/src/SpatialCanvas/useLayerData.ts
Adds resolveLayerElement(layerId, config, elementMap) and replaces direct elementMap.current.get(layerId) lookups throughout useLayerData: prebuilt-data invalidation, async load queuing, renderability, bounds, layer construction, loaded-data getters, tooltips, picks, and Viv props.
Viv layer ordering and wiring
packages/vis/src/SpatialCanvas/VivSpatialViewer.tsx, packages/vis/src/SpatialCanvas/SpatialViewer.tsx, packages/vis/src/SpatialCanvas/index.tsx, packages/vis/src/SpatialCanvas/SpatialCanvasViewer.tsx
Threads optional layerOrder?: string[] from SpatialCanvas into SpatialViewer → VivSpatialViewer; refactors VivSpatialViewer to build ordered layer records, capture/generate a single scale bar if needed, sort by layerOrder, and produce the final composed deck layers.
Tests for element resolution and image composition
packages/vis/tests/spatialCanvasUtils.spec.ts, packages/vis/tests/vivSpatialViewer.spec.ts
Adds unit tests for resolveLayerElement behavior (elementKey resolution and fallbacks) and integration tests asserting VivSpatialViewer preserves distinct image-layer ids, accepts image layers via layerFilter, and respects configured layerOrder when interleaving deck layers.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Poem

🐇 A rabbit hums of keys and maps,

Elements found where config taps,
Points and tiles in ordered queues,
Tests that prove the rendering rules,
Hopping on, the viewer stays in sync.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Fix image layer resolution for controlled overlays' directly describes the main change: resolving layers via LayerConfig.elementKey for controlled overlays, which is the core technical improvement across the changeset.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/audit-layer-prop-flow-design

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@xinaesthete
xinaesthete merged commit 6c9d243 into main May 27, 2026
2 checks passed
@xinaesthete
xinaesthete deleted the codex/audit-layer-prop-flow-design branch May 27, 2026 13:51
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.

1 participant