refactor(details): Improve performance (#2650)

* refactor(performance): Improve details Closes #2639 * refactor(details): Improve performance for details Closes #2639 * fix double query * fix some condition on panel * fix condition, rename file * Typos --------- Co-authored-by: jolevesq <[email protected]>
Canadian-Geospatial-Platform · Dec 13, 2024 · e669a5a · e669a5a
1 parent 0041cbf
commit e669a5a
Show file tree

Hide file tree

Showing 14 changed files with 448 additions and 353 deletions.
diff --git a/packages/geoview-core/public/configs/navigator/28-geocore.json b/packages/geoview-core/public/configs/navigator/28-geocore.json
@@ -25,6 +25,10 @@
       {
         "geoviewLayerType": "geoCore",
         "geoviewLayerId": "03ccfb5c-a06e-43e3-80fd-09d4f8f69703"
+      },
+      {
+        "geoviewLayerType": "geoCore",
+        "geoviewLayerId": "6433173f-bca8-44e6-be8e-3e8a19d3c299"
       }
     ]
   },

diff --git a/...ew-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts b/...ew-core/src/api/event-processors/event-processor-children/feature-info-event-processor.ts
@@ -164,23 +164,18 @@ export class FeatureInfoEventProcessor extends AbstractEventProcessor {
 
     // Depending on the event type
     if (eventType === 'click') {
-      const atLeastOneFeature = layerDataArray.find((layerEntry) => !!layerEntry.features?.length) || false;
+      // Show details panel as soon as there is a click on the map
+      // If the current tab is not 'details' nor 'geochart', switch to details
+      if (!['details', 'geochart'].includes(UIEventProcessor.getActiveFooterBarTab(mapId))) {
+        UIEventProcessor.setActiveFooterBarTab(mapId, 'details');
+      }
+      // Open details appbar tab when user clicked on map layer.
+      if (UIEventProcessor.getAppBarComponents(mapId).includes('details')) {
+        UIEventProcessor.setActiveAppBarTab(mapId, `${mapId}AppbarPanelButtonDetails`, 'details', true, true);
+      }
 
       // Update the layer data array in the store, all the time, for all statuses
       featureInfoState.setterActions.setLayerDataArray(layerDataArray);
-
-      // If there was some features on this propagation
-      if (atLeastOneFeature) {
-        // If the current tab is not 'details' nor 'geochart', switch to details
-        if (!['details', 'geochart'].includes(UIEventProcessor.getActiveFooterBarTab(mapId))) {
-          UIEventProcessor.setActiveFooterBarTab(mapId, 'details');
-        }
-
-        // Open details appbar tab when user clicked on map layer.
-        if (UIEventProcessor.getAppBarComponents(mapId).includes('details')) {
-          UIEventProcessor.setActiveAppBarTab(mapId, `${mapId}AppbarPanelButtonDetails`, 'details', true, true);
-        }
-      }
     } else if (eventType === 'name') {
       // Update the layer data array in the store, all the time, for all statuses
       featureInfoState.setterActions.setLayerDataArray(layerDataArray);

diff --git a/...ges/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts b/...ges/geoview-core/src/api/event-processors/event-processor-children/map-event-processor.ts
@@ -48,7 +48,7 @@ import { UIEventProcessor } from './ui-event-processor';
 import { TypeMapFeaturesConfig } from '@/core/types/global-types';
 import { TypeClickMarker } from '@/core/components';
 import { IMapState, TypeOrderedLayerInfo, TypeScaleInfo } from '@/core/stores/store-interface-and-intial-values/map-state';
-import { TypeFeatureInfoResultSet, TypeHoverFeatureInfo } from '@/core/stores/store-interface-and-intial-values/feature-info-state';
+import { TypeHoverFeatureInfo } from '@/core/stores/store-interface-and-intial-values/feature-info-state';
 import { TypeBasemapProps } from '@/geo/layer/basemap/basemap-types';
 import { LegendEventProcessor } from './legend-event-processor';
 import { TypeLegendLayer } from '@/core/components/layers/types';
@@ -109,6 +109,20 @@ export class MapEventProcessor extends AbstractEventProcessor {
               (removedFeatures[i].geometry as TypeGeometry).ol_uid
             );
         }
+      },
+      {
+        equalityFn: (prev, curr) => {
+          // Quick length checks first (prevents re-render) and calls to removeHighlight
+          if (prev === curr) return true;
+          if (prev.length !== curr.length) return false;
+          if (prev.length === 0) return true;
+
+          // Use Set for O(1) lookup instead of array operations
+          const prevUids = new Set(prev.map((feature) => (feature.geometry as TypeGeometry).ol_uid));
+
+          // Single pass through current features
+          return curr.every((feature) => prevUids.has((feature.geometry as TypeGeometry).ol_uid));
+        },
       }
     );
 
