Refactor visibility property

Signed-off-by: Vijayan Balasubramanian <balasvij@amazon.com>
opensearch-project · Jan 31, 2023 · 2175dd5 · 2175dd5
1 parent ff5d859
commit 2175dd5
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 18 deletions.
diff --git a/public/components/layer_control_panel/layer_control_panel.tsx b/public/components/layer_control_panel/layer_control_panel.tsx
@@ -48,7 +48,7 @@ import {
 } from '../../model/layerRenderController';
 import { MapState } from '../../model/mapState';
 import { ConfigSchema } from '../../../common/config';
-import {moveLayers} from "../../model/map/layer_operations";
+import {moveLayers, updateVisibility} from "../../model/map/layer_operations";
 
 interface MaplibreRef {
   current: Maplibre | null;
@@ -290,7 +290,7 @@ export const LayerControlPanel = memo(
         layer.visibility = LAYER_VISIBILITY.VISIBLE;
         setLayerVisibility(new Map(layerVisibility.set(layer.id, true)));
       }
-      layersFunctionMap[layer.type]?.hide(maplibreRef, layer);
+      updateVisibility(maplibreRef.current!, layer.id, layer.visibility);
     };
 
     const onDeleteLayerIconClick = (layer: MapLayerSpecification) => {

diff --git a/public/model/documentLayerFunctions.ts b/public/model/documentLayerFunctions.ts
@@ -7,14 +7,17 @@ import { Map as Maplibre } from 'maplibre-gl';
 import { parse } from 'wellknown';
 import { DocumentLayerSpecification } from './mapLayerType';
 import { convertGeoPointToGeoJSON, isGeoJSON } from '../utils/geo_formater';
-import { getMaplibreBeforeLayerId} from './layersFunctions';
+import { getMaplibreBeforeLayerId } from './layersFunctions';
 import {
   addCircleLayer,
   addLineLayer,
-  addPolygonLayer, hasLayer, removeLayers,
+  addPolygonLayer,
+  hasLayer,
+  removeLayers,
   updateCircleLayer,
   updateLineLayer,
   updatePolygonLayer,
+  updateVisibility,
 } from './map/layer_operations';
 
 interface MaplibreRef {
@@ -175,7 +178,7 @@ const addNewLayer = (
   }
 };
 
-const updateLayerConfig = (
+const updateLayer = (
   layerConfig: DocumentLayerSpecification,
   maplibreRef: MaplibreRef,
   data: any
@@ -230,21 +233,14 @@ export const DocumentLayerFunctions = {
     data: any,
     beforeLayerId: string | undefined
   ) => {
-    if (hasLayer(maplibreRef.current!, layerConfig.id)) {
-      updateLayerConfig(layerConfig, maplibreRef, data);
-    } else {
-      addNewLayer(layerConfig, maplibreRef, data, beforeLayerId);
-    }
+    return hasLayer(maplibreRef.current!, layerConfig.id)
+      ? updateLayer(layerConfig, maplibreRef, data)
+      : addNewLayer(layerConfig, maplibreRef, data, beforeLayerId);
   },
   remove: (maplibreRef: MaplibreRef, layerConfig: DocumentLayerSpecification) => {
     removeLayers(maplibreRef.current!, layerConfig.id, true);
   },
   hide: (maplibreRef: MaplibreRef, layerConfig: DocumentLayerSpecification) => {
-    const layers = getCurrentStyleLayers(maplibreRef);
-    layers.forEach((layer) => {
-      if (layer.id.includes(layerConfig.id)) {
-        maplibreRef.current?.setLayoutProperty(layer.id, 'visibility', layerConfig.visibility);
-      }
-    });
+    updateVisibility(maplibreRef.current!, layerConfig.id, layerConfig.visibility);
   },
 };
diff --git a/public/model/map/layer_operations.test.ts b/public/model/map/layer_operations.test.ts
@@ -10,7 +10,7 @@ import {
   hasLayer, moveLayers, removeLayers,
   updateCircleLayer,
   updateLineLayer,
-  updatePolygonLayer,
+  updatePolygonLayer, updateVisibility,
 } from './layer_operations';
 import { Map as Maplibre } from 'maplibre-gl';
 import { MockMaplibreMap } from './__mocks__/map';
@@ -359,3 +359,16 @@ describe('delete layer', function () {
     ).toEqual([]);
   });
 });
+
+describe('update visibility', function () {
+  it('should update visibility for given layer', function () {
+    const mockLayer1: MockLayer = new MockLayer('layer-1');
+    const mockLayer2: MockLayer = new MockLayer('layer-11');
+    mockLayer1.setProperty('visibility', 'none');
+    const mockMap: MockMaplibreMap = new MockMaplibreMap([mockLayer1, mockLayer2]);
+    updateVisibility((mockMap as unknown) as Maplibre, 'layer-1', 'visible');
+    expect(mockMap.getLayers().map((layer) => String(layer.getProperty('visibility')))).toEqual(
+      Array(2).fill('visible')
+    );
+  });
+});
diff --git a/public/model/map/layer_operations.ts b/public/model/map/layer_operations.ts
@@ -11,7 +11,7 @@ export const getLayers = (map: Maplibre, dashboardMapsLayerId?: string): LayerSp
     : layers;
 };
 
-export const hasLayer = (map: Maplibre, dashboardMapsLayerId: string) => {
+export const hasLayer = (map: Maplibre, dashboardMapsLayerId: string): boolean => {
   const maplibreMapLayers = getLayers(map);
   for (const layer of maplibreMapLayers) {
     if (layer.id.includes(dashboardMapsLayerId)) {
@@ -42,6 +42,12 @@ export const removeLayers = (map: Maplibre, layerId: string, removeSource?: bool
   }
 };
 
+export const updateVisibility = (map: Maplibre, layerId: string, visibility: string) => {
+  getLayers(map, layerId).forEach((layer) => {
+    map.setLayoutProperty(layer.id, 'visibility', visibility);
+  });
+};
+
 export interface LineLayerSpecification {
   sourceId: string;
   visibility: string;