refactor(component): Review component to improve performance (#2628)

* refactor(component): Review component to improve performance Closes #2626 * Mostly style and finisinh simple component * Fix styles * fix event, style, other --------- Co-authored-by: jolevesq <[email protected]>
Canadian-Geospatial-Platform · Dec 5, 2024 · 7f0d0b4 · 7f0d0b4
1 parent 24f3835
commit 7f0d0b4
Show file tree

Hide file tree

Showing 80 changed files with 1,252 additions and 648 deletions.
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
@@ -427,6 +427,11 @@ export class MapEventProcessor extends AbstractEventProcessor {
     this.getMapStateProtected(mapId).setterActions.setZoom(zoom);
   }
 
+  static setIsMouseInsideMap(mapId: string, inside: boolean): void {
+    // Save in store
+    this.getMapStateProtected(mapId).setterActions.setIsMouseInsideMap(inside);
+  }
+
   static setRotation(mapId: string, rotation: number): void {
     // Save in store
     this.getMapStateProtected(mapId).setterActions.setRotation(rotation);

diff --git a/packages/geoview-core/src/core/components/app-bar/app-bar-style.ts b/packages/geoview-core/src/core/components/app-bar/app-bar-style.ts
@@ -1,8 +1,13 @@
 import { Theme } from '@mui/material/styles';
+import { SxStyles } from '@/ui/style/types';
 
-// ? I doubt we want to define an explicit type for style properties?
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const getSxClasses = (theme: Theme): any => ({
+/**
+ * Get custom sx classes for the app-bar
+ *
+ * @param {Theme} theme the theme object
+ * @returns {Object} the sx classes object
+ */
+export const getSxClasses = (theme: Theme): SxStyles => ({
   appBar: {
     display: 'flex',
     flexDirection: 'row',

diff --git a/packages/geoview-core/src/core/components/app-bar/app-bar.tsx b/packages/geoview-core/src/core/components/app-bar/app-bar.tsx
@@ -31,16 +31,7 @@ import { useMapInteraction, useMapStoreActions } from '@/core/stores/store-inter
 import { useAppFullscreenActive, useAppGeoviewHTMLElement } from '@/core/stores/store-interface-and-intial-values/app-state';
 import { useGeoViewConfig, useGeoViewMapId } from '@/core/stores/geoview-store';
 import { logger } from '@/core/utils/logger';
-import {
-  GuidePanel,
-  Legend,
-  DetailsPanel,
-  AppBarApi,
-  AppBarCreatedEvent,
-  AppBarRemovedEvent,
-  Datapanel,
-  LayersPanel,
-} from '@/core/components';
+import { Guide, Legend, DetailsPanel, AppBarApi, AppBarCreatedEvent, AppBarRemovedEvent, Datapanel, LayersPanel } from '@/core/components';
 import Notifications from '@/core/components/notifications/notifications';
 
 import Version from './buttons/version';
@@ -118,7 +109,7 @@ export function AppBar(props: AppBarProps): JSX.Element {
     }
     return {
       geolocator: { icon: <SearchIcon />, content: <Geolocator key="geolocator" /> },
-      guide: { icon: <QuestionMarkIcon />, content: <GuidePanel fullWidth /> },
+      guide: { icon: <QuestionMarkIcon />, content: <Guide fullWidth /> },
       details: { icon: <InfoOutlinedIcon />, content: <DetailsPanel fullWidth /> },
       legend: { icon: <HubOutlinedIcon />, content: <Legend fullWidth containerType={CONTAINER_TYPE.APP_BAR} /> },
       layers: { icon: <LayersOutlinedIcon />, content: <LayersPanel containerType={CONTAINER_TYPE.APP_BAR} /> },

diff --git a/packages/geoview-core/src/core/components/attribution/attribution.tsx b/packages/geoview-core/src/core/components/attribution/attribution.tsx
@@ -1,41 +1,73 @@
-import { useCallback, useState } from 'react';
+import { useCallback, useMemo, useState, memo } from 'react';
 
 import { useTheme } from '@mui/material/styles';
 
 import { Box, MoreHorizIcon, Popover, IconButton, Typography } from '@/ui';
-import { useUIMapInfoExpanded } from '@/core/stores/store-interface-and-intial-values/ui-state';
 import { useMapAttribution } from '@/core/stores/store-interface-and-intial-values/map-state';
-import { generateId } from '@/core/utils/utilities';
 import { useGeoViewMapId } from '@/core/stores/geoview-store';
 import { logger } from '@/core/utils/logger';
 
+// Constants outside component to prevent recreating every render
+const POPOVER_POSITIONS = {
+  anchorOrigin: {
+    vertical: 'top' as const,
+    horizontal: 'right' as const,
+  },
+  transformOrigin: {
+    vertical: 'bottom' as const,
+    horizontal: 'right' as const,
+  },
+} as const;
+
+const BOX_STYLES = { padding: '1rem', width: '28.125rem' } as const;
+
+const ICON_BUTTON_BASE_STYLES = {
+  width: '30px',
+  height: '30px',
+  my: '1rem',
+  margin: 'auto',
+} as const;
+
 /**
  * Create an Attribution component that will display an attribution box
  * with the attribution text
  *
  * @returns {JSX.Element} created attribution element
  */
-export function Attribution(): JSX.Element {
+// Memoizes entire component, preventing re-renders if props haven't changed
+export const Attribution = memo(function Attribution(): JSX.Element {
   // Log
   logger.logTraceRender('components/attribution/attribution');
 
+  // Hooks
   const theme = useTheme();
 
-  const mapId = useGeoViewMapId();
-  const mapElem = document.getElementById(`shell-${mapId}`);
-
-  // internal state
+  // State
   const [anchorEl, setAnchorEl] = useState<HTMLButtonElement | null>(null);
   const open = Boolean(anchorEl);
 
-  // getStore value
+  // Store
   const mapAttribution = useMapAttribution();
-  const expanded = useUIMapInfoExpanded();
 
+  const mapId = useGeoViewMapId();
+  const mapElem = document.getElementById(`shell-${mapId}`);
+
+  const buttonStyles = {
+    ...ICON_BUTTON_BASE_STYLES,
+    color: theme.palette.geoViewColor.bgColor.light[800],
+  };
+
+  // Memoize values
+  const attributionContent = useMemo(
+    () => mapAttribution.map((attribution) => <Typography key={attribution}>{attribution}</Typography>),
+    [mapAttribution]
+  );
+
+  // Callbacks
+  // Popover state expand/collapse
   const handleOpenPopover = useCallback((event: React.MouseEvent<HTMLButtonElement>): void => {
     setAnchorEl(event.currentTarget);
   }, []);
-
   const handleClosePopover = useCallback(() => {
     setAnchorEl(null);
   }, []);
@@ -49,39 +81,23 @@ export function Attribution(): JSX.Element {
         tooltipPlacement="top"
         tooltip="mapctrl.attribution.tooltip"
         aria-label="mapctrl.attribution.tooltip"
-        sx={{
-          color: theme.palette.geoViewColor.bgColor.light[800],
-          marginTop: expanded ? '0.75rem' : '0.25rem',
-          [theme.breakpoints.up('md')]: {
-            marginTop: expanded ? '1.4375rem' : 'none',
-          },
-          width: '30px',
-          height: '30px',
-          my: '1rem',
-        }}
+        sx={buttonStyles}
       >
         <MoreHorizIcon />
       </IconButton>
       <Popover
         open={open}
         anchorEl={anchorEl}
         container={mapElem}
-        anchorOrigin={{
-          vertical: 'top',
-          horizontal: 'right',
-        }}
+        anchorOrigin={POPOVER_POSITIONS.anchorOrigin}
         transformOrigin={{
           vertical: 'bottom',
           horizontal: 'right',
         }}
         onClose={handleClosePopover}
       >
-        <Box sx={{ padding: '1rem', width: '28.125rem' }}>
-          {mapAttribution.map((attribution) => {
-            return <Typography key={generateId()}>{attribution}</Typography>;
-          })}
-        </Box>
+        <Box sx={BOX_STYLES}>{attributionContent}</Box>
       </Popover>
     </>
   );
-}
+});
diff --git a/packages/geoview-core/src/core/components/click-marker/click-marker.tsx b/packages/geoview-core/src/core/components/click-marker/click-marker.tsx
@@ -1,8 +1,8 @@
-import React, { useEffect, useRef } from 'react';
+import { useEffect, useRef, memo } from 'react';
 
 import { Coordinate } from 'ol/coordinate'; // For typing only
-
 import { Box, ClickMapMarker } from '@/ui';
+
 import { useMapClickMarker, useMapClickCoordinates, useMapStoreActions } from '@/core/stores/store-interface-and-intial-values/map-state';
 import { logger } from '@/core/utils/logger';
 import { TypeJsonObject } from '@/core/types/global-types';
@@ -19,21 +19,22 @@ export type TypeClickMarker = {
  *
  * @returns {JSX.Element} the react element with a marker on click
  */
-export function ClickMarker(): JSX.Element {
-  // Log
+// Memoizes entire component, preventing re-renders if props haven't changed
+export const ClickMarker = memo(function ClickMarker(): JSX.Element {
   logger.logTraceRender('components/click-marker/click-marker');
 
-  const mapId = useGeoViewMapId();
-
-  // internal state
+  // State
   const clickMarkerRef = useRef<HTMLDivElement>(null);
-  const clickMarkerId = `${mapId}-clickmarker`;
+  const clickMarkerId = `${useGeoViewMapId()}-clickmarker`;
 
-  // get values from the store
+  // Store
   const clickMarker = useMapClickMarker();
   const clickCoordinates = useMapClickCoordinates();
   const { setOverlayClickMarkerRef, showClickMarker } = useMapStoreActions();
-  setTimeout(() => setOverlayClickMarkerRef(clickMarkerRef.current as HTMLElement), 0);
+
+  useEffect(() => {
+    setOverlayClickMarkerRef(clickMarkerRef.current as HTMLElement);
+  }, [setOverlayClickMarkerRef]);
 
   useEffect(() => {
     // Log
@@ -67,4 +68,4 @@ export function ClickMarker(): JSX.Element {
       />
     </Box>
   );
-}
+});
diff --git a/packages/geoview-core/src/core/components/common/layer-icon-style.ts b/packages/geoview-core/src/core/components/common/layer-icon-style.ts
@@ -1,8 +1,13 @@
 import { Theme } from '@mui/material/styles';
+import { SxStyles } from '@/ui/style/types';
 
-// ? I doubt we want to define an explicit type for style properties?
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const getSxClasses = (theme: Theme): any => ({
+/**
+ * Get custom sx classes for the common layer icon
+ *
+ * @param {Theme} theme the theme object
+ * @returns {Object} the sx classes object
+ */
+export const getSxClasses = (theme: Theme): SxStyles => ({
   legendIconTransparent: {
     display: 'flex',
     justifyContent: 'center',

diff --git a/packages/geoview-core/src/core/components/common/layer-list-style.ts b/packages/geoview-core/src/core/components/common/layer-list-style.ts
@@ -1,8 +1,13 @@
 import { Theme } from '@mui/material';
+import { SxStyles } from '@/ui/style/types';
 
-// ? I doubt we want to define an explicit type for style properties?
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const getSxClasses = (theme: Theme): any => ({
+/**
+ * Get custom sx classes for the common layer list
+ *
+ * @param {Theme} theme the theme object
+ * @returns {Object} the sx classes object
+ */
+export const getSxClasses = (theme: Theme): SxStyles => ({
   list: {
     overflowY: 'auto',
     color: 'text.primary',
@@ -71,8 +76,10 @@ export const getSxClasses = (theme: Theme): any => ({
       },
     },
   },
-  borderWithIndex: `2px solid ${theme.palette.geoViewColor.primary.main} !important`,
-  borderNone: 'none',
+  borderWithIndex: { border: `2px solid ${theme.palette.geoViewColor.primary.main} !important` },
+  borderNone: {
+    border: 'none',
+  },
   headline: { fontSize: theme.palette.geoViewFontSize.md, fontWeight: 'bold' },
   layersInstructionsPaper: {
     padding: '2rem',

diff --git a/packages/geoview-core/src/core/components/common/responsive-grid-layout-style.ts b/packages/geoview-core/src/core/components/common/responsive-grid-layout-style.ts
@@ -1,8 +1,13 @@
 import { Theme } from '@mui/material/styles';
+import { SxStyles } from '@/ui/style/types';
 
-// ? I doubt we want to define an explicit type for style properties?
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const getSxClasses = (theme: Theme): any => ({
+/**
+ * Get custom sx classes for the common grid layout
+ *
+ * @param {Theme} theme the theme object
+ * @returns {Object} the sx classes object
+ */
+export const getSxClasses = (theme: Theme): SxStyles => ({
   rightButtonsContainer: {
     display: 'flex',
     flexDirection: 'row',

diff --git a/packages/geoview-core/src/core/components/crosshair/crosshair-icon.tsx b/packages/geoview-core/src/core/components/crosshair/crosshair-icon.tsx
@@ -1,12 +1,15 @@
+import { memo } from 'react';
+
 /**
  * Create a cross hair icon
  */
-export function CrosshairIcon(): JSX.Element {
+// Memoizes entire component, preventing re-renders if props haven't changed
+export const CrosshairIcon = memo(function CrosshairIcon(): JSX.Element {
   return (
     <svg xmlns="http://www.w3.org/2000/svg" preserveAspectRatio="xMidYMid meet" viewBox="0 0 275 275" focusable="false">
       <g fill="none" stroke="#616161" strokeWidth="1px" id="crosshairs" transform="translate(0 -1824.72) scale(2)">
         <path d="m136.18 983.66-130.93-0.00001m65.467-65.467v130.93m32.2-65.466c0 17.784-14.417 32.2-32.2 32.2-17.784 0-32.2-14.417-32.2-32.2 0-17.784 14.417-32.2 32.2-32.2 17.784 0 32.2 14.417 32.2 32.2z" />
       </g>
     </svg>
   );
-}
+});
diff --git a/packages/geoview-core/src/core/components/crosshair/crosshair-style.ts b/packages/geoview-core/src/core/components/crosshair/crosshair-style.ts
@@ -1,8 +1,13 @@
 import { Theme } from '@mui/material/styles';
+import { SxStyles } from '@/ui/style/types';
 
-// ? I doubt we want to define an explicit type for style properties?
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
-export const getSxClasses = (theme: Theme): any => ({
+/**
+ * Get custom sx classes for the croohair
+ *
+ * @param {Theme} theme the theme object
+ * @returns {Object} the sx classes object
+ */
+export const getSxClasses = (theme: Theme): SxStyles => ({
   crosshairContainer: {
     position: 'absolute',
     top: theme.spacing(0),

diff --git a/packages/geoview-core/src/core/components/crosshair/crosshair.tsx b/packages/geoview-core/src/core/components/crosshair/crosshair.tsx
@@ -1,15 +1,12 @@
-import { useCallback, useEffect, useRef } from 'react';
-
+import { memo, useCallback, useEffect, useRef } from 'react';
 import { useTheme } from '@mui/material/styles';
-
 import { useTranslation } from 'react-i18next';
-
 import { Box, Fade, Typography } from '@/ui';
+
 import { getSxClasses } from './crosshair-style';
 import { CrosshairIcon } from './crosshair-icon';
 import { useAppCrosshairsActive } from '@/core/stores/store-interface-and-intial-values/app-state';
 import { useMapPointerPosition, useMapStoreActions } from '@/core/stores/store-interface-and-intial-values/map-state';
-
 import { logger } from '@/core/utils/logger';
 
 type CrosshairProps = {
@@ -21,24 +18,25 @@ type CrosshairProps = {
  * @param {CrosshairProps} - Crossahir props who caintain the mapTargetELement
  * @returns {JSX.Element} the crosshair component
  */
-export function Crosshair({ mapTargetElement }: CrosshairProps): JSX.Element {
-  // Log
+// Memoizes entire component, preventing re-renders if props haven't changed
+export const Crosshair = memo(function Crosshair({ mapTargetElement }: CrosshairProps): JSX.Element {
   logger.logTraceRender('components/crosshair/crosshair');
 
+  // Hooks
   const { t } = useTranslation<string>();
-
   const theme = useTheme();
   const sxClasses = getSxClasses(theme);
 
-  // get store values
+  // State (no useState for item used inside function only without rendering... use useRef)
+  const panelButtonId = useRef('');
+  const panDelta = useRef(128);
+
+  //  Store
   const isCrosshairsActive = useAppCrosshairsActive();
   const pointerPosition = useMapPointerPosition();
   const { setClickCoordinates, setMapKeyboardPanInteractions } = useMapStoreActions();
 
-  // do not use useState for item used inside function only without rendering... use useRef
-  const panelButtonId = useRef('');
-  const panDelta = useRef(128);
-
+  // Callbacks
   /**
    * Simulate map mouse click to trigger details panel
    * @function simulateClick
@@ -73,7 +71,8 @@ export function Crosshair({ mapTargetElement }: CrosshairProps): JSX.Element {
         setMapKeyboardPanInteractions(panDelta.current);
       }
     },
-    [setMapKeyboardPanInteractions]
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [] // State setters are stable, no need for dependencies
   );
 
   useEffect(() => {
@@ -110,4 +109,4 @@ export function Crosshair({ mapTargetElement }: CrosshairProps): JSX.Element {
       </Box>
     </Box>
   );
-}
+});