[fix] Fixed bug when switching to dataset layer view (#2209)

Signed-off-by: Ihor Dykhta <dikhta.igor@gmail.com> Co-authored-by: Giuseppe Macrì <macri.giuseppe@gmail.com>
keplergl · May 5, 2023 · 7d3c602 · 7d3c602
1 parent 2275b8e
commit 7d3c602
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 24 deletions.
diff --git a/src/components/src/side-panel/layer-panel/layer-list.tsx b/src/components/src/side-panel/layer-panel/layer-list.tsx
@@ -75,12 +75,14 @@ const SortableStyledItem = styled.div<SortableStyledItemProps>`
   }
 `;
 
+const INITIAL_LAYERS_TO_SHOW: Layer[] = [];
+
 LayerListFactory.deps = [LayerPanelFactory];
 
 function LayerListFactory(LayerPanel: ReturnType<typeof LayerPanelFactory>) {
   // By wrapping layer panel using a sortable element we don't have to implement the drag and drop logic into the panel itself;
   // Developers can provide any layer panel implementation and it will still be sortable
-  const SortableItem = ({layer, layerIndex, panelProps, layerActions}) => {
+  const SortableItem = ({layer, idx, panelProps, layerActions}) => {
     const {attributes, listeners, setNodeRef, isDragging, transform, transition} = useSortable({
       id: layer.id
     });
@@ -97,7 +99,7 @@ function LayerListFactory(LayerPanel: ReturnType<typeof LayerPanelFactory>) {
           {...panelProps}
           {...layerActions}
           key={layer.id}
-          idx={layerIndex}
+          idx={idx}
           layer={layer}
           listeners={listeners}
         />
@@ -132,12 +134,12 @@ function LayerListFactory(LayerPanel: ReturnType<typeof LayerPanelFactory>) {
 
     const layersToShow = useMemo(() => {
       return layerOrder.reduce((acc, layerId) => {
-        const layer = findById(layerId)(layers);
+        const layer = findById(layerId)(layers.filter(Boolean));
         if (!layer) {
           return acc;
         }
         return !layer.config.hidden ? [...acc, layer] : acc;
-      }, [] as Layer[]);
+      }, INITIAL_LAYERS_TO_SHOW);
     }, [layers, layerOrder]);
 
     const sidePanelDndItems = useMemo(() => {
@@ -158,34 +160,39 @@ function LayerListFactory(LayerPanel: ReturnType<typeof LayerPanelFactory>) {
       [layerClasses]
     );
 
-    const layerActions = {
-      layerColorUIChange: visStateActions.layerColorUIChange,
-      layerConfigChange: visStateActions.layerConfigChange,
-      layerVisualChannelConfigChange: visStateActions.layerVisualChannelConfigChange,
-      layerTypeChange: visStateActions.layerTypeChange,
-      layerVisConfigChange: visStateActions.layerVisConfigChange,
-      layerTextLabelChange: visStateActions.layerTextLabelChange,
-      removeLayer: visStateActions.removeLayer,
-      duplicateLayer: visStateActions.duplicateLayer,
-      layerSetIsValid: visStateActions.layerSetIsValid
-    };
-
-    const panelProps = {
-      datasets,
-      openModal,
-      layerTypeOptions
-    };
+    const layerActions = useMemo(
+      () => ({
+        layerColorUIChange: visStateActions.layerColorUIChange,
+        layerConfigChange: visStateActions.layerConfigChange,
+        layerVisualChannelConfigChange: visStateActions.layerVisualChannelConfigChange,
+        layerTypeChange: visStateActions.layerTypeChange,
+        layerVisConfigChange: visStateActions.layerVisConfigChange,
+        layerTextLabelChange: visStateActions.layerTextLabelChange,
+        removeLayer: visStateActions.removeLayer,
+        duplicateLayer: visStateActions.duplicateLayer,
+        layerSetIsValid: visStateActions.layerSetIsValid
+      }),
+      [visStateActions]
+    );
+
+    const panelProps = useMemo(
+      () => ({
+        datasets,
+        openModal,
+        layerTypeOptions
+      }),
+      [datasets, openModal, layerTypeOptions]
+    );
 
     return isSortable ? (
       <>
         <SortableList containerId="sortablelist" sidePanelDndItems={sidePanelDndItems}>
           {/* warning: containerId should be similar to the first key in dndItems defined in kepler-gl.js*/}
-
           {layersToShow.map(layer => (
             <SortableItem
               key={layer.id}
               layer={layer}
-              layerIndex={layers.findIndex(({id}) => id === layer.id)}
+              idx={layers.findIndex(l => l?.id === layer.id)}
               panelProps={panelProps}
               layerActions={layerActions}
             />
@@ -199,6 +206,7 @@ function LayerListFactory(LayerPanel: ReturnType<typeof LayerPanelFactory>) {
             {...panelProps}
             {...layerActions}
             key={layer.id}
+            idx={layers.findIndex(l => l?.id === layer.id)}
             layer={layer}
             isDraggable={false}
           />

diff --git a/src/components/src/side-panel/layer-panel/layer-panel.tsx b/src/components/src/side-panel/layer-panel/layer-panel.tsx
@@ -48,6 +48,7 @@ type LayerPanelProps = {
     requireData: any; //
   }[];
   isDraggable?: boolean;
+  idx: number;
   openModal: ActionHandler<typeof toggleModal>;
   layerColorUIChange: ActionHandler<typeof VisStateActions.layerColorUIChange>;
   layerConfigChange: ActionHandler<typeof VisStateActions.layerConfigChange>;

diff --git a/src/components/src/side-panel/panel-header-action.tsx b/src/components/src/side-panel/panel-header-action.tsx
@@ -94,7 +94,13 @@ export default function PanelHeaderActionFactory(): React.FC<PanelHeaderActionPr
         flush={flush}
       >
         {IconComponent ? (
-          <IconComponent data-tip data-for={`${tooltip}_${id}`} height="16px" onClick={onClick} />
+          <IconComponent
+            className="panel--header__action__component"
+            data-tip
+            data-for={`${tooltip}_${id}`}
+            height="16px"
+            onClick={onClick}
+          />
         ) : null}
         {tooltip ? (
           <Tooltip id={`${tooltip}_${id}`} effect="solid" delayShow={500} type={tooltipType}>

diff --git a/test/browser/components/side-panel/layer-list-test.js b/test/browser/components/side-panel/layer-list-test.js
@@ -20,6 +20,7 @@
 
 import React from 'react';
 import test from 'tape';
+import sinon from 'sinon';
 
 import {StateWMultiH3Layers} from 'test/helpers/mock-state';
 
@@ -98,3 +99,35 @@ test('Components -> SidePanel -> LayerPanel -> LayerList -> render non-sortable
 
   t.end();
 });
+
+test('Components -> SidePanel -> LayerPanel -> LayerList -> pass null entries as layers', t => {
+  let wrapper;
+  const layers = [...defaultProps.layers];
+  layers[0] = null;
+  const removeLayerSpy = sinon.spy();
+  const visStateActions = {...defaultProps.visStateActions};
+  visStateActions.removeLayer = removeLayerSpy;
+  t.doesNotThrow(() => {
+    wrapper = mountWithTheme(
+      <IntlWrapper>
+        <LayerList
+          {...defaultProps}
+          isSortable={false}
+          layers={layers}
+          visStateActions={visStateActions}
+        />
+      </IntlWrapper>
+    );
+  }, 'LayerList should render');
+
+  t.equal(wrapper.find('LayerPanel').length, 1, 'should render 1 LayerPanel');
+
+  const removeLayer = wrapper.find(
+    'div.panel--header__action.layer__remove-layer svg.data-ex-icons-trash'
+  );
+
+  removeLayer.simulate('click');
+  t.equal(removeLayerSpy.called, true, 'Should have called remove layer when clicked');
+
+  t.end();
+});