From e28161145c61ce7ae59b489aee7b5ed07ac4c12d Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Sat, 16 May 2026 02:36:07 +0300 Subject: [PATCH 1/8] fix: fix: prevent unnecessary re-renders Signed-off-by: Ihor Dykhta --- src/components/src/container.tsx | 13 +- src/components/src/kepler-gl.tsx | 158 +++++++++++++++++------ src/components/src/loading-indicator.tsx | 6 +- src/components/src/map-container.tsx | 8 +- src/components/src/side-panel.tsx | 80 +++++++++++- 5 files changed, 216 insertions(+), 49 deletions(-) diff --git a/src/components/src/container.tsx b/src/components/src/container.tsx index dafa6a9bc3..b94ffef6c5 100644 --- a/src/components/src/container.tsx +++ b/src/components/src/container.tsx @@ -20,7 +20,18 @@ export const ERROR_MSG = { const mapStateToProps = (state: any, props: ContainerProps) => ({state, ...props}); const dispatchToProps = (dispatch: Dispatch) => ({dispatch}); -const connector = connect(mapStateToProps, dispatchToProps); +const connector = connect(mapStateToProps, dispatchToProps, null, { + areStatesEqual: (next: any, prev: any, nextOwnProps: ContainerProps) => { + const getState = nextOwnProps.getState || ((s: any) => s.keplerGl); + const id = nextOwnProps.id || 'map'; + const nextInstance = getState(next)?.[id]; + const prevInstance = getState(prev)?.[id]; + if (!prevInstance && !nextInstance) { + return next === prev; + } + return nextInstance === prevInstance; + } +}); type ContainerProps = { id: string; diff --git a/src/components/src/kepler-gl.tsx b/src/components/src/kepler-gl.tsx index 281fa64d74..7121399da0 100644 --- a/src/components/src/kepler-gl.tsx +++ b/src/components/src/kepler-gl.tsx @@ -207,32 +207,79 @@ export function getVisibleDatasets(datasets) { return filterObjectByPredicate(datasets, key => key !== GEOCODER_DATASET_NAME); } -export const sidePanelSelector = (props: KeplerGLProps, availableProviders, filteredDatasets) => ({ - appName: props.appName ? props.appName : DEFAULT_KEPLER_GL_PROPS.appName, - version: props.version ? props.version : DEFAULT_KEPLER_GL_PROPS.version, - appWebsite: props.appWebsite, - mapStyle: props.mapStyle, - onSaveMap: props.onSaveMap, - uiState: props.uiState, - mapStyleActions: props.mapStyleActions, - visStateActions: props.visStateActions, - uiStateActions: props.uiStateActions, - mapStateActions: props.mapStateActions, - - datasets: filteredDatasets, - filters: props.visState.filters, - layers: props.visState.layers, - layerOrder: props.visState.layerOrder, - layerClasses: props.visState.layerClasses, - interactionConfig: props.visState.interactionConfig, - mapInfo: props.visState.mapInfo, - layerBlending: props.visState.layerBlending, - overlayBlending: props.visState.overlayBlending, - - width: props.sidePanelWidth ? props.sidePanelWidth : DEFAULT_KEPLER_GL_PROPS.width, - availableProviders, - mapSaved: props.providerState.mapSaved -}); +export const sidePanelSelector = createSelector( + [ + (props: KeplerGLProps) => props.appName, + (props: KeplerGLProps) => props.version, + (props: KeplerGLProps) => props.appWebsite, + (props: KeplerGLProps) => props.mapStyle, + (props: KeplerGLProps) => props.onSaveMap, + (props: KeplerGLProps) => props.uiState, + (props: KeplerGLProps) => props.mapStyleActions, + (props: KeplerGLProps) => props.visStateActions, + (props: KeplerGLProps) => props.uiStateActions, + (props: KeplerGLProps) => props.mapStateActions, + (props: KeplerGLProps) => props.visState.filters, + (props: KeplerGLProps) => props.visState.layers, + (props: KeplerGLProps) => props.visState.layerOrder, + (props: KeplerGLProps) => props.visState.layerClasses, + (props: KeplerGLProps) => props.visState.interactionConfig, + (props: KeplerGLProps) => props.visState.mapInfo, + (props: KeplerGLProps) => props.visState.layerBlending, + (props: KeplerGLProps) => props.visState.overlayBlending, + (props: KeplerGLProps) => props.sidePanelWidth, + (props: KeplerGLProps) => props.providerState.mapSaved, + (_props: KeplerGLProps, availableProviders) => availableProviders, + (_props: KeplerGLProps, _availableProviders, filteredDatasets) => filteredDatasets + ], + ( + appName, + version, + appWebsite, + mapStyle, + onSaveMap, + uiState, + mapStyleActions, + visStateActions, + uiStateActions, + mapStateActions, + filters, + layers, + layerOrder, + layerClasses, + interactionConfig, + mapInfo, + layerBlending, + overlayBlending, + sidePanelWidth, + mapSaved, + availableProviders, + filteredDatasets + ) => ({ + appName: appName ? appName : DEFAULT_KEPLER_GL_PROPS.appName, + version: version ? version : DEFAULT_KEPLER_GL_PROPS.version, + appWebsite, + mapStyle, + onSaveMap, + uiState, + mapStyleActions, + visStateActions, + uiStateActions, + mapStateActions, + datasets: filteredDatasets, + filters, + layers, + layerOrder, + layerClasses, + interactionConfig, + mapInfo, + layerBlending, + overlayBlending, + width: sidePanelWidth ? sidePanelWidth : DEFAULT_KEPLER_GL_PROPS.width, + availableProviders, + mapSaved + }) +); export const plotContainerSelector = (props: KeplerGLProps) => ({ width: props.width, @@ -255,16 +302,41 @@ export const plotContainerSelector = (props: KeplerGLProps) => ({ export const isSplitSelector = (props: KeplerGLProps) => props.visState.splitMaps && props.visState.splitMaps.length > 1; -export const bottomWidgetSelector = (props: KeplerGLProps, theme) => ({ - filters: props.visState.filters, - datasets: props.visState.datasets, - uiState: props.uiState, - layers: props.visState.layers, - animationConfig: props.visState.animationConfig, - visStateActions: props.visStateActions, - toggleModal: props.uiStateActions.toggleModal, - sidePanelWidth: props.uiState.readOnly ? 0 : props.sidePanelWidth + theme.sidePanel.margin.left -}); +export const bottomWidgetSelector = createSelector( + [ + (props: KeplerGLProps) => props.visState.filters, + (props: KeplerGLProps) => props.visState.datasets, + (props: KeplerGLProps) => props.uiState, + (props: KeplerGLProps) => props.visState.layers, + (props: KeplerGLProps) => props.visState.animationConfig, + (props: KeplerGLProps) => props.visStateActions, + (props: KeplerGLProps) => props.uiStateActions.toggleModal, + (props: KeplerGLProps) => props.uiState.readOnly, + (props: KeplerGLProps) => props.sidePanelWidth, + (_props: KeplerGLProps, theme) => theme + ], + ( + filters, + datasets, + uiState, + layers, + animationConfig, + visStateActions, + toggleModal, + readOnly, + sidePanelWidth, + theme + ) => ({ + filters, + datasets, + uiState, + layers, + animationConfig, + visStateActions, + toggleModal, + sidePanelWidth: readOnly ? 0 : sidePanelWidth + theme.sidePanel.margin.left + }) +); export const modalContainerSelector = (props: KeplerGLProps, rootNode) => ({ appName: props.appName ? props.appName : DEFAULT_KEPLER_GL_PROPS.appName, @@ -413,10 +485,16 @@ export const attributionSelector = createSelector( } ); -export const notificationPanelSelector = (props: KeplerGLProps) => ({ - removeNotification: props.uiStateActions.removeNotification, - notifications: props.uiState.notifications -}); +export const notificationPanelSelector = createSelector( + [ + (props: KeplerGLProps) => props.uiStateActions.removeNotification, + (props: KeplerGLProps) => props.uiState.notifications + ], + (removeNotification, notifications) => ({ + removeNotification, + notifications + }) +); export const DEFAULT_KEPLER_GL_PROPS = { mapStyles: [], diff --git a/src/components/src/loading-indicator.tsx b/src/components/src/loading-indicator.tsx index 056f1a223e..2fec1f9b81 100644 --- a/src/components/src/loading-indicator.tsx +++ b/src/components/src/loading-indicator.tsx @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright contributors to the kepler.gl project -import React, {PropsWithChildren, useRef, useEffect} from 'react'; +import React, {PropsWithChildren, useRef, useEffect, memo} from 'react'; import styled, {withTheme, keyframes} from 'styled-components'; import {getNumRasterTilesBeingLoaded, getNumVectorTilesBeingLoaded} from '@kepler.gl/layers'; @@ -109,4 +109,6 @@ const LoadingIndicator: React.FC = ({ ); }; -export default withTheme(LoadingIndicator) as React.FC>; +const MemoizedLoadingIndicator = memo(LoadingIndicator); + +export default withTheme(MemoizedLoadingIndicator) as React.FC>; diff --git a/src/components/src/map-container.tsx b/src/components/src/map-container.tsx index 09b7b3e43f..b79bf5abc2 100644 --- a/src/components/src/map-container.tsx +++ b/src/components/src/map-container.tsx @@ -243,7 +243,7 @@ type AttributionProps = { baseMapLibraryConfig: BaseMapLibraryConfig; }; -export const Attribution: React.FC = ({ +export const Attribution: React.FC = React.memo(({ showBaseMapLibLogo = true, showOsmBasemapAttribution = false, datasetAttributions, @@ -311,7 +311,7 @@ export const Attribution: React.FC = ({ ]); return memoizedComponents; -}; +}); const StyledAttributionLogoContainer = styled.div<{$left: number}>` position: absolute; @@ -339,7 +339,7 @@ type AttributionLogosProps = { const LOGO_LEFT_ADJUSTMENT = 3; -export const AttributionLogos: React.FC = ({ +export const AttributionLogos: React.FC = React.memo(({ logos, activeSidePanel, sidePanelWidth @@ -365,7 +365,7 @@ export const AttributionLogos: React.FC = ({ ))} ); -}; +}); MapContainerFactory.deps = [MapPopoverFactory, MapControlFactory, EditorFactory]; diff --git a/src/components/src/side-panel.tsx b/src/components/src/side-panel.tsx index ea616c0105..737b71100d 100644 --- a/src/components/src/side-panel.tsx +++ b/src/components/src/side-panel.tsx @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright contributors to the kepler.gl project -import React, {useCallback, useMemo} from 'react'; +import React, {memo, useCallback, useMemo} from 'react'; import { EXPORT_DATA_ID, @@ -248,5 +248,81 @@ export default function SidePanelFactory( }; SidePanel.defaultPanels = fullPanels; - return SidePanel; + + const areSidePanelPropsEqual = (prev: SidePanelProps, next: SidePanelProps): boolean => { + const keys = Object.keys(next) as (keyof SidePanelProps)[]; + for (const key of keys) { + if (prev[key] === next[key]) continue; + + if (key === 'filters') { + const pf = prev.filters; + const nf = next.filters; + if (pf?.length !== nf?.length) return false; + for (let i = 0; i < nf.length; i++) { + if (pf[i] === nf[i]) continue; + if (pf[i].id !== nf[i].id) return false; + if (pf[i].name !== nf[i].name) return false; + if (pf[i].type !== nf[i].type) return false; + if (pf[i].dataId !== nf[i].dataId) return false; + if (pf[i].view !== nf[i].view) return false; + if (pf[i].enabled !== nf[i].enabled) return false; + if (pf[i].plotType !== nf[i].plotType) return false; + if ((pf[i] as any).animationWindow !== (nf[i] as any).animationWindow) return false; + if (pf[i].speed !== nf[i].speed) return false; + if (pf[i].gpu !== nf[i].gpu) return false; + } + continue; + } + + if (key === 'datasets') { + const pd = prev.datasets; + const nd = next.datasets; + const pKeys = Object.keys(pd || {}); + const nKeys = Object.keys(nd || {}); + if (pKeys.length !== nKeys.length) return false; + for (const dk of nKeys) { + if (!pd?.[dk]) return false; + if (pd[dk] === nd[dk]) continue; + if (pd[dk].id !== nd[dk].id) return false; + if (pd[dk].label !== nd[dk].label) return false; + if (pd[dk].color !== nd[dk].color) return false; + if (pd[dk].fields !== nd[dk].fields) return false; + if (pd[dk].dataContainer !== nd[dk].dataContainer) return false; + } + continue; + } + + if (key === 'layers') { + const pl = prev.layers; + const nl = next.layers; + if (pl?.length !== nl?.length) return false; + for (let i = 0; i < nl.length; i++) { + if (pl[i] === nl[i]) continue; + if (pl[i].id !== nl[i].id) return false; + if (pl[i].type !== nl[i].type) return false; + const pc = pl[i].config; + const nc = nl[i].config; + if (pc === nc) continue; + if (pc.label !== nc.label) return false; + if (pc.isVisible !== nc.isVisible) return false; + if (pc.isConfigActive !== nc.isConfigActive) return false; + if (pc.dataId !== nc.dataId) return false; + if (pc.color !== nc.color) return false; + if (pc.columns !== nc.columns) return false; + if (pc.visConfig !== nc.visConfig) return false; + } + continue; + } + + return false; + } + return true; + }; + + const MemoizedSidePanel = memo(SidePanel, areSidePanelPropsEqual) as React.NamedExoticComponent & { + defaultPanels: SidePanelProps['panels']; + }; + MemoizedSidePanel.displayName = 'SidePanel'; + (MemoizedSidePanel as any).defaultPanels = fullPanels; + return MemoizedSidePanel; } From 1af25b512d6cf0825d05232bd36e5fe1ac01d7d6 Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Sat, 16 May 2026 04:00:13 +0300 Subject: [PATCH 2/8] better checks Signed-off-by: Ihor Dykhta --- src/components/src/bottom-widget.tsx | 15 ++++-- src/components/src/map/map-control.tsx | 63 +++++++++++++++++++++++++- src/components/src/side-panel.tsx | 3 ++ 3 files changed, 74 insertions(+), 7 deletions(-) diff --git a/src/components/src/bottom-widget.tsx b/src/components/src/bottom-widget.tsx index db23757e0c..e27fc26dc3 100644 --- a/src/components/src/bottom-widget.tsx +++ b/src/components/src/bottom-widget.tsx @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright contributors to the kepler.gl project -import React, {forwardRef, useMemo, useCallback} from 'react'; +import React, {forwardRef, memo, useMemo, useCallback} from 'react'; import styled, {withTheme, IStyledComponent} from 'styled-components'; import {FILTER_VIEW_TYPES, EXPORT_VIDEO_ID} from '@kepler.gl/constants'; @@ -232,10 +232,15 @@ export default function BottomWidgetFactory( ); }; - return withTheme( - forwardRef((props: BottomWidgetThemedProps, ref: React.ForwardedRef) => ( - - )) + const MemoizedBottomWidget = memo(BottomWidget); + + const ForwardedBottomWidget = forwardRef( + (props: BottomWidgetThemedProps, ref: React.ForwardedRef) => ( + + ) ); + ForwardedBottomWidget.displayName = 'BottomWidget'; + + return withTheme(ForwardedBottomWidget); } /* eslint-enable complexity */ diff --git a/src/components/src/map/map-control.tsx b/src/components/src/map/map-control.tsx index 1802a30e44..67736db29d 100644 --- a/src/components/src/map/map-control.tsx +++ b/src/components/src/map/map-control.tsx @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright contributors to the kepler.gl project -import React from 'react'; +import React, {memo} from 'react'; import styled from 'styled-components'; import KeplerGlLogo from '../common/logo'; @@ -143,7 +143,66 @@ function MapControlFactory( MapControl.displayName = 'MapControl'; - return MapControl; + const areMapControlPropsEqual = (prev: MapControlProps, next: MapControlProps): boolean => { + const keys = Object.keys(next) as (keyof MapControlProps)[]; + for (const key of keys) { + if (prev[key] === next[key]) continue; + + if (key === 'layers') { + const pl = prev.layers; + const nl = next.layers; + if (!pl || !nl || pl.length !== nl.length) return false; + for (let i = 0; i < nl.length; i++) { + if (pl[i] === nl[i]) continue; + if (pl[i].id !== nl[i].id) return false; + if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; + if (pl[i].config.label !== nl[i].config.label) return false; + if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; + } + continue; + } + + if (key === 'datasets') { + const pd = prev.datasets; + const nd = next.datasets; + if (!pd || !nd) return false; + const pKeys = Object.keys(pd); + const nKeys = Object.keys(nd); + if (pKeys.length !== nKeys.length) return false; + for (const dk of nKeys) { + if (!pd[dk]) return false; + if (pd[dk] === nd[dk]) continue; + if (pd[dk].id !== nd[dk].id) return false; + if (pd[dk].label !== nd[dk].label) return false; + if (pd[dk].color !== nd[dk].color) return false; + } + continue; + } + + if (key === 'layersToRender') { + const pl = prev.layersToRender; + const nl = next.layersToRender; + if (!pl || !nl) return false; + const pKeys = Object.keys(pl); + const nKeys = Object.keys(nl); + if (pKeys.length !== nKeys.length) return false; + for (const lk of nKeys) { + if (pl[lk] !== nl[lk]) return false; + } + continue; + } + + return false; + } + return true; + }; + + const MemoizedMapControl = memo(MapControl, areMapControlPropsEqual) as React.NamedExoticComponent & { + defaultActionComponents: MapControlProps['actionComponents']; + }; + (MemoizedMapControl as any).defaultActionComponents = DEFAULT_ACTIONS; + + return MemoizedMapControl; } export default MapControlFactory; diff --git a/src/components/src/side-panel.tsx b/src/components/src/side-panel.tsx index 737b71100d..2c55ad037d 100644 --- a/src/components/src/side-panel.tsx +++ b/src/components/src/side-panel.tsx @@ -270,6 +270,9 @@ export default function SidePanelFactory( if ((pf[i] as any).animationWindow !== (nf[i] as any).animationWindow) return false; if (pf[i].speed !== nf[i].speed) return false; if (pf[i].gpu !== nf[i].gpu) return false; + if ((nf[i] as any).isAnimating && nf[i].view !== 'enlarged') { + if (pf[i].value !== nf[i].value) return false; + } } continue; } From 712b5523732db745d09746c8d89b2cc04acceec4 Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Sat, 16 May 2026 04:41:45 +0300 Subject: [PATCH 3/8] better checks Signed-off-by: Ihor Dykhta --- src/components/src/side-panel.tsx | 19 +++++++------------ 1 file changed, 7 insertions(+), 12 deletions(-) diff --git a/src/components/src/side-panel.tsx b/src/components/src/side-panel.tsx index 2c55ad037d..075b96d75c 100644 --- a/src/components/src/side-panel.tsx +++ b/src/components/src/side-panel.tsx @@ -258,6 +258,7 @@ export default function SidePanelFactory( const pf = prev.filters; const nf = next.filters; if (pf?.length !== nf?.length) return false; + const isFilterPanelOpen = (next as any).uiState?.activeSidePanel === 'filter'; for (let i = 0; i < nf.length; i++) { if (pf[i] === nf[i]) continue; if (pf[i].id !== nf[i].id) return false; @@ -270,7 +271,7 @@ export default function SidePanelFactory( if ((pf[i] as any).animationWindow !== (nf[i] as any).animationWindow) return false; if (pf[i].speed !== nf[i].speed) return false; if (pf[i].gpu !== nf[i].gpu) return false; - if ((nf[i] as any).isAnimating && nf[i].view !== 'enlarged') { + if (isFilterPanelOpen && nf[i].view !== 'enlarged') { if (pf[i].value !== nf[i].value) return false; } } @@ -296,23 +297,17 @@ export default function SidePanelFactory( } if (key === 'layers') { + const isLayerPanelOpen = (next as any).uiState?.activeSidePanel === 'layer'; + if (isLayerPanelOpen) { + const filtersAlsoChanged = prev.filters !== next.filters; + if (!filtersAlsoChanged) return false; + } const pl = prev.layers; const nl = next.layers; if (pl?.length !== nl?.length) return false; for (let i = 0; i < nl.length; i++) { - if (pl[i] === nl[i]) continue; if (pl[i].id !== nl[i].id) return false; if (pl[i].type !== nl[i].type) return false; - const pc = pl[i].config; - const nc = nl[i].config; - if (pc === nc) continue; - if (pc.label !== nc.label) return false; - if (pc.isVisible !== nc.isVisible) return false; - if (pc.isConfigActive !== nc.isConfigActive) return false; - if (pc.dataId !== nc.dataId) return false; - if (pc.color !== nc.color) return false; - if (pc.columns !== nc.columns) return false; - if (pc.visConfig !== nc.visConfig) return false; } continue; } From 44817e303c04858f3f7c18e31c52f0df9bb57259 Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Sun, 2 Aug 2026 06:14:29 +0300 Subject: [PATCH 4/8] refactor Signed-off-by: Ihor Dykhta --- src/components/src/container.tsx | 5 ++ src/components/src/kepler-gl.tsx | 110 +++++++---------------- src/components/src/loading-indicator.tsx | 6 +- src/components/src/map/map-control.tsx | 4 + src/components/src/side-panel.tsx | 37 ++++++-- 5 files changed, 76 insertions(+), 86 deletions(-) diff --git a/src/components/src/container.tsx b/src/components/src/container.tsx index 580a47f8ee..5ddb209023 100644 --- a/src/components/src/container.tsx +++ b/src/components/src/container.tsx @@ -22,11 +22,16 @@ export const ERROR_MSG = { const mapStateToProps = (state: any, props: ContainerProps) => ({state, ...props}); const dispatchToProps = (dispatch: Dispatch) => ({dispatch}); const connector = connect(mapStateToProps, dispatchToProps, null, { + // Skip re-render when this kepler.gl instance slice is unchanged. + // The outer Container only uses `state` for the existence check (line below). + // All actual data subscriptions are handled by the inner KeplerGL via + // keplerGlConnect, which has its own independent Redux subscription. areStatesEqual: (next: any, prev: any, nextOwnProps: ContainerProps) => { const getState = nextOwnProps.getState || ((s: any) => s.keplerGl); const id = nextOwnProps.id || 'map'; const nextInstance = getState(next)?.[id]; const prevInstance = getState(prev)?.[id]; + // If neither instance exists yet, fall back to full equality check if (!prevInstance && !nextInstance) { return next === prev; } diff --git a/src/components/src/kepler-gl.tsx b/src/components/src/kepler-gl.tsx index bb7b7dd2d6..785279c5a3 100644 --- a/src/components/src/kepler-gl.tsx +++ b/src/components/src/kepler-gl.tsx @@ -207,82 +207,40 @@ export function getVisibleDatasets(datasets) { return filterObjectByPredicate(datasets, key => key !== GEOCODER_DATASET_NAME); } -export const sidePanelSelector = createSelector( - [ - (props: KeplerGLProps) => props.appName, - (props: KeplerGLProps) => props.version, - (props: KeplerGLProps) => props.appWebsite, - (props: KeplerGLProps) => props.mapStyle, - (props: KeplerGLProps) => props.mapState, - (props: KeplerGLProps) => props.onSaveMap, - (props: KeplerGLProps) => props.uiState, - (props: KeplerGLProps) => props.mapStyleActions, - (props: KeplerGLProps) => props.visStateActions, - (props: KeplerGLProps) => props.uiStateActions, - (props: KeplerGLProps) => props.mapStateActions, - (props: KeplerGLProps) => props.visState.filters, - (props: KeplerGLProps) => props.visState.layers, - (props: KeplerGLProps) => props.visState.layerOrder, - (props: KeplerGLProps) => props.visState.layerClasses, - (props: KeplerGLProps) => props.visState.interactionConfig, - (props: KeplerGLProps) => props.visState.mapInfo, - (props: KeplerGLProps) => props.visState.layerBlending, - (props: KeplerGLProps) => props.visState.overlayBlending, - (props: KeplerGLProps) => props.sidePanelWidth, - (props: KeplerGLProps) => props.providerState.mapSaved, - (_props: KeplerGLProps, availableProviders) => availableProviders, - (_props: KeplerGLProps, _availableProviders, filteredDatasets) => filteredDatasets - ], - ( - appName, - version, - appWebsite, - mapStyle, - mapState, - onSaveMap, - uiState, - mapStyleActions, - visStateActions, - uiStateActions, - mapStateActions, - filters, - layers, - layerOrder, - layerClasses, - interactionConfig, - mapInfo, - layerBlending, - overlayBlending, - sidePanelWidth, - mapSaved, - availableProviders, - filteredDatasets - ) => ({ - appName: appName ? appName : DEFAULT_KEPLER_GL_PROPS.appName, - version: version ? version : DEFAULT_KEPLER_GL_PROPS.version, - appWebsite, - mapStyle, - mapState, - onSaveMap, - uiState, - mapStyleActions, - visStateActions, - uiStateActions, - mapStateActions, - datasets: filteredDatasets, - filters, - layers, - layerOrder, - layerClasses, - interactionConfig, - mapInfo, - layerBlending, - overlayBlending, - width: sidePanelWidth ? sidePanelWidth : DEFAULT_KEPLER_GL_PROPS.width, - availableProviders, - mapSaved - }) -); +// sidePanelSelector is a plain function (not memoized at this level) because: +// 1. It receives `availableProviders` and `filteredDatasets` as extra args, which are +// already individually memoized at the instance level in KeplerGL. +// 2. A module-level createSelector with extra positional args only caches one result, +// so it would thrash on every render with multiple instances or changing extras. +export const sidePanelSelector = ( + props: KeplerGLProps, + availableProviders, + filteredDatasets +) => ({ + appName: props.appName ? props.appName : DEFAULT_KEPLER_GL_PROPS.appName, + version: props.version ? props.version : DEFAULT_KEPLER_GL_PROPS.version, + appWebsite: props.appWebsite, + mapStyle: props.mapStyle, + mapState: props.mapState, + onSaveMap: props.onSaveMap, + uiState: props.uiState, + mapStyleActions: props.mapStyleActions, + visStateActions: props.visStateActions, + uiStateActions: props.uiStateActions, + mapStateActions: props.mapStateActions, + datasets: filteredDatasets, + filters: props.visState.filters, + layers: props.visState.layers, + layerOrder: props.visState.layerOrder, + layerClasses: props.visState.layerClasses, + interactionConfig: props.visState.interactionConfig, + mapInfo: props.visState.mapInfo, + layerBlending: props.visState.layerBlending, + overlayBlending: props.visState.overlayBlending, + width: props.sidePanelWidth ? props.sidePanelWidth : DEFAULT_KEPLER_GL_PROPS.width, + availableProviders, + mapSaved: props.providerState.mapSaved +}); export const plotContainerSelector = (props: KeplerGLProps) => ({ width: props.width, diff --git a/src/components/src/loading-indicator.tsx b/src/components/src/loading-indicator.tsx index dae3c62a57..02a05b9eb2 100644 --- a/src/components/src/loading-indicator.tsx +++ b/src/components/src/loading-indicator.tsx @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT // Copyright contributors to the kepler.gl project -import React, {PropsWithChildren, useRef, useEffect, memo} from 'react'; +import React, {PropsWithChildren, useRef, useEffect} from 'react'; import styled, {withTheme, keyframes} from 'styled-components'; import {getNumRasterTilesBeingLoaded, getNumVectorTilesBeingLoaded} from '@kepler.gl/layers'; @@ -113,6 +113,4 @@ const LoadingIndicator: React.FC = ({ ); }; -const MemoizedLoadingIndicator = memo(LoadingIndicator); - -export default withTheme(MemoizedLoadingIndicator) as React.FC>; +export default withTheme(LoadingIndicator) as React.FC>; diff --git a/src/components/src/map/map-control.tsx b/src/components/src/map/map-control.tsx index 0aec5d8689..b8c95036d9 100644 --- a/src/components/src/map/map-control.tsx +++ b/src/components/src/map/map-control.tsx @@ -176,9 +176,13 @@ function MapControlFactory( for (let i = 0; i < nl.length; i++) { if (pl[i] === nl[i]) continue; if (pl[i].id !== nl[i].id) return false; + if (pl[i].type !== nl[i].type) return false; if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; if (pl[i].config.label !== nl[i].config.label) return false; if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; + // color is shown in the legend panel + if (pl[i].config.color !== nl[i].config.color) return false; + if (pl[i].config.highlightColor !== nl[i].config.highlightColor) return false; } continue; } diff --git a/src/components/src/side-panel.tsx b/src/components/src/side-panel.tsx index 3f3769b3aa..e26af380a4 100644 --- a/src/components/src/side-panel.tsx +++ b/src/components/src/side-panel.tsx @@ -273,7 +273,9 @@ export default function SidePanelFactory( if ((pf[i] as any).animationWindow !== (nf[i] as any).animationWindow) return false; if (pf[i].speed !== nf[i].speed) return false; if (pf[i].gpu !== nf[i].gpu) return false; - if (isFilterPanelOpen && nf[i].view !== 'enlarged') { + // Always check value when the filter panel is open so the panel stays up-to-date. + // Outside the panel, suppress value changes to avoid re-renders during animation. + if (isFilterPanelOpen) { if (pf[i].value !== nf[i].value) return false; } } @@ -299,21 +301,44 @@ export default function SidePanelFactory( } if (key === 'layers') { - const isLayerPanelOpen = (next as any).uiState?.activeSidePanel === 'layer'; - if (isLayerPanelOpen) { - const filtersAlsoChanged = prev.filters !== next.filters; - if (!filtersAlsoChanged) return false; - } const pl = prev.layers; const nl = next.layers; if (pl?.length !== nl?.length) return false; + const isLayerPanelOpen = (next as any).uiState?.activeSidePanel === 'layer'; for (let i = 0; i < nl.length; i++) { + if (pl[i] === nl[i]) continue; if (pl[i].id !== nl[i].id) return false; if (pl[i].type !== nl[i].type) return false; + if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; + if (pl[i].config.label !== nl[i].config.label) return false; + // When the layer panel is open, also check config fields visible in the panel UI + if (isLayerPanelOpen) { + if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; + if (pl[i].config.color !== nl[i].config.color) return false; + if (pl[i].config.highlightColor !== nl[i].config.highlightColor) return false; + if (pl[i].config.visConfig !== nl[i].config.visConfig) return false; + if (pl[i].config.dataId !== nl[i].config.dataId) return false; + if (pl[i].config.columns !== nl[i].config.columns) return false; + } } continue; } + if (key === 'layerOrder') { + // layerOrder changes during drag-and-drop reordering — always re-render + return false; + } + + if (key === 'mapState') { + // mapState changes on every pan/zoom frame (latitude, longitude, zoom, bearing, pitch). + // SidePanel only cares about globe.enabled (used in MapManager to conditionally + // show the globe settings panel). Suppress all other mapState changes. + const pm = prev.mapState; + const nm = next.mapState; + if (pm?.globe?.enabled !== nm?.globe?.enabled) return false; + continue; + } + return false; } return true; From 0681350197ba971a911ce16d6c58ba55781aede8 Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Sun, 2 Aug 2026 20:30:01 +0300 Subject: [PATCH 5/8] follow up Signed-off-by: Ihor Dykhta --- src/components/src/bottom-widget.tsx | 15 +- src/components/src/kepler-gl.tsx | 60 ++- src/components/src/map-container.tsx | 172 -------- src/components/src/map/map-control.spec.tsx | 239 ++++++++++++ src/components/src/map/map-control.tsx | 231 ++++++++--- src/components/src/side-panel.spec.tsx | 412 ++++++++++++++++++++ src/components/src/side-panel.tsx | 303 +++++++++----- 7 files changed, 1069 insertions(+), 363 deletions(-) create mode 100644 src/components/src/map/map-control.spec.tsx create mode 100644 src/components/src/side-panel.spec.tsx diff --git a/src/components/src/bottom-widget.tsx b/src/components/src/bottom-widget.tsx index 8797c85bdf..f0b9a72e85 100644 --- a/src/components/src/bottom-widget.tsx +++ b/src/components/src/bottom-widget.tsx @@ -232,15 +232,22 @@ export default function BottomWidgetFactory( ); }; - const MemoizedBottomWidget = memo(BottomWidget); + // Wrap order matters for memo to be effective: + // 1. withTheme — injects `theme` prop from styled-components context + // 2. forwardRef — converts the React ref to the `rootRef` prop + // 3. memo — outermost guard; sees stable props after theme injection + // + // If memo wrapped a component that still had withTheme outside it, withTheme + // would produce a new props object on every render and bust the memo cache. + const ThemedBottomWidget = withTheme(BottomWidget); const ForwardedBottomWidget = forwardRef( - (props: BottomWidgetThemedProps, ref: React.ForwardedRef) => ( - + (props: Omit, ref: React.ForwardedRef) => ( + ) ); ForwardedBottomWidget.displayName = 'BottomWidget'; - return withTheme(ForwardedBottomWidget); + return memo(ForwardedBottomWidget) as unknown as React.FC; } /* eslint-enable complexity */ diff --git a/src/components/src/kepler-gl.tsx b/src/components/src/kepler-gl.tsx index 785279c5a3..8682f03fad 100644 --- a/src/components/src/kepler-gl.tsx +++ b/src/components/src/kepler-gl.tsx @@ -263,41 +263,23 @@ export const plotContainerSelector = (props: KeplerGLProps) => ({ export const isSplitSelector = (props: KeplerGLProps) => props.visState.splitMaps && props.visState.splitMaps.length > 1; -export const bottomWidgetSelector = createSelector( - [ - (props: KeplerGLProps) => props.visState.filters, - (props: KeplerGLProps) => props.visState.datasets, - (props: KeplerGLProps) => props.uiState, - (props: KeplerGLProps) => props.visState.layers, - (props: KeplerGLProps) => props.visState.animationConfig, - (props: KeplerGLProps) => props.visStateActions, - (props: KeplerGLProps) => props.uiStateActions.toggleModal, - (props: KeplerGLProps) => props.uiState.readOnly, - (props: KeplerGLProps) => props.sidePanelWidth, - (_props: KeplerGLProps, theme) => theme - ], - ( - filters, - datasets, - uiState, - layers, - animationConfig, - visStateActions, - toggleModal, - readOnly, - sidePanelWidth, - theme - ) => ({ - filters, - datasets, - uiState, - layers, - animationConfig, - visStateActions, - toggleModal, - sidePanelWidth: readOnly ? 0 : sidePanelWidth + theme.sidePanel.margin.left - }) -); +// bottomWidgetSelector is a plain function (not memoized at this level) because: +// 1. It takes `theme` as a second positional argument. +// 2. A module-level createSelector with extra positional args only caches one result, +// so it would thrash on every render with multiple instances or changing themes. +// Individual fields (filters, layers, etc.) are already memoized by Redux selectors. +export const bottomWidgetSelector = (props: KeplerGLProps, theme) => ({ + filters: props.visState.filters, + datasets: props.visState.datasets, + uiState: props.uiState, + layers: props.visState.layers, + animationConfig: props.visState.animationConfig, + visStateActions: props.visStateActions, + toggleModal: props.uiStateActions.toggleModal, + sidePanelWidth: props.uiState.readOnly + ? 0 + : props.sidePanelWidth + theme.sidePanel.margin.left +}); export const modalContainerSelector = (props: KeplerGLProps, rootNode) => ({ appName: props.appName ? props.appName : DEFAULT_KEPLER_GL_PROPS.appName, @@ -540,7 +522,13 @@ function KeplerGlFactory( SidePanel: ReturnType, PlotContainer: ReturnType, NotificationPanel: ReturnType, - DndContext: ReturnType + DndContext: ReturnType, + // EffectManager is listed in deps so the injector graph includes it and + // consumers can override it via injectComponents([EffectManagerFactory, Custom]). + // KeplerGl itself does not render EffectManager — that is handled by + // MapControl in custom factory overrides (see examples/demo-app). + // eslint-disable-next-line @typescript-eslint/no-unused-vars + _EffectManager: ReturnType ): React.ComponentType KeplerGlState}> { /** @typedef {import('./kepler-gl').UnconnectedKeplerGlProps} KeplerGlProps */ /** @augments React.Component */ diff --git a/src/components/src/map-container.tsx b/src/components/src/map-container.tsx index 529c2213c3..58e5ddca87 100644 --- a/src/components/src/map-container.tsx +++ b/src/components/src/map-container.tsx @@ -205,178 +205,6 @@ export const Droppable = ({containerId}) => { // re-exported here to preserve the public import path (@kepler.gl/components). export {Attribution, AttributionLogos, renderBasemapAttribution, dedupeBasemapAttributions}; -const StyledDatasetAttributionsContainer = styled.div` - max-width: ${props => (props.isPalm ? '200px' : '300px')}; - text-overflow: ellipsis; - white-space: nowrap; - overflow: hidden; - color: ${props => props.theme.labelColor}; - margin-right: 2px; - margin-bottom: 1px; - line-height: ${props => (props.isPalm ? '1em' : '1.4em')}; - - &:hover { - white-space: inherit; - } -`; - -const DatasetAttributions = ({ - datasetAttributions, - isPalm -}: { - datasetAttributions: DatasetAttribution[]; - isPalm: boolean; -}) => ( - <> - {datasetAttributions?.length ? ( - - {datasetAttributions.map((ds, idx) => ( - - {ds.title} - {idx !== datasetAttributions.length - 1 ? ', ' : null} - - ))} - - ) : null} - -); - -type AttributionProps = { - showBaseMapLibLogo: boolean; - showOsmBasemapAttribution: boolean; - datasetAttributions: DatasetAttribution[]; - baseMapLibraryConfig: BaseMapLibraryConfig; -}; - -export const Attribution: React.FC = React.memo(({ - showBaseMapLibLogo = true, - showOsmBasemapAttribution = false, - datasetAttributions, - baseMapLibraryConfig -}: AttributionProps) => { - const isPalm = hasMobileWidth(breakPointValues); - - const memoizedComponents = useMemo(() => { - if (!showBaseMapLibLogo) { - return ( - - - -
- {datasetAttributions?.length ? | : null} - - © kepler.gl - -
- - - ); - } - - return ( - - - -
- {datasetAttributions?.length ? | : null} - {isPalm ? : null} - - © kepler.gl - - {showOsmBasemapAttribution ? ( - <> - | - - © OpenStreetMap - - - ) : null} - | - {!isPalm ? : null} -
- - - ); - }, [ - showBaseMapLibLogo, - showOsmBasemapAttribution, - datasetAttributions, - isPalm, - baseMapLibraryConfig - ]); - - return memoizedComponents; -}); - -const StyledAttributionLogoContainer = styled.div<{$left: number}>` - position: absolute; - bottom: ${props => props.theme.sidePanel.margin.left}px; - left: ${props => props.$left}px; - z-index: 1; - display: flex; - align-items: flex-end; - gap: 4px; - pointer-events: auto; - transition: left 250ms ease-in-out; -`; - -const StyledLogoLink = styled.a<{$enabled: boolean}>` - cursor: ${props => (props.$enabled ? 'pointer' : 'default')}; - display: flex; - align-items: flex-end; -`; - -type AttributionLogosProps = { - logos: AttributionWithStyle[]; - activeSidePanel?: boolean; - sidePanelWidth?: number; -}; - -const LOGO_LEFT_ADJUSTMENT = 3; - -export const AttributionLogos: React.FC = React.memo(({ - logos, - activeSidePanel, - sidePanelWidth -}) => { - const theme = useTheme() as any; - const left = - (activeSidePanel ? (sidePanelWidth || 0) + LOGO_LEFT_ADJUSTMENT : 0) + - theme.sidePanel.margin.left; - - if (!logos?.length) return null; - return ( - - {logos.map((logo, idx) => ( - - {logo.title} - - ))} - - ); -}); - MapContainerFactory.deps = [MapPopoverFactory, MapControlFactory, EditorFactory, MapScaleFactory]; type MapboxStyle = string | object | undefined; diff --git a/src/components/src/map/map-control.spec.tsx b/src/components/src/map/map-control.spec.tsx new file mode 100644 index 0000000000..903fd1e972 --- /dev/null +++ b/src/components/src/map/map-control.spec.tsx @@ -0,0 +1,239 @@ +// SPDX-License-Identifier: MIT +// Copyright contributors to the kepler.gl project + +import {areMapControlPropsEqual, MapControlProps} from './map-control'; + +// ─── Helpers ─────────────────────────────────────────────────────────────── + +function makeLayer(overrides: Record = {}): any { + return { + id: 'l1', + type: 'point', + config: { + isVisible: true, + label: 'Layer 1', + isConfigActive: false, + color: [255, 0, 0], + highlightColor: [0, 255, 0] + }, + ...overrides + }; +} + +function makeDataset(overrides: Record = {}): any { + return { + id: 'ds1', + label: 'Dataset 1', + color: [0, 0, 255], + ...overrides + }; +} + +// Stable shared objects — all tests that don't override a specific prop +// will share these references, so unrelated props always pass reference equality. +const STABLE = { + callbacks: { + onTogglePerspective: jest.fn(), + onToggleSplitMap: jest.fn() as any, + onToggleSplitMapViewport: jest.fn(), + onMapToggleLayer: jest.fn(), + onToggleMapControl: jest.fn(), + onSetEditorMode: jest.fn(), + onToggleEditorVisibility: jest.fn(), + onLayerVisConfigChange: jest.fn(), + onSetLocale: jest.fn() as any, + setMapControlSettings: jest.fn() as any + }, + datasets: {} as any, + layers: [] as any[], + layerOrder: [] as any[], + layersToRender: {} as {[key: string]: boolean}, + mapControls: {} as any, + editor: {} as any +}; + +function baseProps(): MapControlProps { + return { + ...STABLE.callbacks, + datasets: STABLE.datasets, + dragRotate: false, + isSplit: false, + primary: true, + layers: STABLE.layers, + layerOrder: STABLE.layerOrder, + layersToRender: STABLE.layersToRender, + mapIndex: 0, + mapControls: STABLE.mapControls, + top: 0, + availableLocales: ['en'], + locale: 'en', + activeSidePanel: null, + editor: STABLE.editor + }; +} + +/** + * Create a prev/next pair where only the specified key differs. + * Both objects share the same base so all other prop references are identical. + */ +function pair( + key: keyof MapControlProps, + prevVal: any, + nextVal: any +): [MapControlProps, MapControlProps] { + const base = baseProps(); + const prev = {...base, [key]: prevVal}; + const next = {...base, [key]: nextVal}; + return [prev, next]; +} + +const isEqual = areMapControlPropsEqual; + +// ═══════════════════════════════════════════════════════════════════════════ +// Baseline +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areMapControlPropsEqual — baseline', () => { + test('returns true when props are identical references', () => { + const props = baseProps(); + expect(isEqual(props, props)).toBe(true); + }); + + test('returns false when a simple scalar prop changes (e.g. locale)', () => { + const [prev, next] = pair('locale', 'en', 'fr'); + expect(isEqual(prev, next)).toBe(false); + }); + + test('returns false when isSplit changes', () => { + const [prev, next] = pair('isSplit', false, true); + expect(isEqual(prev, next)).toBe(false); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// layers +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areMapControlPropsEqual — layers', () => { + test('re-renders when a layer is added', () => { + const [prev, next] = pair('layers', [], [makeLayer()]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.isVisible changes', () => { + const layer = makeLayer(); + const [prev, next] = pair('layers', [layer], [ + {...layer, config: {...layer.config, isVisible: false}} + ]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.label changes', () => { + const layer = makeLayer(); + const [prev, next] = pair('layers', [layer], [ + {...layer, config: {...layer.config, label: 'New'}} + ]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.color changes (legend color)', () => { + const layer = makeLayer(); + const [prev, next] = pair('layers', [layer], [ + {...layer, config: {...layer.config, color: [0, 255, 0]}} + ]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.highlightColor changes', () => { + const layer = makeLayer(); + const [prev, next] = pair('layers', [layer], [ + {...layer, config: {...layer.config, highlightColor: [255, 0, 255]}} + ]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.isConfigActive changes', () => { + const layer = makeLayer(); + const [prev, next] = pair('layers', [layer], [ + {...layer, config: {...layer.config, isConfigActive: true}} + ]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when layer object ref changes but all checked fields are the same', () => { + const layer = makeLayer(); + const [prev, next] = pair('layers', [layer], [{...layer}]); // spread = new object, same values + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// datasets +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areMapControlPropsEqual — datasets', () => { + test('re-renders when a dataset is added', () => { + const [prev, next] = pair('datasets', {}, {ds1: makeDataset()}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when dataset label changes', () => { + const ds = makeDataset({label: 'old'}); + const [prev, next] = pair('datasets', {ds1: ds}, {ds1: {...ds, label: 'new'}}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when dataset color changes', () => { + const ds = makeDataset({color: [255, 0, 0]}); + const [prev, next] = pair('datasets', {ds1: ds}, {ds1: {...ds, color: [0, 255, 0]}}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when dataset object is recreated with same values', () => { + const ds = makeDataset(); + const [prev, next] = pair('datasets', {ds1: ds}, {ds1: {...ds}}); + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// layersToRender +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areMapControlPropsEqual — layersToRender', () => { + test('re-renders when a layer visibility entry changes', () => { + const [prev, next] = pair('layersToRender', {l1: true}, {l1: false}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when a new layer visibility entry is added', () => { + const [prev, next] = pair('layersToRender', {l1: true}, {l1: true, l2: true}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when layersToRender object is recreated with same values', () => { + const [prev, next] = pair('layersToRender', {l1: true, l2: false}, {l1: true, l2: false}); + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// mapControls (unhandled prop — reference equality) +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areMapControlPropsEqual — mapControls (reference equality)', () => { + test('re-renders when mapControls reference changes', () => { + const [prev, next] = pair( + 'mapControls', + {mapLegend: {show: true}} as any, + {mapLegend: {show: true}} as any // new object + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when mapControls reference is the same', () => { + const mapControls = {mapLegend: {show: true}} as any; + const [prev, next] = pair('mapControls', mapControls, mapControls); + expect(isEqual(prev, next)).toBe(true); + }); +}); diff --git a/src/components/src/map/map-control.tsx b/src/components/src/map/map-control.tsx index b8c95036d9..f74be847b6 100644 --- a/src/components/src/map/map-control.tsx +++ b/src/components/src/map/map-control.tsx @@ -94,6 +94,179 @@ export type MapControlProps = { onToggleLayerForMap?: (mapIndex: number, layerId: string) => void; }; +/** + * Custom `React.memo` comparator for `MapControl`. + * + * ───────────────────────────────────────────────────────────────────────────── + * WHY THIS EXISTS + * ───────────────────────────────────────────────────────────────────────────── + * `MapControl` sits inside `MapContainer`, which re-renders on every Redux + * state update (including animation ticks and mouse-move events). Most of + * those updates are irrelevant to the map controls UI. This comparator + * prevents `MapControl` from re-rendering unless something visible in the + * controls (layer legend, dataset list, viewport sync button, etc.) actually + * changed. + * + * ───────────────────────────────────────────────────────────────────────────── + * HOW IT WORKS + * ───────────────────────────────────────────────────────────────────────────── + * The comparator iterates every key in `next`. For each key: + * • If `prev[key] === next[key]` → skip (no change, no re-render needed). + * • If the key has a custom handler below → apply field-level comparison + * that only checks fields actually rendered by MapControl. + * • Otherwise → `return false` (unrecognised change, always re-render). + * + * Returning `true` = props are "equal" → React skips the re-render. + * Returning `false` = props differ → React re-renders the component. + * + * ───────────────────────────────────────────────────────────────────────────── + * PROP-BY-PROP CONTRACT (covers every key in MapControlProps) + * ───────────────────────────────────────────────────────────────────────────── + * + * dragRotate, isSplit, primary — value equality (fallthrough) + * mapIndex, top — value equality (fallthrough) + * locale, availableLocales — value equality (fallthrough) + * activeSidePanel — value equality (fallthrough) + * readOnly, scale, mapHeight — value equality (fallthrough) + * mapViewMode — value equality (fallthrough) + * isExport — value equality (fallthrough) + * logoComponent — reference equality (fallthrough) + * actionComponents — reference equality (fallthrough) + * NOTE: pass a stable reference — a new + * `[...]` literal on every render defeats + * the optimisation. + * mapControls — reference equality (fallthrough) + * Redux produces a new reference when any + * map control setting changes, so this is + * correct. + * mapState, mapStateActions — reference equality (fallthrough) + * mapState changes on every animation frame + * during pan/zoom; MapControl does NOT + * receive mapState directly — it receives + * specific action dispatchers. If mapState + * is ever passed directly, add a handler + * like the one in areSidePanelPropsEqual. + * splitMaps — reference equality (fallthrough) + * editor — reference equality (fallthrough) + * onTogglePerspective, onToggleSplitMap, + * onToggleSplitMapViewport, onMapToggleLayer, + * onToggleMapControl, onSetEditorMode, + * onToggleEditorVisibility, + * onLayerVisConfigChange, + * onToggleLayerVisibility, onSetLocale, + * onSetMapSplitMode, onSetMapViewMode, + * setMapControlSettings, + * onToggleLayerForMap — reference equality (fallthrough) + * Callbacks should be stable references + * (e.g. from Redux bindActionCreators or + * useCallback). + * mapLayers — reference equality (fallthrough) + * + * layers — PARTIALLY COMPARED + * Checks: id, type, config.isVisible, + * config.label, config.isConfigActive, + * config.color, config.highlightColor. + * These are the fields shown in the legend. + * Other config fields (e.g. visConfig, + * columns) are not rendered by MapControl + * and are intentionally suppressed. + * + * datasets — PARTIALLY COMPARED + * Checks per dataset: id, label, color. + * The dataset list is shown in the legend. + * Heavy fields (fields array, dataContainer) + * are not rendered and are suppressed. + * + * layersToRender — DEEP-COMPARED + * Each `layerId → boolean` entry is compared + * individually. A change in any entry + * triggers a re-render (layer visibility in + * the legend). + * + * layerOrder — reference equality (fallthrough) + * MapControl renders layers in `layers` order, + * not `layerOrder` order, so layerOrder + * changes do not require special handling. + * + * ───────────────────────────────────────────────────────────────────────────── + * MAINTENANCE RULES — READ BEFORE EDITING + * ───────────────────────────────────────────────────────────────────────────── + * 1. Adding a prop to MapControlProps: + * • If it is a stable scalar, callback, or Redux-slice reference: + * the fallthrough `return false` handles it correctly. + * No change needed here, but add a row to the prop table above. + * • If it is a large object with irrelevant high-frequency sub-fields: + * add an explicit handler that only checks the fields MapControl + * actually renders. + * + * 2. Adding a field rendered in the legend to `layers` or `datasets`: + * Add it to the respective handler so changes to that field trigger + * a re-render. + * + * 3. Never remove a check without verifying the legend still updates + * correctly when that field changes in a real browser session. + * + * 4. All comparator behaviour is covered by areMapControlPropsEqual tests in + * map-control.spec.tsx. Add a test for every new case. + */ +export const areMapControlPropsEqual = (prev: MapControlProps, next: MapControlProps): boolean => { + const keys = Object.keys(next) as (keyof MapControlProps)[]; + for (const key of keys) { + if (prev[key] === next[key]) continue; + + if (key === 'layers') { + const pl = prev.layers; + const nl = next.layers; + if (!pl || !nl || pl.length !== nl.length) return false; + for (let i = 0; i < nl.length; i++) { + if (pl[i] === nl[i]) continue; + if (pl[i].id !== nl[i].id) return false; + if (pl[i].type !== nl[i].type) return false; + if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; + if (pl[i].config.label !== nl[i].config.label) return false; + if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; + // color is shown in the legend panel + if (pl[i].config.color !== nl[i].config.color) return false; + if (pl[i].config.highlightColor !== nl[i].config.highlightColor) return false; + } + continue; + } + + if (key === 'datasets') { + const pd = prev.datasets; + const nd = next.datasets; + if (!pd || !nd) return false; + const pKeys = Object.keys(pd); + const nKeys = Object.keys(nd); + if (pKeys.length !== nKeys.length) return false; + for (const dk of nKeys) { + if (!pd[dk]) return false; + if (pd[dk] === nd[dk]) continue; + if (pd[dk].id !== nd[dk].id) return false; + if (pd[dk].label !== nd[dk].label) return false; + if (pd[dk].color !== nd[dk].color) return false; + } + continue; + } + + if (key === 'layersToRender') { + const pl = prev.layersToRender; + const nl = next.layersToRender; + if (!pl || !nl) return false; + const pKeys = Object.keys(pl); + const nKeys = Object.keys(nl); + if (pKeys.length !== nKeys.length) return false; + for (const lk of nKeys) { + if (pl[lk] !== nl[lk]) return false; + } + continue; + } + + return false; + } + return true; +}; + MapControlFactory.deps = [ SplitMapButtonFactory, Toggle3dButtonFactory, @@ -164,64 +337,6 @@ function MapControlFactory( MapControl.displayName = 'MapControl'; - const areMapControlPropsEqual = (prev: MapControlProps, next: MapControlProps): boolean => { - const keys = Object.keys(next) as (keyof MapControlProps)[]; - for (const key of keys) { - if (prev[key] === next[key]) continue; - - if (key === 'layers') { - const pl = prev.layers; - const nl = next.layers; - if (!pl || !nl || pl.length !== nl.length) return false; - for (let i = 0; i < nl.length; i++) { - if (pl[i] === nl[i]) continue; - if (pl[i].id !== nl[i].id) return false; - if (pl[i].type !== nl[i].type) return false; - if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; - if (pl[i].config.label !== nl[i].config.label) return false; - if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; - // color is shown in the legend panel - if (pl[i].config.color !== nl[i].config.color) return false; - if (pl[i].config.highlightColor !== nl[i].config.highlightColor) return false; - } - continue; - } - - if (key === 'datasets') { - const pd = prev.datasets; - const nd = next.datasets; - if (!pd || !nd) return false; - const pKeys = Object.keys(pd); - const nKeys = Object.keys(nd); - if (pKeys.length !== nKeys.length) return false; - for (const dk of nKeys) { - if (!pd[dk]) return false; - if (pd[dk] === nd[dk]) continue; - if (pd[dk].id !== nd[dk].id) return false; - if (pd[dk].label !== nd[dk].label) return false; - if (pd[dk].color !== nd[dk].color) return false; - } - continue; - } - - if (key === 'layersToRender') { - const pl = prev.layersToRender; - const nl = next.layersToRender; - if (!pl || !nl) return false; - const pKeys = Object.keys(pl); - const nKeys = Object.keys(nl); - if (pKeys.length !== nKeys.length) return false; - for (const lk of nKeys) { - if (pl[lk] !== nl[lk]) return false; - } - continue; - } - - return false; - } - return true; - }; - const MemoizedMapControl = memo(MapControl, areMapControlPropsEqual) as React.NamedExoticComponent & { defaultActionComponents: MapControlProps['actionComponents']; }; diff --git a/src/components/src/side-panel.spec.tsx b/src/components/src/side-panel.spec.tsx new file mode 100644 index 0000000000..55e0ac360c --- /dev/null +++ b/src/components/src/side-panel.spec.tsx @@ -0,0 +1,412 @@ +// SPDX-License-Identifier: MIT +// Copyright contributors to the kepler.gl project + +// Mock the heavy factory dependencies that side-panel.tsx imports transitively. +// We only need to test the pure comparator function, not render the component. +jest.mock('./side-panel/layer-manager', () => () => null); +jest.mock('./side-panel/filter-manager', () => () => null); +jest.mock('./side-panel/interaction-manager', () => () => null); +jest.mock('./side-panel/map-manager', () => () => null); +jest.mock('./side-panel/custom-panel', () => { + const factory = () => null; + factory.panels = []; + factory.getProps = () => ({}); + return factory; +}); +jest.mock('./side-panel/side-bar', () => () => null); +jest.mock('./side-panel/panel-header', () => () => null); +jest.mock('./side-panel/panel-toggle', () => () => null); + +import {areSidePanelPropsEqual} from './side-panel'; +import {SidePanelProps} from './types'; + +// ─── Helpers ─────────────────────────────────────────────────────────────── + +function makeUiState(activeSidePanel: string | null = null): any { + return {activeSidePanel}; +} + +function makeFilter(overrides: Record = {}): any { + return { + id: 'f1', + name: 'my filter', + type: 'range', + dataId: ['ds1'], + view: 'side', + enabled: true, + plotType: {type: 'histogram'}, + animationWindow: 'free', + speed: 1, + gpu: false, + value: [0, 100], + ...overrides + }; +} + +function makeLayer(overrides: Record = {}): any { + return { + id: 'l1', + type: 'point', + config: { + isVisible: true, + label: 'My Layer', + isConfigActive: false, + color: [255, 0, 0], + highlightColor: [0, 255, 0], + visConfig: {}, + dataId: 'ds1', + columns: {} + }, + ...overrides + }; +} + +function makeDataset(overrides: Record = {}): any { + const fields = [{name: 'col1'}]; + const dataContainer = {}; + return { + id: 'ds1', + label: 'Dataset 1', + color: [0, 0, 255], + fields, + dataContainer, + ...overrides + }; +} + +// Stable shared objects used as defaults — all tests share these references +// so props that aren't under test always pass the reference-equality check. +const STABLE = { + actions: { + uiStateActions: {} as any, + visStateActions: {} as any, + mapStateActions: {} as any, + mapStyleActions: {} as any + }, + filters: [] as any[], + layers: [] as any[], + layerOrder: [] as any[], + layerClasses: {}, + interactionConfig: {} as any, + mapInfo: {}, + mapStyle: {} as any, + datasets: {}, + availableProviders: {} +}; + +/** Build a base props object. All unspecified props share stable references. */ +function baseProps(): SidePanelProps { + return { + ...STABLE.actions, + appName: 'kepler.gl', + appWebsite: 'https://kepler.gl', + version: '3.0.0', + filters: STABLE.filters, + layers: STABLE.layers, + layerOrder: STABLE.layerOrder, + layerClasses: STABLE.layerClasses, + layerBlending: 'normal', + overlayBlending: 'normal', + interactionConfig: STABLE.interactionConfig, + mapInfo: STABLE.mapInfo, + mapStyle: STABLE.mapStyle, + mapState: undefined, + datasets: STABLE.datasets, + uiState: makeUiState() as any, + availableProviders: STABLE.availableProviders, + mapSaved: null, + width: 300, + onSaveMap: undefined + } as SidePanelProps; +} + +/** + * Create a prev/next pair where only the specified key differs. + * Both objects share the same base so all other prop references are identical. + */ +function pair(key: keyof SidePanelProps, prevVal: any, nextVal: any): [SidePanelProps, SidePanelProps] { + const base = baseProps(); + const sharedUiState = base.uiState; + const prev = {...base, uiState: sharedUiState, [key]: prevVal}; + const next = {...base, uiState: sharedUiState, [key]: nextVal}; + return [prev, next]; +} + +function pairWithUiState( + key: keyof SidePanelProps, + prevVal: any, + nextVal: any, + activeSidePanel: string | null +): [SidePanelProps, SidePanelProps] { + const base = baseProps(); + const sharedUiState = makeUiState(activeSidePanel) as any; + const prev = {...base, uiState: sharedUiState, [key]: prevVal}; + const next = {...base, uiState: sharedUiState, [key]: nextVal}; + return [prev, next]; +} + +const isEqual = areSidePanelPropsEqual; + +// ═══════════════════════════════════════════════════════════════════════════ +// Baseline +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — baseline', () => { + test('returns true when props are identical references', () => { + const props = baseProps(); + expect(isEqual(props, props)).toBe(true); + }); + + test('returns false when a simple scalar prop changes', () => { + const [prev, next] = pair('appName', 'kepler.gl', 'my-app'); + expect(isEqual(prev, next)).toBe(false); + }); + + test('returns true when all props share the same references', () => { + const prev = baseProps(); + const next = {...prev}; // shallow copy — all refs identical + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// Animation / mousemove suppression +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — animation & mousemove suppression', () => { + test('does NOT re-render when only filter value changes (animation tick)', () => { + const filter = makeFilter({value: [0, 100]}); + const [prev, next] = pair('filters', [filter], [{...filter, value: [0, 50]}]); + expect(isEqual(prev, next)).toBe(true); + }); + + test('does NOT re-render when mapState lat/lng/zoom changes (map pan)', () => { + const [prev, next] = pair( + 'mapState', + {latitude: 37.7, longitude: -122.4, zoom: 10, globe: {enabled: false}}, + {latitude: 37.8, longitude: -122.3, zoom: 11, globe: {enabled: false}} + ); + expect(isEqual(prev, next)).toBe(true); + }); + + test('does NOT re-render when mapState bearing/pitch changes', () => { + const [prev, next] = pair( + 'mapState', + {bearing: 0, pitch: 0, globe: {enabled: false}}, + {bearing: 45, pitch: 30, globe: {enabled: false}} + ); + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// mapState — globe toggle +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — mapState.globe.enabled', () => { + test('re-renders when globe is enabled', () => { + const [prev, next] = pair( + 'mapState', + {latitude: 0, longitude: 0, zoom: 2, globe: {enabled: false}}, + {latitude: 0, longitude: 0, zoom: 2, globe: {enabled: true}} + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when globe is disabled', () => { + const [prev, next] = pair( + 'mapState', + {globe: {enabled: true}}, + {globe: {enabled: false}} + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when mapState is undefined on both sides', () => { + const [prev, next] = pair('mapState', undefined, undefined); + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// filters +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — filters', () => { + test('re-renders when a filter is added', () => { + const [prev, next] = pair('filters', [], [makeFilter()]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when filter.enabled changes', () => { + const filter = makeFilter({enabled: true}); + const [prev, next] = pair('filters', [filter], [{...filter, enabled: false}]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when filter.view changes', () => { + const filter = makeFilter({view: 'side'}); + const [prev, next] = pair('filters', [filter], [{...filter, view: 'enlarged'}]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when filter.value changes and filter panel is open', () => { + const filter = makeFilter({value: [0, 100]}); + const [prev, next] = pairWithUiState( + 'filters', + [filter], + [{...filter, value: [10, 90]}], + 'filter' + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when filter.value changes and filter panel is closed', () => { + const filter = makeFilter({value: [0, 100]}); + const [prev, next] = pairWithUiState( + 'filters', + [filter], + [{...filter, value: [10, 90]}], + null + ); + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// layers +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — layers', () => { + test('re-renders when a layer is added', () => { + const [prev, next] = pair('layers', [], [makeLayer()]); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.isVisible changes', () => { + const layer = makeLayer(); + const [prev, next] = pair( + 'layers', + [layer], + [{...layer, config: {...layer.config, isVisible: false}}] + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.label changes', () => { + const layer = makeLayer(); + const [prev, next] = pair( + 'layers', + [layer], + [{...layer, config: {...layer.config, label: 'New Name'}}] + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when layer config color changes and layer panel is closed', () => { + const layer = makeLayer(); + const [prev, next] = pairWithUiState( + 'layers', + [layer], + [{...layer, config: {...layer.config, color: [0, 255, 0]}}], + null + ); + expect(isEqual(prev, next)).toBe(true); + }); + + test('re-renders when layer.config.color changes and layer panel is open', () => { + const layer = makeLayer(); + const [prev, next] = pairWithUiState( + 'layers', + [layer], + [{...layer, config: {...layer.config, color: [0, 255, 0]}}], + 'layer' + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.visConfig changes and layer panel is open', () => { + const layer = makeLayer(); + const [prev, next] = pairWithUiState( + 'layers', + [layer], + [{...layer, config: {...layer.config, visConfig: {radius: 20}}}], + 'layer' + ); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when layer.config.isConfigActive changes and layer panel is open', () => { + const layer = makeLayer(); + const [prev, next] = pairWithUiState( + 'layers', + [layer], + [{...layer, config: {...layer.config, isConfigActive: true}}], + 'layer' + ); + expect(isEqual(prev, next)).toBe(false); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// layerOrder +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — layerOrder', () => { + test('re-renders when layerOrder changes (drag-and-drop)', () => { + const [prev, next] = pair('layerOrder', ['l1', 'l2'], ['l2', 'l1']); + expect(isEqual(prev, next)).toBe(false); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// datasets +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — datasets', () => { + test('re-renders when a dataset is added', () => { + const [prev, next] = pair('datasets', {}, {ds1: makeDataset()}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when dataset label changes', () => { + const ds = makeDataset({label: 'old'}); + const [prev, next] = pair('datasets', {ds1: ds}, {ds1: {...ds, label: 'new'}}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when dataset color changes', () => { + const ds = makeDataset({color: [255, 0, 0]}); + const [prev, next] = pair('datasets', {ds1: ds}, {ds1: {...ds, color: [0, 255, 0]}}); + expect(isEqual(prev, next)).toBe(false); + }); + + test('does NOT re-render when dataset object is recreated with same values', () => { + const fields = [{name: 'col1'}]; // same reference + const dataContainer = {}; // same reference + const ds = makeDataset({fields, dataContainer}); + const [prev, next] = pair('datasets', {ds1: ds}, {ds1: {...ds}}); + expect(isEqual(prev, next)).toBe(true); + }); +}); + +// ═══════════════════════════════════════════════════════════════════════════ +// uiState +// ═══════════════════════════════════════════════════════════════════════════ + +describe('areSidePanelPropsEqual — uiState', () => { + test('re-renders when activeSidePanel changes', () => { + const [prev, next] = pair('uiState', makeUiState(null), makeUiState('layer')); + expect(isEqual(prev, next)).toBe(false); + }); + + test('re-renders when uiState ref changes (new Redux state slice)', () => { + const [prev, next] = pair( + 'uiState', + {activeSidePanel: 'layer'}, + {activeSidePanel: 'layer'} // same shape, different object + ); + expect(isEqual(prev, next)).toBe(false); + }); +}); + diff --git a/src/components/src/side-panel.tsx b/src/components/src/side-panel.tsx index e26af380a4..7f8e16e528 100644 --- a/src/components/src/side-panel.tsx +++ b/src/components/src/side-panel.tsx @@ -30,6 +30,216 @@ import CustomPanelsFactory from './side-panel/custom-panel'; import styled from 'styled-components'; import {SidePanelProps, SidePanelItem} from './types'; +/** + * Custom `React.memo` comparator for `SidePanel`. + * + * ───────────────────────────────────────────────────────────────────────────── + * WHY THIS EXISTS + * ───────────────────────────────────────────────────────────────────────────── + * During map animation and mouse-move events the Redux store updates at + * animation-frame frequency. Without memoization every tick causes `KeplerGL` + * to re-render, which rebuilds the `sidePanelSelector` result object and passes + * it to `SidePanel`. Because the selector returns a new object each time, + * React re-renders `SidePanel` and its entire sub-tree even though nothing + * visible in the panel actually changed. + * + * This comparator allows `SidePanel` to skip re-renders caused by animation + * ticks or map-pan events while still updating promptly when the user edits a + * layer, adds a dataset, toggles a filter, etc. + * + * ───────────────────────────────────────────────────────────────────────────── + * HOW IT WORKS + * ───────────────────────────────────────────────────────────────────────────── + * The comparator iterates every key in `next`. For each key: + * • If `prev[key] === next[key]` → skip (no change, no re-render needed). + * • If the key has a custom handler below → apply partial deep comparison + * that intentionally ignores high-frequency sub-fields. + * • Otherwise → `return false` (unrecognised change, always re-render). + * + * Returning `true` = props are "equal" → React skips the re-render. + * Returning `false` = props differ → React re-renders the component. + * + * ───────────────────────────────────────────────────────────────────────────── + * PROP-BY-PROP CONTRACT (covers every key in SidePanelProps) + * ───────────────────────────────────────────────────────────────────────────── + * + * appName, appWebsite, version, onSaveMap — reference/value equality (fallthrough) + * width — value equality (fallthrough) + * layerBlending, overlayBlending — value equality (fallthrough) + * layerClasses — reference equality (fallthrough) + * interactionConfig — reference equality (fallthrough) + * mapInfo — reference equality (fallthrough) + * mapStyle — reference equality (fallthrough) + * mapSaved — value equality (fallthrough) + * availableProviders — reference equality (fallthrough) + * panels — reference equality (fallthrough) + * NOTE: pass a stable reference (defined + * outside the render function) or memo + * the array — a new `[...]` literal on + * every render defeats the optimisation. + * uiState — reference equality (fallthrough) + * Redux produces a new reference for any + * uiState change, so this is correct and + * ensures the SidePanel always reflects + * the latest UI state. + * uiStateActions, visStateActions, + * mapStateActions, mapStyleActions — reference equality (fallthrough) + * Action creators are stable across renders. + * + * mapState — PARTIALLY SUPPRESSED + * mapState.latitude/longitude/zoom/bearing/ + * pitch change on every animation frame + * during pan/zoom. SidePanel only uses + * mapState.globe.enabled (in MapManager to + * show/hide the globe-config panel), so + * only that field triggers a re-render. + * + * filters — PARTIALLY SUPPRESSED + * Always compared: id, name, type, dataId, + * view, enabled, plotType, animationWindow, + * speed, gpu. + * `value` is only compared when the filter + * panel is open — otherwise animation ticks + * that continuously update filter values + * (time-range slider) would cause the panel + * to re-render on every frame. + * + * layers — PARTIALLY SUPPRESSED + * Always compared: id, type, config.isVisible, + * config.label. + * When the layer panel is open, also compared: + * config.isConfigActive, config.color, + * config.highlightColor, config.visConfig, + * config.dataId, config.columns. + * Other config fields (e.g. per-layer + * animation state) are suppressed when the + * panel is closed. + * + * layerOrder — ALWAYS RE-RENDERS + * Changes when the user drags layers to + * reorder them. Always triggers re-render. + * + * datasets — PARTIALLY SUPPRESSED + * Compared per dataset: id, label, color, + * fields (ref), dataContainer (ref). + * Internal dataset mutations that don't + * change these refs are suppressed. + * + * ───────────────────────────────────────────────────────────────────────────── + * MAINTENANCE RULES — READ BEFORE EDITING + * ───────────────────────────────────────────────────────────────────────────── + * 1. Adding a prop to SidePanelProps: + * • If it is a stable scalar or reference (e.g. a Redux slice or action + * creator): the fallthrough `return false` handles it correctly. + * No change needed here, but add a row to the prop table above. + * • If it is an object recreated on every render that contains + * high-frequency fields (like mapState): add an explicit handler + * that suppresses the irrelevant sub-fields. + * + * 2. Adding a field to `filters`, `layers`, or `datasets`: + * • Decide whether SidePanel needs to see the field while the relevant + * panel is closed. If yes, add it to the "always compared" section. + * If no (e.g. an animation-tick value), add it to the panel-open guard. + * + * 3. Never remove a check without verifying the corresponding panel still + * updates correctly when that field changes in a real browser session. + * + * 4. All comparator behaviour is covered by areSidePanelPropsEqual tests in + * side-panel.spec.tsx. Add a test for every new case. + */ +export const areSidePanelPropsEqual = (prev: SidePanelProps, next: SidePanelProps): boolean => { + const keys = Object.keys(next) as (keyof SidePanelProps)[]; + for (const key of keys) { + if (prev[key] === next[key]) continue; + + if (key === 'filters') { + const pf = prev.filters; + const nf = next.filters; + if (pf?.length !== nf?.length) return false; + const isFilterPanelOpen = (next as any).uiState?.activeSidePanel === 'filter'; + for (let i = 0; i < nf.length; i++) { + if (pf[i] === nf[i]) continue; + if (pf[i].id !== nf[i].id) return false; + if (pf[i].name !== nf[i].name) return false; + if (pf[i].type !== nf[i].type) return false; + if (pf[i].dataId !== nf[i].dataId) return false; + if (pf[i].view !== nf[i].view) return false; + if (pf[i].enabled !== nf[i].enabled) return false; + if (pf[i].plotType !== nf[i].plotType) return false; + if ((pf[i] as any).animationWindow !== (nf[i] as any).animationWindow) return false; + if (pf[i].speed !== nf[i].speed) return false; + if (pf[i].gpu !== nf[i].gpu) return false; + // Always check value when the filter panel is open so the panel stays up-to-date. + // Outside the panel, suppress value changes to avoid re-renders during animation. + if (isFilterPanelOpen) { + if (pf[i].value !== nf[i].value) return false; + } + } + continue; + } + + if (key === 'datasets') { + const pd = prev.datasets; + const nd = next.datasets; + const pKeys = Object.keys(pd || {}); + const nKeys = Object.keys(nd || {}); + if (pKeys.length !== nKeys.length) return false; + for (const dk of nKeys) { + if (!pd?.[dk]) return false; + if (pd[dk] === nd[dk]) continue; + if (pd[dk].id !== nd[dk].id) return false; + if (pd[dk].label !== nd[dk].label) return false; + if (pd[dk].color !== nd[dk].color) return false; + if (pd[dk].fields !== nd[dk].fields) return false; + if (pd[dk].dataContainer !== nd[dk].dataContainer) return false; + } + continue; + } + + if (key === 'layers') { + const pl = prev.layers; + const nl = next.layers; + if (pl?.length !== nl?.length) return false; + const isLayerPanelOpen = (next as any).uiState?.activeSidePanel === 'layer'; + for (let i = 0; i < nl.length; i++) { + if (pl[i] === nl[i]) continue; + if (pl[i].id !== nl[i].id) return false; + if (pl[i].type !== nl[i].type) return false; + if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; + if (pl[i].config.label !== nl[i].config.label) return false; + // When the layer panel is open, also check config fields visible in the panel UI + if (isLayerPanelOpen) { + if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; + if (pl[i].config.color !== nl[i].config.color) return false; + if (pl[i].config.highlightColor !== nl[i].config.highlightColor) return false; + if (pl[i].config.visConfig !== nl[i].config.visConfig) return false; + if (pl[i].config.dataId !== nl[i].config.dataId) return false; + if (pl[i].config.columns !== nl[i].config.columns) return false; + } + } + continue; + } + + if (key === 'layerOrder') { + // layerOrder changes during drag-and-drop reordering — always re-render + return false; + } + + if (key === 'mapState') { + // mapState changes on every pan/zoom frame (latitude, longitude, zoom, bearing, pitch). + // SidePanel only cares about globe.enabled (used in MapManager to conditionally + // show the globe settings panel). Suppress all other mapState changes. + const pm = prev.mapState; + const nm = next.mapState; + if (pm?.globe?.enabled !== nm?.globe?.enabled) return false; + continue; + } + + return false; + } + return true; +}; + export const StyledSidePanelContent = styled.div` ${props => props.theme.sidePanelScrollBar}; flex-grow: 1; @@ -251,99 +461,6 @@ export default function SidePanelFactory( SidePanel.defaultPanels = fullPanels; - const areSidePanelPropsEqual = (prev: SidePanelProps, next: SidePanelProps): boolean => { - const keys = Object.keys(next) as (keyof SidePanelProps)[]; - for (const key of keys) { - if (prev[key] === next[key]) continue; - - if (key === 'filters') { - const pf = prev.filters; - const nf = next.filters; - if (pf?.length !== nf?.length) return false; - const isFilterPanelOpen = (next as any).uiState?.activeSidePanel === 'filter'; - for (let i = 0; i < nf.length; i++) { - if (pf[i] === nf[i]) continue; - if (pf[i].id !== nf[i].id) return false; - if (pf[i].name !== nf[i].name) return false; - if (pf[i].type !== nf[i].type) return false; - if (pf[i].dataId !== nf[i].dataId) return false; - if (pf[i].view !== nf[i].view) return false; - if (pf[i].enabled !== nf[i].enabled) return false; - if (pf[i].plotType !== nf[i].plotType) return false; - if ((pf[i] as any).animationWindow !== (nf[i] as any).animationWindow) return false; - if (pf[i].speed !== nf[i].speed) return false; - if (pf[i].gpu !== nf[i].gpu) return false; - // Always check value when the filter panel is open so the panel stays up-to-date. - // Outside the panel, suppress value changes to avoid re-renders during animation. - if (isFilterPanelOpen) { - if (pf[i].value !== nf[i].value) return false; - } - } - continue; - } - - if (key === 'datasets') { - const pd = prev.datasets; - const nd = next.datasets; - const pKeys = Object.keys(pd || {}); - const nKeys = Object.keys(nd || {}); - if (pKeys.length !== nKeys.length) return false; - for (const dk of nKeys) { - if (!pd?.[dk]) return false; - if (pd[dk] === nd[dk]) continue; - if (pd[dk].id !== nd[dk].id) return false; - if (pd[dk].label !== nd[dk].label) return false; - if (pd[dk].color !== nd[dk].color) return false; - if (pd[dk].fields !== nd[dk].fields) return false; - if (pd[dk].dataContainer !== nd[dk].dataContainer) return false; - } - continue; - } - - if (key === 'layers') { - const pl = prev.layers; - const nl = next.layers; - if (pl?.length !== nl?.length) return false; - const isLayerPanelOpen = (next as any).uiState?.activeSidePanel === 'layer'; - for (let i = 0; i < nl.length; i++) { - if (pl[i] === nl[i]) continue; - if (pl[i].id !== nl[i].id) return false; - if (pl[i].type !== nl[i].type) return false; - if (pl[i].config.isVisible !== nl[i].config.isVisible) return false; - if (pl[i].config.label !== nl[i].config.label) return false; - // When the layer panel is open, also check config fields visible in the panel UI - if (isLayerPanelOpen) { - if (pl[i].config.isConfigActive !== nl[i].config.isConfigActive) return false; - if (pl[i].config.color !== nl[i].config.color) return false; - if (pl[i].config.highlightColor !== nl[i].config.highlightColor) return false; - if (pl[i].config.visConfig !== nl[i].config.visConfig) return false; - if (pl[i].config.dataId !== nl[i].config.dataId) return false; - if (pl[i].config.columns !== nl[i].config.columns) return false; - } - } - continue; - } - - if (key === 'layerOrder') { - // layerOrder changes during drag-and-drop reordering — always re-render - return false; - } - - if (key === 'mapState') { - // mapState changes on every pan/zoom frame (latitude, longitude, zoom, bearing, pitch). - // SidePanel only cares about globe.enabled (used in MapManager to conditionally - // show the globe settings panel). Suppress all other mapState changes. - const pm = prev.mapState; - const nm = next.mapState; - if (pm?.globe?.enabled !== nm?.globe?.enabled) return false; - continue; - } - - return false; - } - return true; - }; - const MemoizedSidePanel = memo(SidePanel, areSidePanelPropsEqual) as React.NamedExoticComponent & { defaultPanels: SidePanelProps['panels']; }; From 80a218343eb5521e44eba3fc3e598eacf9d4eceb Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Mon, 3 Aug 2026 00:16:55 +0300 Subject: [PATCH 6/8] fix tests Signed-off-by: Ihor Dykhta --- test/browser/components/kepler-gl-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/browser/components/kepler-gl-test.js b/test/browser/components/kepler-gl-test.js index 26a6489ade..2f31f8fd61 100644 --- a/test/browser/components/kepler-gl-test.js +++ b/test/browser/components/kepler-gl-test.js @@ -73,7 +73,7 @@ test('Components -> KeplerGl -> Mount', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find(SidePanel).length, 1, 'should render SidePanel'); + t.equal(wrapper.find('SidePanel').length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 1, 'should render MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -116,7 +116,7 @@ test('Components -> KeplerGl -> Mount -> readOnly', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find(SidePanel).length, 0, 'should not render SidePanel'); + t.equal(wrapper.find('SidePanel').length, 0, 'should not render SidePanel'); t.equal(wrapper.find(MapContainer).length, 1, 'should render MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -162,7 +162,7 @@ test('Components -> KeplerGl -> Mount -> Plot', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find(SidePanel).length, 1, 'should render SidePanel'); + t.equal(wrapper.find('SidePanel').length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 2, 'should render 2 MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -205,7 +205,7 @@ test('Components -> KeplerGl -> Mount -> Split Maps', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find(SidePanel).length, 1, 'should render SidePanel'); + t.equal(wrapper.find('SidePanel').length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 2, 'should render 2 MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); From 085ee539ab9403f02c96cbb87b191b433b7eeb6b Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Mon, 3 Aug 2026 00:32:34 +0300 Subject: [PATCH 7/8] fix tests Signed-off-by: Ihor Dykhta --- test/browser/components/kepler-gl-test.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/browser/components/kepler-gl-test.js b/test/browser/components/kepler-gl-test.js index 2f31f8fd61..f8f7a750be 100644 --- a/test/browser/components/kepler-gl-test.js +++ b/test/browser/components/kepler-gl-test.js @@ -73,7 +73,7 @@ test('Components -> KeplerGl -> Mount', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('SidePanel').length, 1, 'should render SidePanel'); + t.equal(wrapper.find('.side-panel__content').length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 1, 'should render MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -116,7 +116,7 @@ test('Components -> KeplerGl -> Mount -> readOnly', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('SidePanel').length, 0, 'should not render SidePanel'); + t.equal(wrapper.find('.side-panel__content').length, 0, 'should not render SidePanel'); t.equal(wrapper.find(MapContainer).length, 1, 'should render MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -162,7 +162,7 @@ test('Components -> KeplerGl -> Mount -> Plot', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('SidePanel').length, 1, 'should render SidePanel'); + t.equal(wrapper.find('.side-panel__content').length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 2, 'should render 2 MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -205,7 +205,7 @@ test('Components -> KeplerGl -> Mount -> Split Maps', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('SidePanel').length, 1, 'should render SidePanel'); + t.equal(wrapper.find('.side-panel__content').length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 2, 'should render 2 MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); From 7fd7f49f3a83779a5d40425f80a054e9dba6ab96 Mon Sep 17 00:00:00 2001 From: Ihor Dykhta Date: Mon, 3 Aug 2026 00:56:38 +0300 Subject: [PATCH 8/8] tests Signed-off-by: Ihor Dykhta --- test/browser/components/kepler-gl-test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/browser/components/kepler-gl-test.js b/test/browser/components/kepler-gl-test.js index f8f7a750be..9a9ab8e1bf 100644 --- a/test/browser/components/kepler-gl-test.js +++ b/test/browser/components/kepler-gl-test.js @@ -15,6 +15,7 @@ import { appInjector, KeplerGlFactory, SidePanelFactory, + PanelHeaderFactory, MapContainerFactory, BottomWidgetFactory, ModalContainerFactory, @@ -33,6 +34,7 @@ import {StateWithGeocoderDataset} from 'test/helpers/mock-state'; const KeplerGl = appInjector.get(KeplerGlFactory); const SidePanel = appInjector.get(SidePanelFactory); +const PanelHeader = appInjector.get(PanelHeaderFactory); const MapContainer = appInjector.get(MapContainerFactory); const BottomWidget = appInjector.get(BottomWidgetFactory); const ModalContainer = appInjector.get(ModalContainerFactory); @@ -73,7 +75,7 @@ test('Components -> KeplerGl -> Mount', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('.side-panel__content').length, 1, 'should render SidePanel'); + t.equal(wrapper.find(PanelHeader).length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 1, 'should render MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -116,7 +118,7 @@ test('Components -> KeplerGl -> Mount -> readOnly', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('.side-panel__content').length, 0, 'should not render SidePanel'); + t.equal(wrapper.find(PanelHeader).length, 0, 'should not render SidePanel'); t.equal(wrapper.find(MapContainer).length, 1, 'should render MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -162,7 +164,7 @@ test('Components -> KeplerGl -> Mount -> Plot', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('.side-panel__content').length, 1, 'should render SidePanel'); + t.equal(wrapper.find(PanelHeader).length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 2, 'should render 2 MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer'); @@ -205,7 +207,7 @@ test('Components -> KeplerGl -> Mount -> Split Maps', t => { }, 'Should not throw error when mount KeplerGl'); t.equal(wrapper.find(KeplerGl).length, 1, 'should render KeplerGl'); - t.equal(wrapper.find('.side-panel__content').length, 1, 'should render SidePanel'); + t.equal(wrapper.find(PanelHeader).length, 1, 'should render SidePanel'); t.equal(wrapper.find(MapContainer).length, 2, 'should render 2 MapContainer'); t.equal(wrapper.find(BottomWidget).length, 1, 'should render BottomWidget'); t.equal(wrapper.find(ModalContainer).length, 1, 'should render ModalContainer');