Sync maplibre layer order after layers rendered (#353)

* Sync maplibre layer order after layers rendered Signed-off-by: Junqiu Lei <junqiu@amazon.com> * Resolve feedback Signed-off-by: Junqiu Lei <junqiu@amazon.com> --------- Signed-off-by: Junqiu Lei <junqiu@amazon.com>
opensearch-project · Mar 20, 2023 · d2c818f · d2c818f
1 parent c5ee47a
commit d2c818f
Show file tree

Hide file tree

Showing 9 changed files with 104 additions and 154 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -36,6 +36,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 * Fix color picker component issue ([#305](https://github.com/opensearch-project/dashboards-maps/pull/305))
 * fix: layer filter setting been reset unexpectedly ([#327](https://github.com/opensearch-project/dashboards-maps/pull/327))
 * Fix data query in dashboard mode when enable around map filter ([#339](https://github.com/opensearch-project/dashboards-maps/pull/339))
+* Sync maplibre layer order after layers rendered ([#353](https://github.com/opensearch-project/dashboards-maps/pull/353))
 
 ### Infrastructure
 * Add CHANGELOG ([#342](https://github.com/opensearch-project/dashboards-maps/pull/342))

diff --git a/public/components/map_container/map_container.tsx b/public/components/map_container/map_container.tsx
@@ -24,6 +24,7 @@ import {
   renderBaseLayers,
   handleDataLayerRender,
   handleBaseLayerRender,
+  orderLayers,
 } from '../../model/layerRenderController';
 import { useOpenSearchDashboards } from '../../../../../src/plugins/opensearch_dashboards_react/public';
 import { ResizeChecker } from '../../../../../src/plugins/opensearch_dashboards_utils/public';
@@ -165,20 +166,28 @@ export const MapContainer = ({
       return;
     }
 
+    const orderLayersAfterRenderLoaded = () => orderLayers(layers, maplibreRef.current!);
+
     if (isUpdatingLayerRender || isReadOnlyMode) {
       if (selectedLayerConfig) {
         if (baseLayerTypeLookup[selectedLayerConfig.type]) {
-          handleBaseLayerRender(selectedLayerConfig, maplibreRef, undefined);
+          handleBaseLayerRender(selectedLayerConfig, maplibreRef);
         } else {
           updateIndexPatterns();
-          handleDataLayerRender(selectedLayerConfig, mapState, services, maplibreRef, undefined);
+          handleDataLayerRender(selectedLayerConfig, mapState, services, maplibreRef);
         }
       } else {
         renderDataLayers(layers, mapState, services, maplibreRef, timeRange, filters, query);
         renderBaseLayers(layers, maplibreRef);
+        // Because of async layer rendering, layers order is not guaranteed, so we need to order layers
+        // after all layers are rendered.
+        maplibreRef.current!.once('idle', orderLayersAfterRenderLoaded);
       }
       setIsUpdatingLayerRender(false);
     }
+    return () => {
+      maplibreRef.current!.off('idle', orderLayersAfterRenderLoaded);
+    };
   }, [layers, mounted, timeRange, filters, query, mapState, isReadOnlyMode]);
 
   useEffect(() => {

diff --git a/public/components/map_top_nav/top_nav_menu.tsx b/public/components/map_top_nav/top_nav_menu.tsx
@@ -92,7 +92,7 @@ export const MapTopNavMenu = ({
   const refreshDataLayerRender = () => {
     layers.forEach((layer: MapLayerSpecification) => {
       if (layer.type === DASHBOARDS_MAPS_LAYER_TYPE.DOCUMENTS) {
-        handleDataLayerRender(layer, mapState, services, maplibreRef, undefined);
+        handleDataLayerRender(layer, mapState, services, maplibreRef);
       }
     });
   };

diff --git a/public/model/OSMLayerFunctions.ts b/public/model/OSMLayerFunctions.ts
@@ -1,6 +1,5 @@
 import { Map as Maplibre, LayerSpecification, SymbolLayerSpecification } from 'maplibre-gl';
 import { OSMLayerSpecification } from './mapLayerType';
-import { getMaplibreBeforeLayerId } from './layersFunctions';
 import { getLayers, hasLayer } from './map/layer_operations';
 import { getMapLanguage } from '../../common/util';
 
@@ -53,26 +52,21 @@ const setLanguage = (maplibreRef: MaplibreRef, styleLayer: LayerSpecification) =
   }
 };
 
-const addNewLayer = (
-  layerConfig: OSMLayerSpecification,
-  maplibreRef: MaplibreRef,
-  beforeLayerId: string | undefined
-) => {
+const addNewLayer = (layerConfig: OSMLayerSpecification, maplibreRef: MaplibreRef) => {
   if (maplibreRef.current) {
     const { source, style } = layerConfig;
     maplibreRef.current.addSource(layerConfig.id, {
       type: 'vector',
       url: source?.dataURL,
     });
     fetchStyleLayers(style?.styleURL).then((styleLayers: LayerSpecification[]) => {
-      const beforeMbLayerId = getMaplibreBeforeLayerId(layerConfig, maplibreRef, beforeLayerId);
       styleLayers.forEach((styleLayer) => {
         styleLayer.id = styleLayer.id + '_' + layerConfig.id;
         // TODO: Add comments on why we skip background type
         if (styleLayer.type !== 'background') {
           styleLayer.source = layerConfig.id;
         }
-        maplibreRef.current?.addLayer(styleLayer, beforeMbLayerId);
+        maplibreRef.current?.addLayer(styleLayer);
         setLanguage(maplibreRef, styleLayer);
         maplibreRef.current?.setLayoutProperty(styleLayer.id, 'visibility', layerConfig.visibility);
         maplibreRef.current?.setLayerZoomRange(
@@ -96,15 +90,11 @@ const addNewLayer = (
 
 // Functions for OpenSearch maps vector tile layer
 export const OSMLayerFunctions = {
-  render: (
-    maplibreRef: MaplibreRef,
-    layerConfig: OSMLayerSpecification,
-    beforeLayerId: string | undefined
-  ) => {
+  render: (maplibreRef: MaplibreRef, layerConfig: OSMLayerSpecification) => {
     // If layer already exist in maplibre source, update layer config
     // else add new layer.
     return hasLayer(maplibreRef.current!, layerConfig.id)
       ? updateLayerConfig(layerConfig, maplibreRef)
-      : addNewLayer(layerConfig, maplibreRef, beforeLayerId);
+      : addNewLayer(layerConfig, maplibreRef);
   },
 };
diff --git a/public/model/customLayerFunctions.ts b/public/model/customLayerFunctions.ts
@@ -1,6 +1,5 @@
 import { Map as Maplibre, AttributionControl, RasterSourceSpecification } from 'maplibre-gl';
 import { CustomLayerSpecification, OSMLayerSpecification } from './mapLayerType';
-import { getMaplibreBeforeLayerId } from './layersFunctions';
 import { hasLayer, removeLayers } from './map/layer_operations';
 
 interface MaplibreRef {
@@ -44,11 +43,7 @@ const updateLayerConfig = (layerConfig: CustomLayerSpecification, maplibreRef: M
   }
 };
 
-const addNewLayer = (
-  layerConfig: CustomLayerSpecification,
-  maplibreRef: MaplibreRef,
-  beforeLayerId: string | undefined
-) => {
+const addNewLayer = (layerConfig: CustomLayerSpecification, maplibreRef: MaplibreRef) => {
   const maplibreInstance = maplibreRef.current;
   if (maplibreInstance) {
     const tilesURL = getCustomMapURL(layerConfig);
@@ -59,23 +54,19 @@ const addNewLayer = (
       tileSize: 256,
       attribution: layerSource?.attribution,
     });
-    const beforeMbLayerId = getMaplibreBeforeLayerId(layerConfig, maplibreRef, beforeLayerId);
-    maplibreInstance.addLayer(
-      {
-        id: layerConfig.id,
-        type: 'raster',
-        source: layerConfig.id,
-        paint: {
-          'raster-opacity': layerConfig.opacity / 100,
-        },
-        layout: {
-          visibility: layerConfig.visibility === 'visible' ? 'visible' : 'none',
-        },
-        minzoom: layerConfig.zoomRange[0],
-        maxzoom: layerConfig.zoomRange[1],
+    maplibreInstance.addLayer({
+      id: layerConfig.id,
+      type: 'raster',
+      source: layerConfig.id,
+      paint: {
+        'raster-opacity': layerConfig.opacity / 100,
+      },
+      layout: {
+        visibility: layerConfig.visibility === 'visible' ? 'visible' : 'none',
       },
-      beforeMbLayerId
-    );
+      minzoom: layerConfig.zoomRange[0],
+      maxzoom: layerConfig.zoomRange[1],
+    });
   }
 };
 
@@ -92,14 +83,10 @@ const getCustomMapURL = (layerConfig: CustomLayerSpecification) => {
 };
 
 export const CustomLayerFunctions = {
-  render: (
-    maplibreRef: MaplibreRef,
-    layerConfig: CustomLayerSpecification,
-    beforeLayerId: string | undefined
-  ) => {
+  render: (maplibreRef: MaplibreRef, layerConfig: CustomLayerSpecification) => {
     return hasLayer(maplibreRef.current!, layerConfig.id)
       ? updateLayerConfig(layerConfig, maplibreRef)
-      : addNewLayer(layerConfig, maplibreRef, beforeLayerId);
+      : addNewLayer(layerConfig, maplibreRef);
   },
   remove: (maplibreRef: MaplibreRef, layerConfig: OSMLayerSpecification) => {
     removeLayers(maplibreRef.current!, layerConfig.id, true);

diff --git a/public/model/documentLayerFunctions.ts b/public/model/documentLayerFunctions.ts
@@ -7,7 +7,6 @@ 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 {
   addCircleLayer,
   addLineLayer,
@@ -131,56 +130,43 @@ const addNewLayer = (
   if (!maplibreInstance) {
     return;
   }
-  const mbLayerBeforeId = getMaplibreBeforeLayerId(layerConfig, maplibreRef, beforeLayerId);
   const source = getLayerSource(data, layerConfig);
   maplibreInstance.addSource(layerConfig.id, {
     type: 'geojson',
     data: source,
   });
-  addCircleLayer(
-    maplibreInstance,
-    {
-      fillColor: layerConfig.style?.fillColor,
+  addCircleLayer(maplibreInstance, {
+    fillColor: layerConfig.style?.fillColor,
+    maxZoom: layerConfig.zoomRange[1],
+    minZoom: layerConfig.zoomRange[0],
+    opacity: layerConfig.opacity,
+    outlineColor: layerConfig.style?.borderColor,
+    radius: layerConfig.style?.markerSize,
+    sourceId: layerConfig.id,
+    visibility: layerConfig.visibility,
+    width: layerConfig.style?.borderThickness,
+  });
+  const geoFieldType = getGeoFieldType(layerConfig);
+  if (geoFieldType === 'geo_shape') {
+    addLineLayer(maplibreInstance, {
+      width: layerConfig.style?.borderThickness,
+      color: layerConfig.style?.fillColor,
       maxZoom: layerConfig.zoomRange[1],
       minZoom: layerConfig.zoomRange[0],
       opacity: layerConfig.opacity,
-      outlineColor: layerConfig.style?.borderColor,
-      radius: layerConfig.style?.markerSize,
       sourceId: layerConfig.id,
       visibility: layerConfig.visibility,
+    });
+    addPolygonLayer(maplibreInstance, {
       width: layerConfig.style?.borderThickness,
-    },
-    mbLayerBeforeId
-  );
-  const geoFieldType = getGeoFieldType(layerConfig);
-  if (geoFieldType === 'geo_shape') {
-    addLineLayer(
-      maplibreInstance,
-      {
-        width: layerConfig.style?.borderThickness,
-        color: layerConfig.style?.fillColor,
-        maxZoom: layerConfig.zoomRange[1],
-        minZoom: layerConfig.zoomRange[0],
-        opacity: layerConfig.opacity,
-        sourceId: layerConfig.id,
-        visibility: layerConfig.visibility,
-      },
-      mbLayerBeforeId
-    );
-    addPolygonLayer(
-      maplibreInstance,
-      {
-        width: layerConfig.style?.borderThickness,
-        fillColor: layerConfig.style?.fillColor,
-        maxZoom: layerConfig.zoomRange[1],
-        minZoom: layerConfig.zoomRange[0],
-        opacity: layerConfig.opacity,
-        sourceId: layerConfig.id,
-        outlineColor: layerConfig.style?.borderColor,
-        visibility: layerConfig.visibility,
-      },
-      mbLayerBeforeId
-    );
+      fillColor: layerConfig.style?.fillColor,
+      maxZoom: layerConfig.zoomRange[1],
+      minZoom: layerConfig.zoomRange[0],
+      opacity: layerConfig.opacity,
+      sourceId: layerConfig.id,
+      outlineColor: layerConfig.style?.borderColor,
+      visibility: layerConfig.visibility,
+    });
   }
 };
 

diff --git a/public/model/layerRenderController.ts b/public/model/layerRenderController.ts
@@ -18,12 +18,7 @@ import {
   Query,
   FILTERS,
 } from '../../../../src/plugins/data/common';
-import {
-  getDataLayers,
-  getMapBeforeLayerId,
-  getBaseLayers,
-  layersFunctionMap,
-} from './layersFunctions';
+import { getDataLayers, getBaseLayers, layersFunctionMap } from './layersFunctions';
 import { MapServices } from '../types';
 import { MapState } from './mapState';
 import { GeoBounds, getBounds } from './map/boundary';
@@ -106,7 +101,6 @@ export const handleDataLayerRender = (
   mapState: MapState,
   services: MapServices,
   maplibreRef: MaplibreRef,
-  beforeLayerId: string | undefined,
   timeRange?: TimeRange,
   filtersFromDashboard?: Filter[],
   query?: Query
@@ -135,18 +129,17 @@ export const handleDataLayerRender = (
     (result) => {
       const { layer, dataSource } = result;
       if (layer.type === DASHBOARDS_MAPS_LAYER_TYPE.DOCUMENTS) {
-        layersFunctionMap[layer.type].render(maplibreRef, layer, dataSource, beforeLayerId);
+        layersFunctionMap[layer.type].render(maplibreRef, layer, dataSource);
       }
     }
   );
 };
 
 export const handleBaseLayerRender = (
   layer: MapLayerSpecification,
-  maplibreRef: MaplibreRef,
-  beforeLayerId: string | undefined
+  maplibreRef: MaplibreRef
 ): void => {
-  layersFunctionMap[layer.type].render(maplibreRef, layer, beforeLayerId);
+  layersFunctionMap[layer.type].render(maplibreRef, layer);
 };
 
 export const renderDataLayers = (
@@ -159,13 +152,11 @@ export const renderDataLayers = (
   query?: Query
 ): void => {
   getDataLayers(layers).forEach((layer) => {
-    const beforeLayerId = getMapBeforeLayerId(layers, layer.id);
     handleDataLayerRender(
       layer,
       mapState,
       services,
       maplibreRef,
-      beforeLayerId,
       timeRange,
       filtersFromDashboard,
       query
@@ -178,7 +169,19 @@ export const renderBaseLayers = (
   maplibreRef: MaplibreRef
 ): void => {
   getBaseLayers(layers).forEach((layer) => {
-    const beforeLayerId = getMapBeforeLayerId(layers, layer.id);
-    handleBaseLayerRender(layer, maplibreRef, beforeLayerId);
+    handleBaseLayerRender(layer, maplibreRef);
+  });
+};
+
+// Order maplibre layers based on the order of dashboard-maps layers
+export const orderLayers = (mapLayers: MapLayerSpecification[], maplibre: Maplibre): void => {
+  const maplibreLayers = maplibre.getStyle().layers;
+  if (!maplibreLayers) return;
+  mapLayers.forEach((layer) => {
+    const layerId = layer.id;
+    const mbLayers = maplibreLayers.filter((mbLayer) => mbLayer.id.includes(layerId));
+    mbLayers.forEach((mbLayer, index) => {
+      maplibre.moveLayer(mbLayer.id);
+    });
   });
 };
diff --git a/public/model/layersFunctions.ts b/public/model/layersFunctions.ts
@@ -67,9 +67,7 @@ export const baseLayerTypeLookup = {
 };
 
 export const getDataLayers = (layers: MapLayerSpecification[]): DataLayerSpecification[] => {
-  return layers.filter(
-    (layer) => !baseLayerTypeLookup[layer.type]
-  ) as DataLayerSpecification[];
+  return layers.filter((layer) => !baseLayerTypeLookup[layer.type]) as DataLayerSpecification[];
 };
 
 export const getBaseLayers = (layers: MapLayerSpecification[]): BaseLayerSpecification[] => {