From 049e7e5ac5a27dda28152ae7883217a8c57dfc0b Mon Sep 17 00:00:00 2001 From: jolevesq Date: Thu, 5 Dec 2024 10:40:44 -0500 Subject: [PATCH 1/2] refactor(common): Improve performance of common component Closes #2630 --- .../core/components/common/use-lightbox.tsx | 149 +++++++++++------- 1 file changed, 91 insertions(+), 58 deletions(-) diff --git a/packages/geoview-core/src/core/components/common/use-lightbox.tsx b/packages/geoview-core/src/core/components/common/use-lightbox.tsx index 41947cc1d6b..badae399c20 100644 --- a/packages/geoview-core/src/core/components/common/use-lightbox.tsx +++ b/packages/geoview-core/src/core/components/common/use-lightbox.tsx @@ -1,79 +1,112 @@ -import { useState } from 'react'; +import { memo, useCallback, useState } from 'react'; import { Box } from '@/ui'; import { LightBoxSlides, LightboxImg } from '@/core/components/lightbox/lightbox'; import { useUIActiveTrapGeoView } from '@/core/stores/store-interface-and-intial-values/ui-state'; +import { logger } from '@/core/utils/logger'; +// Constants outside component to prevent recreating every render +const FOCUS_DELAY = 250; +const BASE64_IMAGE_PATTERN = /^data:image\/(png|jpeg|gif|webp);base64/; + +// Define props interface for BaseLightBoxComponent +interface BaseLightBoxProps { + isLightBoxOpen: boolean; + slides: LightBoxSlides[]; + slidesIndex: number; + imgScale?: number; + aliasIndex: string; + onExit: () => void; +} interface UseLightBoxReturnType { - initLightBox: (images: string, alias: string, index: number | undefined, scale?: number) => void; + initLightBox: (images: string, alias: string, index?: number, scale?: number) => void; LightBoxComponent: () => JSX.Element; } -/** - * Custom Lightbox hook which handle rendering of the lightbox. - * @returns {UseLightBoxReturnType} - */ +// Memoized base component with props +const BaseLightBoxComponent = memo(function BaseLightBoxComponent({ + isLightBoxOpen, + slides, + slidesIndex, + imgScale, + aliasIndex, + onExit, +}: BaseLightBoxProps) { + logger.logTraceRender('components/common/use-lightbox (BaseLightBoxComponent)'); + // Store + const activeTrapGeoView = useUIActiveTrapGeoView(); + + // Callbacks + const handleLightboxExit = useCallback(() => { + onExit(); + + if (!activeTrapGeoView) return; + + setTimeout(() => { + const element = document.querySelector(`.returnLightboxFocusItem-${aliasIndex}`) as HTMLElement; + if (element) { + element.focus(); + element.classList.add('keyboard-focused'); + } + }, FOCUS_DELAY); + }, [activeTrapGeoView, aliasIndex, onExit]); + + if (!isLightBoxOpen) return ; + + return ; +}); + export function useLightBox(): UseLightBoxReturnType { - // Internal state + // State const [isLightBoxOpen, setIsLightBoxOpen] = useState(false); const [slides, setSlides] = useState([]); const [slidesIndex, setSlidesIndex] = useState(0); const [imgScale, setImgScale] = useState(); const [aliasIndex, setAliasIndex] = useState('0'); - // Store state - const activeTrapGeoView = useUIActiveTrapGeoView(); - - /** - * Initialize lightbox with state. - * @param {string} images images url formatted as string and joined with ';' identifier. - * @param {string} alias alt tag for the image. - * @param {number | undefined} index index of the image which is displayed. - */ - const initLightBox = (images: string, alias: string, index: number | undefined, scale?: number): void => { - setIsLightBoxOpen(true); - let slidesList = []; - if (images.startsWith('data:image/png;base64')) { - // special case - check if image is base64 and its a single image - slidesList = [{ src: images, alt: alias, downloadUrl: '' }]; - } else { - slidesList = images.split(';').map((item) => ({ src: item, alt: alias, downloadUrl: item })); + // Callbacks + const createSlidesList = useCallback((images: string, alias: string): LightBoxSlides[] => { + if (BASE64_IMAGE_PATTERN.test(images)) { + return [{ src: images, alt: alias, downloadUrl: '' }]; } - setSlides(slidesList); - setSlidesIndex(index ?? 0); - setImgScale(scale); - setAliasIndex(alias.split('_')[0]); - }; + return images.split(';').map((item) => ({ + src: item, + alt: alias, + downloadUrl: item, + })); + }, []); - /** - * Create LightBox Component based on lightbox is opened or not. - * @returns {JSX.Element} - */ - function LightBoxComponent(): JSX.Element { - // TODO: fix bug https://github.com/Canadian-Geospatial-Platform/geoview/issues/2553 - return isLightBoxOpen ? ( - { - setIsLightBoxOpen(false); - setSlides([]); - setSlidesIndex(0); + const handleExit = useCallback(() => { + setIsLightBoxOpen(false); + setSlides([]); + setSlidesIndex(0); + }, []); - // If keyboard navigation mode enable, focus to caller item (with timeout so keyboard-focused class can be applied) - if (activeTrapGeoView) { - setTimeout(() => { - const element = document.querySelector(`.returnLightboxFocusItem-${aliasIndex}`) as HTMLElement; - element?.focus(); - element?.classList.add('keyboard-focused'); - }, 250); - } - }} + const initLightBox = useCallback( + (images: string, alias: string, index?: number, scale?: number): void => { + setIsLightBoxOpen(true); + setSlides(createSlidesList(images, alias)); + setSlidesIndex(index ?? 0); + setImgScale(scale); + setAliasIndex(alias.split('_')[0]); + }, + [createSlidesList] + ); + + const LightBoxComponent = useCallback(() => { + return ( + - ) : ( - ); - } - return { initLightBox, LightBoxComponent }; + }, [isLightBoxOpen, slides, slidesIndex, imgScale, aliasIndex, handleExit]); + + return { + initLightBox, + LightBoxComponent, + }; } From 1801bdbcea79d3622a17640889da8784a050f58f Mon Sep 17 00:00:00 2001 From: Johann Levesque Date: Fri, 6 Dec 2024 09:57:01 -0500 Subject: [PATCH 2/2] Fix other component --- .../common/focus-trap-container.tsx | 65 ++++--- .../components/common/full-screen-dialog.tsx | 31 ++- .../src/core/components/common/layer-icon.tsx | 178 +++++++++++------- .../src/core/components/common/layer-list.tsx | 91 ++++----- .../src/core/components/common/layout.tsx | 32 +++- .../common/responsive-grid-layout.tsx | 2 +- .../components/common/responsive-grid.tsx | 116 +++++++----- .../common/use-footer-panel-height.tsx | 153 ++++++++------- .../layer-state.ts | 2 +- .../src/core/utils/useWhatChanged.ts | 14 ++ 10 files changed, 400 insertions(+), 284 deletions(-) diff --git a/packages/geoview-core/src/core/components/common/focus-trap-container.tsx b/packages/geoview-core/src/core/components/common/focus-trap-container.tsx index cf77069dcd6..9a9dba9c9e0 100644 --- a/packages/geoview-core/src/core/components/common/focus-trap-container.tsx +++ b/packages/geoview-core/src/core/components/common/focus-trap-container.tsx @@ -1,4 +1,4 @@ -import { ReactNode, useEffect } from 'react'; +import { memo, ReactNode, useCallback, useEffect, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; import { FocusTrap, Box, Button } from '@/ui'; import { logger } from '@/core/utils/logger'; @@ -6,35 +6,65 @@ import { useUIActiveFocusItem, useUIActiveTrapGeoView, useUIStoreActions } from import { TypeContainerBox } from '@/core/types/global-types'; import { CONTAINER_TYPE } from '@/core/utils/constant'; -interface FocusTrapContainerType { +interface FocusTrapContainerProps { children: ReactNode; id: string; containerType?: TypeContainerBox; open?: boolean; } +// Constants outside component to prevent recreating every render +const EXIT_BUTTON_STYLES = { + width: '95%', + margin: '10px auto', +} as const; + +const FOCUS_DELAY = 0; + /** * Focus trap container which will trap the focus when navigating through keyboard tab. * @param {TypeChildren} children dom elements wrapped in Focus trap. * @param {boolean} open enable and disabling of focus trap. * @returns {JSX.Element} */ -export function FocusTrapContainer({ children, open = false, id, containerType }: FocusTrapContainerType): JSX.Element { - // Log +// Memoizes entire component, preventing re-renders if props haven't changed +export const FocusTrapContainer = memo(function FocusTrapContainer({ + children, + open = false, + id, + containerType, +}: FocusTrapContainerProps): JSX.Element { logger.logTraceRender('component/common/FocusTrapContainer', containerType); + // Hooks const { t } = useTranslation(); - // get values from the store + // Store const { disableFocusTrap } = useUIStoreActions(); const activeTrapGeoView = useUIActiveTrapGeoView(); const focusItem = useUIActiveFocusItem(); - const handleClose = (): void => { + // Callbacks + const handleClose = useCallback((): void => { disableFocusTrap(id); - }; + }, [disableFocusTrap, id]); + + // Memoize + const isActive = useMemo(() => id === focusItem.activeElementId || open, [id, focusItem.activeElementId, open]); + + const showExitButton = useMemo( + () => containerType === CONTAINER_TYPE.FOOTER_BAR && activeTrapGeoView, + [containerType, activeTrapGeoView] + ); + + const exitButtonStyles = useMemo( + () => ({ + ...EXIT_BUTTON_STYLES, + display: activeTrapGeoView ? 'block' : 'none', + }), + [activeTrapGeoView] + ); - // #region REACT HOOKS // if keyboard navigation if turned off, remove trap settings useEffect(() => { // Log @@ -49,22 +79,15 @@ export function FocusTrapContainer({ children, open = false, id, containerType } logger.logTraceUseEffect('FOCUS-TRAP-ELEMENT - focusItem', focusItem); if (id === focusItem.activeElementId) { - setTimeout(() => document.getElementById(`${id}-exit-btn`)?.focus(), 0); + setTimeout(() => document.getElementById(`${id}-exit-btn`)?.focus(), FOCUS_DELAY); } }, [focusItem, id]); - // #endregion return ( - - - {containerType === CONTAINER_TYPE.FOOTER_BAR && activeTrapGeoView && ( - )} @@ -72,4 +95,4 @@ export function FocusTrapContainer({ children, open = false, id, containerType } ); -} +}); diff --git a/packages/geoview-core/src/core/components/common/full-screen-dialog.tsx b/packages/geoview-core/src/core/components/common/full-screen-dialog.tsx index 9eb0f3f7ddd..832a942c66a 100644 --- a/packages/geoview-core/src/core/components/common/full-screen-dialog.tsx +++ b/packages/geoview-core/src/core/components/common/full-screen-dialog.tsx @@ -1,5 +1,5 @@ +import { memo, ReactNode } from 'react'; import { DialogProps } from '@mui/material'; -import { ReactNode } from 'react'; import { CloseIcon, Dialog, DialogContent, IconButton } from '@/ui'; interface FullScreenDialogProps extends DialogProps { @@ -8,17 +8,32 @@ interface FullScreenDialogProps extends DialogProps { children: ReactNode; } -function FullScreenDialog({ open, onClose, children }: FullScreenDialogProps): JSX.Element { +// Constant styles to prevent recreation on each render +const DIALOG_CONTENT_STYLES = { + display: 'flex', + flexDirection: 'column', + alignItems: 'end', +} as const; + +const CLOSE_BUTTON_STYLES = { + marginBottom: '1.5rem', +} as const; + +// Memoizes entire component, preventing re-renders if props haven't changed +export const FullScreenDialog = memo(function FullScreenDialog({ + open, + onClose, + children, + ...dialogProps +}: FullScreenDialogProps): JSX.Element { return ( - - - + + + {children} ); -} - -export default FullScreenDialog; +}); diff --git a/packages/geoview-core/src/core/components/common/layer-icon.tsx b/packages/geoview-core/src/core/components/common/layer-icon.tsx index 1c007808f79..499f6acfe66 100644 --- a/packages/geoview-core/src/core/components/common/layer-icon.tsx +++ b/packages/geoview-core/src/core/components/common/layer-icon.tsx @@ -1,5 +1,7 @@ +import { memo, useCallback, useMemo } from 'react'; import { useTheme } from '@mui/material/styles'; import { Box, CircularProgressBase, ErrorIcon, GroupWorkOutlinedIcon, IconButton, BrowserNotSupportedIcon } from '@/ui'; + import { TypeLegendLayer } from '@/core/components/layers/types'; import { getSxClasses } from './layer-icon-style'; import { useIconLayerSet } from '@/core/stores/store-interface-and-intial-values/layer-state'; @@ -11,95 +13,129 @@ export interface TypeIconStackProps { onStackIconClick?: (e: React.KeyboardEvent) => void; } +interface LayerIconProps { + layer: TypeLegendLayer | LayerListEntry; +} + +// Constants outside component to prevent recreating every render +const LOADING_BOX_STYLES = { + padding: '5px', + marginRight: '10px', +} as const; + +const ICON_BUTTON_BASE_PROPS = { + color: 'primary' as const, + size: 'small' as const, + tabIndex: -1, + 'aria-hidden': true, +}; + /** * Icon Stack to represent layer icons * * @param {string} layerPath * @returns {JSX.Element} the icon stack item */ -function IconStack({ layerPath, onIconClick, onStackIconClick }: TypeIconStackProps): JSX.Element | null { +// Memoizes entire component, preventing re-renders if props haven't changed +const IconStack = memo(function IconStack({ layerPath, onIconClick, onStackIconClick }: TypeIconStackProps): JSX.Element | null { + // Hooks const theme = useTheme(); const sxClasses = getSxClasses(theme); + // Store const iconData = useIconLayerSet(layerPath); - const iconImage: string = iconData?.length > 0 ? iconData[0] : ''; - const iconImageStacked: string = iconData?.length > 1 ? iconData[1] : ''; - const numOfIcons: number | undefined = iconData?.length; - - const iconStackContent = (): JSX.Element | null => { - if (numOfIcons === 1) { - return ( -