@@ -412,15 +426,10 @@ export class MapEventProcessor extends AbstractEventProcessor {
     this.getMapStateProtected(mapId).setterActions.setPointerPosition(pointerPosition);
   }
 
-  static setClickCoordinates(mapId: string, clickCoordinates: TypeMapMouseInfo): Promise<TypeFeatureInfoResultSet> {
-    // Perform query via the feature info layer set process
-    const promise = this.getMapViewerLayerAPI(mapId).featureInfoLayerSet.queryLayers(clickCoordinates.lnglat);
-
+  static setClickCoordinates(mapId: string, clickCoordinates: TypeMapMouseInfo): void {
+    // GV: We do not need to perform query, there is a handler on the map click in layer set.
     // Save in store
     this.getMapStateProtected(mapId).setterActions.setClickCoordinates(clickCoordinates);
-
-    // Return the promise
-    return promise;
   }
 
   static setZoom(mapId: string, zoom: number): void {

diff --git a/packages/geoview-core/src/core/components/crosshair/crosshair.tsx b/packages/geoview-core/src/core/components/crosshair/crosshair.tsx
@@ -47,10 +47,7 @@ export const Crosshair = memo(function Crosshair({ mapTargetElement }: Crosshair
       logger.logTraceUseCallback('CROSSHAIR - simulateClick', pointerPosition);
       if (event.key === 'Enter' && pointerPosition) {
         // Update the store
-        setClickCoordinates(pointerPosition).catch((error) => {
-          // Log
-          logger.logPromiseFailed('Failed to setClickCoordinates in crosshair.simulateClick', error);
-        });
+        setClickCoordinates(pointerPosition);
       }
     },
     [pointerPosition, setClickCoordinates]

diff --git a/packages/geoview-core/src/core/components/details/details-panel.tsx b/packages/geoview-core/src/core/components/details/details-panel.tsx
@@ -15,9 +15,9 @@ import { TypeFeatureInfoEntry, TypeGeometry, TypeLayerData } from '@/geo/map/map
 
 import { LayerListEntry, Layout } from '@/core/components/common';
 import { getSxClasses } from './details-style';
-import { FeatureInfo } from './feature-info-new';
-import { LAYER_STATUS, TABS } from '@/core/utils/constant';
-import DetailsSkeleton from './details-skeleton';
+import { FeatureInfo } from './feature-info';
+import { FEATURE_INFO_STATUS, TABS } from '@/core/utils/constant';
+import { DetailsSkeleton } from './details-skeleton';
 
 interface DetailsPanelType {
   fullWidth?: boolean;
@@ -29,38 +29,31 @@ interface DetailsPanelType {
  * @returns {JSX.Element} the layers list
  */
 export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Element {
-  // Log
   logger.logTraceRender('components/details/details-panel');
 
+  // Hooks
   const { t } = useTranslation<string>();
-
   const theme = useTheme();
   const sxClasses = getSxClasses(theme);
 
-  // Get states and actions from store
+  // Store
   const mapId = useGeoViewMapId();
   const selectedLayerPath = useDetailsSelectedLayerPath();
   const arrayOfLayerDataBatch = useDetailsLayerDataArrayBatch();
   const checkedFeatures = useDetailsCheckedFeatures();
   const visibleLayers = useMapVisibleLayers();
   const mapClickCoordinates = useMapClickCoordinates();
-
   const { setSelectedLayerPath, removeCheckedFeature, setLayerDataArrayBatchLayerPathBypass } = useDetailsStoreActions();
   const { addHighlightedFeature, removeHighlightedFeature } = useMapStoreActions();
 
-  // #region USE STATE SECTION ****************************************************************************************
-
-  // internal state
+  // States
   const [currentFeatureIndex, setCurrentFeatureIndex] = useState<number>(0);
   const [selectedLayerPathLocal, setselectedLayerPathLocal] = useState<string>(selectedLayerPath);
   const [arrayOfLayerListLocal, setArrayOfLayerListLocal] = useState<LayerListEntry[]>([]);
-
   const prevLayerSelected = useRef<TypeLayerData>();
   const prevLayerFeatures = useRef<TypeFeatureInfoEntry[] | undefined | null>();
   const prevFeatureIndex = useRef<number>(0); // 0 because that's the default index for the features
 
-  // #endregion
-
   // #region MAIN HOOKS SECTION ***************************************************************************************
 
   /**
@@ -69,33 +62,36 @@ export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Eleme
    * @param {TypeFeatureInfoEntry} feature The feature to check
    * @returns {boolean} true if feature is in checkedFeatures
    */
+  // Create a memoized Set of checked feature IDs
+  const checkedFeaturesSet = useMemo(() => {
+    return new Set(checkedFeatures.map((feature) => (feature.geometry as TypeGeometry)?.ol_uid));
+  }, [checkedFeatures]);
+
+  // Modified isFeatureInCheckedFeatures using the Set for O(1) lookup
   const isFeatureInCheckedFeatures = useCallback(
     (feature: TypeFeatureInfoEntry): boolean => {
-      // Log
-      logger.logTraceUseCallback('DETAILS-PANEL - isFeatureInCheckedFeatures');
-
-      return checkedFeatures.some((checkedFeature) => {
-        return (checkedFeature.geometry as TypeGeometry)?.ol_uid === (feature.geometry as TypeGeometry)?.ol_uid;
-      });
+      return checkedFeaturesSet.has((feature.geometry as TypeGeometry)?.ol_uid);
     },
-    [checkedFeatures]
+    [checkedFeaturesSet]
   );
 
   /**
    * Clears the highlighed features when they are not checked.
    * @param {TypeFeatureInfoEntry[] | undefined | null} arrayToClear The array to clear of the unchecked features
    */
+  // Modified clearHighlightsUnchecked
   const clearHighlightsUnchecked = useCallback(
     (arrayToClear: TypeFeatureInfoEntry[] | undefined | null) => {
-      // Log
       logger.logTraceUseCallback('DETAILS-PANEL - clearHighlightsUnchecked');
 
-      // Clear any feature that's not currently checked
       arrayToClear?.forEach((feature) => {
-        if (!isFeatureInCheckedFeatures(feature)) removeHighlightedFeature(feature);
+        const featureId = (feature.geometry as TypeGeometry)?.ol_uid;
+        if (!checkedFeaturesSet.has(featureId)) {
+          removeHighlightedFeature(feature);
+        }
       });
     },
-    [isFeatureInCheckedFeatures, removeHighlightedFeature]
+    [checkedFeaturesSet, removeHighlightedFeature]
   );
 
   /**
@@ -301,7 +297,6 @@ export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Eleme
   // #endregion
 
   // #region EVENT HANDLERS SECTION ***********************************************************************************
-
   /**
    * Handles click to remove all features in right panel.
    */
@@ -348,7 +343,6 @@ export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Eleme
     },
     [setSelectedLayerPath]
   );
-
   // #endregion
 
   // #region PROCESSING ***********************************************************************************************
@@ -414,31 +408,39 @@ export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Eleme
   }, [mapClickCoordinates, memoLayersList]);
 
   /**
-   * Check all layers status is processing while querying
+   * Check all layers status is processed while querying
    */
-  const memoIsAllLayersQueryStatusProcessing = useMemo(() => {
+  const memoIsAllLayersQueryStatusProcessed = useMemo(() => {
     // Log
-    logger.logTraceUseMemo('DETAILS-PANEL - order layer status processing.');
+    logger.logTraceUseMemo('DETAILS-PANEL - AllLayersQueryStatusProcessed.');
 
     if (!arrayOfLayerDataBatch || arrayOfLayerDataBatch?.length === 0) return () => false;
 
-    return () => !!arrayOfLayerDataBatch?.every((layer) => layer.queryStatus === LAYER_STATUS.PROCESSING);
+    return () => arrayOfLayerDataBatch?.every((layer) => layer.queryStatus === FEATURE_INFO_STATUS.PROCESSED);
   }, [arrayOfLayerDataBatch]);
 
   // #endregion
 
-  // #region RENDER SECTION *******************************************************************************************
-
   /**
    * Render the right panel content based on detail's layer and loading status.
    * NOTE: Here we return null, so that in responsive grid layout, it can be used as flag to render the guide for details.
    * @returns {JSX.Element | null} JSX.Element | null
    */
   const renderContent = (): JSX.Element | null => {
-    if (memoIsAllLayersQueryStatusProcessing()) {
+    // If there is no layer, return null for the guide to show
+    if ((memoLayersList && memoLayersList.length === 0) || selectedLayerPath === '') {
+      return null;
+    }
+
+    // Until process or something found for selected layerPath, return skeleton
+    if (!memoIsAllLayersQueryStatusProcessed() && !(memoSelectedLayerDataFeatures && memoSelectedLayerDataFeatures.length > 0)) {
       return <DetailsSkeleton />;
     }
+
     if (memoSelectedLayerDataFeatures && memoSelectedLayerDataFeatures.length > 0) {
+      // Get only the current feature
+      const currentFeature = memoSelectedLayerDataFeatures[currentFeatureIndex];
+
       return (
         <Box sx={fullWidth ? sxClasses.rightPanelContainer : { ...sxClasses.rightPanelContainer }}>
           <Grid container sx={sxClasses.rightPanelBtnHolder}>
@@ -486,10 +488,12 @@ export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Eleme
               </Box>
             </Grid>
           </Grid>
-          <FeatureInfo features={memoSelectedLayerData?.features} currentFeatureIndex={currentFeatureIndex} />
+          <FeatureInfo feature={currentFeature} />
         </Box>
       );
     }
+
+    // if no condition met, return null for Guide tab
     return null;
   };
 
@@ -505,6 +509,4 @@ export function DetailsPanel({ fullWidth = false }: DetailsPanelType): JSX.Eleme
       {renderContent()}
     </Layout>
   );
-
-  // # endregion
 }
diff --git a/packages/geoview-core/src/core/components/details/details-skeleton.tsx b/packages/geoview-core/src/core/components/details/details-skeleton.tsx
@@ -1,21 +1,32 @@
+import { memo } from 'react';
 import { Box, Skeleton } from '@/ui';
 
+// Constants outside component to prevent recreating every render
+const sizes = ['15%', '10%', '15%', '25%', '10%', '20%', '10%'];
+
+const SKELETON_STYLES = {
+  box: { padding: '10px' },
+  title: { mb: 1 },
+  text: { pt: 4, pb: 4 },
+} as const;
+
 /**
  * Custom details skeleton build with mui skeleton component.
  * @returns {JSX.Element}
  */
-export default function DetailsSkeleton(): JSX.Element {
-  const sizes = ['15%', '10%', '15%', '25%', '10%', '20%', '10%'];
+// Memoizes entire component, preventing re-renders if props haven't changed
+export const DetailsSkeleton = memo(function DetailsSkeleton(): JSX.Element {
   return (
-    <Box padding={8}>
-      <Box pb={8}>
+    <Box sx={SKELETON_STYLES.box}>
+      <Skeleton variant="text" width="60%" height={32} sx={SKELETON_STYLES.title} />
+      <Box sx={SKELETON_STYLES.box}>
         {sizes.map((size, index) => (
-          <Box display="flex" justifyContent="space-between" pt={4} pb={4} key={`${index.toString()}-${size}}`}>
+          <Box display="flex" justifyContent="space-between" sx={SKELETON_STYLES.text} key={`${index.toString()}-${size}}`}>
             <Skeleton variant="text" width={size} height="25px" />
             <Skeleton variant="text" width={size} height="25px" />
           </Box>
         ))}
       </Box>
     </Box>
   );
-}
+});