Repository navigation
Conversation
Contributor
📝 WalkthroughWalkthroughAdds split-map support: new Kepler slice methods for split-map and per-map layer toggles, default split-map UI state, a SplitMapIndexContext, split-aware rendering of multiple MapContainer instances, and legend components that filter layers by split panel index. Changes
Sequence DiagramsequenceDiagram
actor User
participant KeplerSlice as KeplerSlice (State)
participant KeplerMapContainer as KeplerMapContainer
participant SplitMapsContainer as SplitMapsContainer
participant MapContainer as MapContainer (per-split)
participant CustomMapLegend as CustomMapLegend
User->>KeplerSlice: toggleSplitMap(mapId[, index])
KeplerSlice->>KeplerSlice: register map if needed\nupdate splitMaps state
KeplerSlice-->>KeplerMapContainer: state change
KeplerMapContainer->>KeplerMapContainer: compute isSplit (splitMaps.length > 1)
alt isSplit = true
KeplerMapContainer->>SplitMapsContainer: render multiple MapContainer instances
SplitMapsContainer->>MapContainer: provide SplitMapIndexContext (index)
MapContainer->>CustomMapLegend: render with mapIndex from context
CustomMapLegend->>CustomMapLegend: filter layers by split panel membership
CustomMapLegend-->>User: display split-aware legend
else isSplit = false
KeplerMapContainer->>MapContainer: render single instance
MapContainer->>CustomMapLegend: render default legend
CustomMapLegend-->>User: display full legend
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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. 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/kepler/src/components/CustomMapLegendPanel.tsx (1)
12-21:⚠️ Potential issue | 🟡 MinorDon't forward
mapIndexintoMapLegendPanel.
mapIndexis a custom prop added here, but it's being spread ontoMapLegendPanel(from@kepler.gl/components), which doesn't declare it. At best it's ignored; at worst it propagates to a DOM node and triggers a React unknown-attribute warning. Follow the same destructure-then-spread pattern used inCustomMapLegend.tsx(lines 47–52).♻️ Proposed fix
- const CustomMapLegendPanel = ( - props: MapLegendPanelProps & {mapIndex?: number}, - ) => { + const CustomMapLegendPanel = ({ + mapIndex: _mapIndex, + ...props + }: MapLegendPanelProps & {mapIndex?: number}) => { return ( <> <style>{`.draggable-legend .map-control__panel-header { display: none !important; }`}</style> <MapLegendPanel {...props} /> </> ); };Note: if the intent is to have the panel actually consume
mapIndex(e.g., forward it down to the legend), you'll also need a wiring path — currently nothing reads it here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kepler/src/components/CustomMapLegendPanel.tsx` around lines 12 - 21, The CustomMapLegendPanel currently spreads props (MapLegendPanelProps & {mapIndex?: number}) directly into MapLegendPanel, which forwards the custom mapIndex prop into a component that doesn't expect it; fix this by destructuring mapIndex out of the incoming props in CustomMapLegendPanel (const {mapIndex, ...rest} = props) and then pass only the rest into MapLegendPanel (MapLegendPanel {...rest}) so mapIndex is not forwarded to MapLegendPanel or any DOM node; keep the existing style injection and only remove mapIndex from the spread.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/kepler/src/KeplerSlice.ts`:
- Around line 595-601: The implementation of toggleSplitMap currently defines
the parameter as (mapId, index) and uses an unsafe cast (index as number);
update the implementation signature to accept an optional index parameter
(mapId, index?) to match the public type (toggleSplitMap: (mapId: string,
index?: number) => void) and remove the unnecessary "as number" cast; ensure the
body still calls get().kepler.registerKeplerMapIfNotExists(mapId) and
get().kepler.dispatchAction(mapId, toggleSplitMapAction(index)) so callers may
omit index safely.
---
Outside diff comments:
In `@packages/kepler/src/components/CustomMapLegendPanel.tsx`:
- Around line 12-21: The CustomMapLegendPanel currently spreads props
(MapLegendPanelProps & {mapIndex?: number}) directly into MapLegendPanel, which
forwards the custom mapIndex prop into a component that doesn't expect it; fix
this by destructuring mapIndex out of the incoming props in CustomMapLegendPanel
(const {mapIndex, ...rest} = props) and then pass only the rest into
MapLegendPanel (MapLegendPanel {...rest}) so mapIndex is not forwarded to
MapLegendPanel or any DOM node; keep the existing style injection and only
remove mapIndex from the spread.
🪄 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: e81da3b2-22c5-4186-b7f1-8fef59d36a7b
📒 Files selected for processing (5)
packages/kepler/src/KeplerSlice.tspackages/kepler/src/components/CustomMapLegend.tsxpackages/kepler/src/components/CustomMapLegendPanel.tsxpackages/kepler/src/components/KeplerMapContainer.tsxpackages/kepler/src/index.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/kepler/src/components/CustomMapLegend.tsx (1)
98-108: Use a stable key for legend items.
visibleLayersis a derived/filtered list, sokey={index}ties React identity to position rather than to the layer. Toggling visibility, reordering layers, or switching split panels can shift indices and causeLayerLegendItem's localisExpandedstate to "leak" between layers. Preferlayer.id.♻️ Proposed fix
- {visibleLayers.map((layer, index) => { - return ( - <LayerLegendItem - key={index} - layer={layer} - containerW={containerW} - isExport={isExport} - {...restProps} - /> - ); - })} + {visibleLayers.map((layer) => ( + <LayerLegendItem + key={layer.id} + layer={layer} + containerW={containerW} + isExport={isExport} + {...restProps} + /> + ))}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/kepler/src/components/CustomMapLegend.tsx` around lines 98 - 108, The legend items use array index for React keys which causes identity to shift; update the mapping over visibleLayers so LayerLegendItem uses a stable unique key (e.g., layer.id) instead of key={index}. Locate the map where visibleLayers.map(...) renders <LayerLegendItem ... /> and replace the index-based key with the layer's unique identifier (layer.id) to prevent local state (like isExpanded) from leaking between different layers when order or filtering changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/kepler/src/components/CustomMapLegend.tsx`:
- Around line 98-108: The legend items use array index for React keys which
causes identity to shift; update the mapping over visibleLayers so
LayerLegendItem uses a stable unique key (e.g., layer.id) instead of
key={index}. Locate the map where visibleLayers.map(...) renders
<LayerLegendItem ... /> and replace the index-based key with the layer's unique
identifier (layer.id) to prevent local state (like isExpanded) from leaking
between different layers when order or filtering changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 204fac4a-a10d-4114-bfe8-f79300f07923
📒 Files selected for processing (5)
packages/kepler/src/KeplerSlice.tspackages/kepler/src/components/CustomMapLegend.tsxpackages/kepler/src/components/KeplerMapContainer.tsxpackages/kepler/src/components/SplitMapIndexContext.tspackages/kepler/src/index.ts
✅ Files skipped from review due to trivial changes (1)
- packages/kepler/src/components/SplitMapIndexContext.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/kepler/src/index.ts
- packages/kepler/src/KeplerSlice.ts
Collaborator
Author
ilyabo
approved these changes
Apr 27, 2026
lixun910
deleted the
562-keplerslice-add-splitmap-action-to-create-dual-map-view-in-kepler
branch
April 27, 2026 18:33
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
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 by CodeRabbit