Skip to content

fix: prevent unnecessary UI re-renders#3439

Draft
igorDykhta wants to merge 3 commits into
masterfrom
igr/fix-improve-ui-redraw
Draft

fix: prevent unnecessary UI re-renders#3439
igorDykhta wants to merge 3 commits into
masterfrom
igr/fix-improve-ui-redraw

Conversation

@igorDykhta
Copy link
Copy Markdown
Collaborator

Screenshot 2026-05-16 at 4 43 26 AM

Ihor Dykhta added 3 commits May 16, 2026 02:36
Signed-off-by: Ihor Dykhta <ihordykhta@Ihors-MacBook-Pro.local>
Signed-off-by: Ihor Dykhta <ihordykhta@Ihors-MacBook-Pro.local>
Signed-off-by: Ihor Dykhta <ihordykhta@Ihors-MacBook-Pro.local>
@igorDykhta igorDykhta added investigation needed 3.3 Kepler.gl 3.3 release labels May 16, 2026
@igorDykhta igorDykhta self-assigned this May 16, 2026
@igorDykhta igorDykhta requested review from ilyabo and lixun910 May 18, 2026 15:27
@igorDykhta igorDykhta marked this pull request as ready for review May 18, 2026 15:29
Copilot AI review requested due to automatic review settings May 18, 2026 15:29
@igorDykhta igorDykhta marked this pull request as draft May 18, 2026 15:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR targets issue #2626 by reducing unnecessary React re-renders in kepler.gl’s UI during high-frequency updates (e.g., animation ticks and mouse move) via memoization and more stable prop selection.

Changes:

  • Replaced several plain “selector functions” in kepler-gl.tsx with reselect memoized selectors to stabilize derived prop objects.
  • Added React.memo (with custom equality functions in some cases) to key UI components (SidePanel, MapControl, BottomWidget, etc.) to avoid re-render cascades.
  • Customized react-redux connect behavior in container.tsx using areStatesEqual to prevent updates when the current kepler.gl instance slice is unchanged.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/components/src/side-panel.tsx Wraps SidePanel in React.memo with a custom prop comparator to reduce re-renders during animation/state churn.
src/components/src/map/map-control.tsx Wraps MapControl in React.memo with a custom comparator for layers/datasets-related props.
src/components/src/map-container.tsx Memoizes Attribution / AttributionLogos components to reduce repeated renders.
src/components/src/loading-indicator.tsx Memoizes LoadingIndicator before applying withTheme.
src/components/src/kepler-gl.tsx Uses reselect to memoize side panel, bottom widget, and notification selectors.
src/components/src/container.tsx Adds connect option areStatesEqual to avoid container updates when instance state ref is unchanged.
src/components/src/bottom-widget.tsx Memoizes BottomWidget and wraps it with forwardRef + withTheme.
Comments suppressed due to low confidence (3)

src/components/src/map-container.tsx:252

  • Attribution is now wrapped in React.memo, but it derives isPalm from hasMobileWidth(breakPointValues) (a matchMedia query) rather than from props/state. With memoization, the component won’t re-render when the media query result changes (e.g. window resize), so the attribution layout can get stuck in the wrong mobile/desktop mode. Consider removing React.memo here, or subscribing to the media query (e.g. via a hook/useSyncExternalStore) and/or passing a size breakpoint value as a prop so resizes trigger a re-render.
export const Attribution: React.FC<AttributionProps> = React.memo(({
  showBaseMapLibLogo = true,
  showOsmBasemapAttribution = false,
  datasetAttributions,
  baseMapLibraryConfig
}: AttributionProps) => {
  const isPalm = hasMobileWidth(breakPointValues);

src/components/src/side-panel.tsx:312

  • In the layers branch of areSidePanelPropsEqual, when the layer panel is open and both layers and filters change in the same update, the early-return is skipped and the comparison falls back to only checking layer.id/layer.type. That can treat materially changed layer configs as “equal” and prevent the Layer panel UI from updating. If the intent is to suppress animation-driven churn, consider gating on a more explicit signal (e.g. animation action/flag) or expanding the comparison for the layer panel open case to include the fields that drive the layer list UI.
      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].id !== nl[i].id) return false;
          if (pl[i].type !== nl[i].type) return false;
        }
        continue;

src/components/src/map/map-control.tsx:205

  • After wrapping MapControl with memo, React DevTools will typically show it as Memo(MapControl). If you want to preserve the original component name for debugging (similar to what was done for SidePanel), set MemoizedMapControl.displayName = 'MapControl'.
  const MemoizedMapControl = memo(MapControl, areMapControlPropsEqual) as React.NamedExoticComponent<MapControlProps> & {
    defaultActionComponents: MapControlProps['actionComponents'];
  };
  (MemoizedMapControl as any).defaultActionComponents = DEFAULT_ACTIONS;

  return MemoizedMapControl;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +253 to +254
const keys = Object.keys(next) as (keyof SidePanelProps)[];
for (const key of keys) {

return MapControl;
const areMapControlPropsEqual = (prev: MapControlProps, next: MapControlProps): boolean => {
const keys = Object.keys(next) as (keyof MapControlProps)[];
return MapControl;
const areMapControlPropsEqual = (prev: MapControlProps, next: MapControlProps): boolean => {
const keys = Object.keys(next) as (keyof MapControlProps)[];
for (const key of keys) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wouldn't some deepEqual simplify this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3.3 Kepler.gl 3.3 release investigation needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants