[Bug] preserveLayerOrder when replace data (#2214)

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com>
Co-authored-by: Shan He <heshan0131@gmail.com>

src/components/src/geocoder-panel.tsx

@@ -23,7 +23,6 @@ import styled from 'styled-components';
 import classnames from 'classnames';
 import {processRowObject} from '@kepler.gl/processors';
 import {FlyToInterpolator} from '@deck.gl/core';
-import KeplerGlSchema from '@kepler.gl/schemas';
 import {getCenterAndZoomFromBounds} from '@kepler.gl/utils';
 
 import Geocoder, {Result} from './geocoder/geocoder';
@@ -57,14 +56,14 @@ const ICON_LAYER = {
   }
 };
 
-const PARSED_CONFIG = KeplerGlSchema.parseSavedConfig({
-  version: 'v1',
-  config: {
+function generateConfig(layerOrder) {
+  return {
     visState: {
-      layers: [ICON_LAYER]
+      layers: [ICON_LAYER],
+      layerOrder: [ICON_LAYER.id, ...layerOrder]
     }
-  }
-});
+  };
+}
 
 interface StyledGeocoderPanelProps {
   width?: number;
@@ -116,8 +115,7 @@ export function getUpdateVisDataPayload(lat, lon, text) {
     [generateGeocoderDataset(lat, lon, text)],
     {
       keepExistingConfig: true
-    },
-    PARSED_CONFIG
+    }
   ];
 }
 
@@ -129,6 +127,7 @@ interface GeocoderPanelProps {
   updateVisData: Function;
   removeDataset: Function;
   updateMap: Function;
+  layerOrder: string[];
 
   transitionDuration?: number;
   width?: number;
@@ -154,8 +153,13 @@ export default function GeocoderPanelFactory(): ComponentType<GeocoderPanelProps
         text,
         bbox
       } = geoItem;
+      const {layerOrder} = this.props;
+
       this.removeGeocoderDataset();
-      this.props.updateVisData(...getUpdateVisDataPayload(lat, lon, text));
+      this.props.updateVisData(
+        ...getUpdateVisDataPayload(lat, lon, text),
+        generateConfig(layerOrder)
+      );
       const bounds = bbox || [
         lon - GEOCODER_GEO_OFFSET,
         lat - GEOCODER_GEO_OFFSET,

src/components/src/kepler-gl.tsx

@@ -285,6 +285,7 @@ export const geoCoderPanelSelector = (
   mapboxApiAccessToken: props.mapboxApiAccessToken,
   mapState: props.mapState,
   uiState: props.uiState,
+  layerOrder: props.visState.layerOrder,
   updateVisData: props.visStateActions.updateVisData,
   removeDataset: props.visStateActions.removeDataset,
   updateMap: props.mapStateActions.updateMap,

src/reducers/src/vis-state-updaters.ts

@@ -2724,6 +2724,8 @@ export function prepareStateForDatasetReplace<T extends VisState>(
 ): T {
   const serializedState = serializeVisState(state, state.schema);
   const nextState = replaceDatasetAndDeps(state, dataId, dataIdToUse);
+  // make a copy of layerOrder, because layer id will be removed from it by calling removeLayerUpdater
+  const preserveLayerOrder = [...state.layerOrder];
 
   // preserve dataset order
   nextState.preserveDatasetOrder = Object.keys(state.datasets).map(d =>
@@ -2734,6 +2736,7 @@ export function prepareStateForDatasetReplace<T extends VisState>(
   if (nextState.layerToBeMerged?.length) {
     // copy split maps to be merged, because it will be reset in remove layer
     nextState.splitMapsToBeMerged = serializedState?.splitMaps ?? [];
+    nextState.layerOrder = [...preserveLayerOrder];
   }
 
   return nextState;

test/browser/components/geocoder-panel-test.js

@@ -43,7 +43,7 @@ test('GeocoderPanel - render', t => {
     width: 800,
     height: 800
   };
-
+  const layerOrder = ['layer_1', 'layer_2'];
   const mockUiState = InitialState.uiState;
 
   const mockGeoItem = {
@@ -139,7 +139,8 @@ test('GeocoderPanel - render', t => {
               }
             }
           }
-        ]
+        ],
+        layerOrder: ['geocoder_layer', 'layer_1', 'layer_2']
       }
     }
   ];
@@ -157,6 +158,7 @@ test('GeocoderPanel - render', t => {
           updateVisData={updateVisData}
           removeDataset={removeDataset}
           updateMap={updateMap}
+          layerOrder={layerOrder}
         />
       </IntlWrapper>
     );

test/node/reducers/composer-state-test.js

@@ -492,3 +492,50 @@ test('#composerStateReducer - replaceDataInMapUpdater', t => {
   t.deepEqual(nextState.visState.splitMapsToBeMerged, [], 'should reset splitMapsToBeMerged');
   t.end();
 });
+
+test('#composerStateReducer - replaceDataInMapUpdater: same dataId', t => {
+  const datasets = {
+    data: processCsvData(testCsvData),
+    info: {
+      id: sampleConfig.dataId
+    }
+  };
+  const datasetToUse = {
+    data: processCsvData(dataWithNulls),
+    info: {
+      id: sampleConfig.dataId
+    }
+  };
+  const state = keplerGlReducer({}, registerEntry({id: 'test'})).test;
+
+  // old state contain splitMaps
+  const oldState = addDataToMapUpdater(state, {
+    payload: {
+      datasets,
+      config: sampleConfig.config
+    }
+  });
+
+  const oldSavedConfig = state.visState.schema.getConfigToSave(oldState).config;
+
+  const nextState = replaceDataInMapUpdater(oldState, {
+    payload: {
+      datasetToReplaceId: sampleConfig.dataId,
+      datasetToUse
+    }
+  });
+
+  // dataset should be replaced
+  t.ok(nextState.visState.datasets[sampleConfig.dataId], ' dataset should be replaced');
+  const nextSavedConfig = nextState.visState.schema.getConfigToSave(nextState).config;
+
+  const expectedLayers = oldSavedConfig.visState.layers.map(l => ({
+    ...l,
+    config: {
+      ...l.config,
+      dataId: sampleConfig.dataId
+    }
+  }));
+  t.deepEqual(nextSavedConfig.visState.layers, expectedLayers, 'should replace layer dataId');
+  t.end();
+});

test/node/reducers/vis-state-test.js

@@ -5244,7 +5244,11 @@ test('VisStateUpdater -> prepareStateForDatasetReplace', t => {
   t.deepEqual(nextState.filterToBeMerged[0].dataId, [dataIdToUse], 'should replace filter dataId');
 
   // preserveLayerOrder
-  t.deepEqual(nextState.layerOrder, ['geojson-1'], 'should remove layer from layer order');
+  t.deepEqual(
+    nextState.layerOrder,
+    ['geojson-1', 'point-0'],
+    'should not remove layer from layer order'
+  );
   t.deepEqual(
     nextState.preserveLayerOrder,
     ['geojson-1', 'point-0'],