From ef43e12c12c5cc2fa412231f8aeb353bab08c296 Mon Sep 17 00:00:00 2001 From: Alex <94073946+Alex-NRCan@users.noreply.github.com> Date: Fri, 16 Feb 2024 07:43:25 -0500 Subject: [PATCH] Attempt to fix the panel event handling vs rendering (#1813) --- .../add-new-layer/add-new-layer.tsx | 2 +- packages/geoview-core/src/ui/panel/panel.tsx | 218 ++++++++++-------- .../src/ui/style/themeOptionsGenerator.ts | 4 +- 3 files changed, 126 insertions(+), 98 deletions(-) diff --git a/packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx b/packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx index 134a6f86218..a7056f26d48 100644 --- a/packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx +++ b/packages/geoview-core/src/core/components/layers/left-panel/add-new-layer/add-new-layer.tsx @@ -867,7 +867,7 @@ export function AddNewLayer(): JSX.Element { } setIsLoading(false); let message = ''; - switch(geoviewLayerInstance?.layerPhase) { + switch (geoviewLayerInstance?.layerPhase) { case 'loading': message = api.utilities.replaceParams([layerName], t('layers.layerAddedAndLoading')); break; diff --git a/packages/geoview-core/src/ui/panel/panel.tsx b/packages/geoview-core/src/ui/panel/panel.tsx index cf163714e4b..fd2f1497334 100644 --- a/packages/geoview-core/src/ui/panel/panel.tsx +++ b/packages/geoview-core/src/ui/panel/panel.tsx @@ -78,13 +78,19 @@ export function Panel(props: TypePanelAppProps): JSX.Element { * function that causes rerender when changing panel content */ const updateComponent = useCallback(() => { + // Log + logger.logTraceUseCallback('UI.PANEL - updateComponent'); + updatePanelContent((count) => count + 1); }, []); /** * Close the panel */ - const closePanel = (): void => { + const closePanel = useCallback((): void => { + // Log + logger.logTraceUseCallback('UI.PANEL - closePanel'); + const buttonElement = document.getElementById(mapId)?.querySelector(`#${button.id}`); if (buttonElement) { @@ -105,55 +111,60 @@ export function Panel(props: TypePanelAppProps): JSX.Element { // Closed onPanelClosed?.(); - }; - - const panelChangeContentListenerFunction = (payload: PayloadBaseClass) => { - // Log - logger.logTraceCoreAPIEvent('UI.PANEL - panelChangeContentListenerFunction', payload); - - if (payloadIsAPanelContent(payload)) { - // set focus on close button on panel content change - setTimeout(() => { - if (closeBtnRef && closeBtnRef.current) (closeBtnRef.current as HTMLElement).focus(); - }, 100); - - if (payload.buttonId === button.id!) { - updateComponent(); - } - } - }; + }, [mapId, button, onPanelClosed]); - // TODO: Check - This is probably not at the right place. This handler will be 'added' on every render and the single removal (in the useEffect unmount) likely isn't enough to remove them all - // listen to change panel content and rerender right after the panel has been created - api.event.on(EVENT_NAMES.PANEL.EVENT_PANEL_CHANGE_CONTENT, panelChangeContentListenerFunction, `${mapId}/${button.id!}`); + const panelChangeContentListenerFunction = useCallback( + (payload: PayloadBaseClass) => { + // Log + logger.logTraceCoreAPIEvent('UI.PANEL - panelChangeContentListenerFunction', payload); - const openPanelListenerFunction = (payload: PayloadBaseClass) => { - // Log - logger.logTraceCoreAPIEvent('UI.PANEL - openPanelListenerFunction', payload); - - if (payloadHasAButtonIdAndType(payload)) { - // set focus on close button on panel open - setPanelStatus(true); - - if (onPanelOpened) { - // Wait the transition period (+50 ms just to be sure of shenanigans) + if (payloadIsAPanelContent(payload)) { + // set focus on close button on panel content change setTimeout(() => { - onPanelOpened(); - }, theme.transitions.duration.standard + 50); + if (closeBtnRef && closeBtnRef.current) (closeBtnRef.current as HTMLElement).focus(); + }, 100); + + if (payload.buttonId === button.id!) { + updateComponent(); + } } + }, + [button, updateComponent] + ); - if (closeBtnRef && closeBtnRef.current) { - (closeBtnRef.current as HTMLElement).focus(); + const openPanelListenerFunction = useCallback( + (payload: PayloadBaseClass) => { + // Log + logger.logTraceCoreAPIEvent('UI.PANEL - openPanelListenerFunction', payload); + + if (payloadHasAButtonIdAndType(payload)) { + // set focus on close button on panel open + setPanelStatus(true); + + if (onPanelOpened) { + // Wait the transition period (+50 ms just to be sure of shenanigans) + setTimeout(() => { + onPanelOpened(); + }, theme.transitions.duration.standard + 50); + } + + if (closeBtnRef && closeBtnRef.current) { + (closeBtnRef.current as HTMLElement).focus(); + } } - } - }; + }, + [onPanelOpened, theme] + ); - const closePanelListenerFunction = (payload: PayloadBaseClass) => { - // Log - logger.logTraceCoreAPIEvent('UI.PANEL - closePanelListenerFunction', payload); + const closePanelListenerFunction = useCallback( + (payload: PayloadBaseClass) => { + // Log + logger.logTraceCoreAPIEvent('UI.PANEL - closePanelListenerFunction', payload); - if (payloadHasAButtonIdAndType(payload)) closePanel(); - }; + if (payloadHasAButtonIdAndType(payload)) closePanel(); + }, + [closePanel] + ); const closeAllPanelListenerFunction = (payload: PayloadBaseClass) => { // Log @@ -162,55 +173,61 @@ export function Panel(props: TypePanelAppProps): JSX.Element { if (payloadHasAButtonIdAndType(payload)) setPanelStatus(false); }; - const panelAddActionListenerFunction = (payload: PayloadBaseClass) => { - // Log - logger.logTraceCoreAPIEvent('UI.PANEL - panelAddActionListenerFunction', payload); - - if (payloadIsAPanelAction(payload)) { - if (payload.buttonId === button.id!) { - const { actionButton } = payload; - - setActionButtons((prev) => [ - ...prev, - (actionButton.action)} - size="small" - > - {typeof actionButton.children === 'string' ? ( - - ) : ( - (actionButton.children as ReactNode) - )} - , - ]); + const panelAddActionListenerFunction = useCallback( + (payload: PayloadBaseClass) => { + // Log + logger.logTraceCoreAPIEvent('UI.PANEL - panelAddActionListenerFunction', payload); + + if (payloadIsAPanelAction(payload)) { + if (payload.buttonId === button.id!) { + const { actionButton } = payload; + + setActionButtons((prev) => [ + ...prev, + (actionButton.action)} + size="small" + > + {typeof actionButton.children === 'string' ? ( + + ) : ( + (actionButton.children as ReactNode) + )} + , + ]); + } } - } - }; + }, + [button.id] + ); - const panelRemoveActionListenerFunction = (payload: PayloadBaseClass) => { - // Log - logger.logTraceCoreAPIEvent('UI.PANEL - panelRemoveActionListenerFunction', payload); - - if (payloadIsAPanelAction(payload)) { - if (payload.buttonId === button.id!) { - setActionButtons((list) => - list.filter((item) => { - return item.props.id !== payload.actionButton.actionButtonId; - }) - ); + const panelRemoveActionListenerFunction = useCallback( + (payload: PayloadBaseClass) => { + // Log + logger.logTraceCoreAPIEvent('UI.PANEL - panelRemoveActionListenerFunction', payload); + + if (payloadIsAPanelAction(payload)) { + if (payload.buttonId === button.id!) { + setActionButtons((list) => + list.filter((item) => { + return item.props.id !== payload.actionButton.actionButtonId; + }) + ); + } } - } - }; + }, + [button] + ); useEffect(() => { // Log @@ -237,16 +254,27 @@ export function Panel(props: TypePanelAppProps): JSX.Element { // listen to remove action button event api.event.on(EVENT_NAMES.PANEL.EVENT_PANEL_REMOVE_ACTION, panelRemoveActionListenerFunction, `${mapId}/${button.id!}`); + // listen to change panel content and rerender right after the panel has been created + api.event.on(EVENT_NAMES.PANEL.EVENT_PANEL_CHANGE_CONTENT, panelChangeContentListenerFunction, `${mapId}/${button.id!}`); + return () => { - api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_OPEN, `${mapId}/${button.id!}`, openPanelListenerFunction); - api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_CLOSE, `${mapId}/${button.id!}`, closePanelListenerFunction); - api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_CLOSE_ALL, `${mapId}/${button.id!}`, closeAllPanelListenerFunction); - api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_ADD_ACTION, `${mapId}/${button.id!}`, panelAddActionListenerFunction); - api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_REMOVE_ACTION, `${mapId}/${button.id!}`, panelRemoveActionListenerFunction); api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_CHANGE_CONTENT, `${mapId}/${button.id!}`, panelChangeContentListenerFunction); + api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_REMOVE_ACTION, `${mapId}/${button.id!}`, panelRemoveActionListenerFunction); + api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_ADD_ACTION, `${mapId}/${button.id!}`, panelAddActionListenerFunction); + api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_CLOSE_ALL, `${mapId}/${button.id!}`, closeAllPanelListenerFunction); + api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_CLOSE, `${mapId}/${button.id!}`, closePanelListenerFunction); + api.event.off(EVENT_NAMES.PANEL.EVENT_PANEL_OPEN, `${mapId}/${button.id!}`, openPanelListenerFunction); }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); + }, [ + mapId, + button.id, + panel, + openPanelListenerFunction, + closePanelListenerFunction, + panelAddActionListenerFunction, + panelChangeContentListenerFunction, + panelRemoveActionListenerFunction, + ]); useEffect(() => { // Log diff --git a/packages/geoview-core/src/ui/style/themeOptionsGenerator.ts b/packages/geoview-core/src/ui/style/themeOptionsGenerator.ts index 732bc1447dd..b1edd93738e 100644 --- a/packages/geoview-core/src/ui/style/themeOptionsGenerator.ts +++ b/packages/geoview-core/src/ui/style/themeOptionsGenerator.ts @@ -2,7 +2,7 @@ import { ThemeOptions } from '@mui/material'; import { IGeoViewColors } from './types'; import { font, headingStyles, opacity, geoViewColors as defaultGeoViewColors, geoViewFontSizes } from './default'; -export const generateThemeOptions = function (geoViewColors: IGeoViewColors = defaultGeoViewColors): ThemeOptions { +export const generateThemeOptions = (geoViewColors: IGeoViewColors = defaultGeoViewColors): ThemeOptions => { const themeOptions: ThemeOptions = { palette: { geoViewColor: geoViewColors, @@ -59,7 +59,7 @@ export const generateThemeOptions = function (geoViewColors: IGeoViewColors = de divider: 'rgba(0, 0, 0, 0.12)', background: { paper: geoViewColors.bgColor.light[600], - default: geoViewColors.bgColor.light[500] + default: geoViewColors.bgColor.light[500], }, action: { active: geoViewColors.primary.main,