From eb6b814fc9c35feb6aa5f4378632b53202b0d8a3 Mon Sep 17 00:00:00 2001 From: Joe Boccanfuso <109477394+jbocce@users.noreply.github.com> Date: Wed, 14 Jan 2026 16:50:24 -0500 Subject: [PATCH] fix(window-level-action-menu): Window level menu no longer needs two clicks to open after it is used (#5711) The window level menu's logic for when it is opened and closed now also runs when the AllInOneMenu triggers the open or close. Using refs to store the latest function references in the AllInOneMenu component to avoid triggering a useEffect when the functions change. Removed an unused element prop from the WindowLevelActionMenu component. --- .../WindowLevelActionMenu.tsx | 8 ++++-- .../WindowLevelActionMenuWrapper.tsx | 4 +-- .../src/components/AllInOneMenu/Menu.tsx | 28 +++++++++++++++---- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenu.tsx b/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenu.tsx index c844df39b..6946127f6 100644 --- a/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenu.tsx +++ b/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenu.tsx @@ -11,22 +11,23 @@ import i18n from 'i18next'; export type WindowLevelActionMenuProps = { viewportId: string; - element?: HTMLElement; align?: 'start' | 'end' | 'center'; side?: 'top' | 'bottom' | 'left' | 'right'; + onVisibilityChange?: (isVisible: boolean) => void; }; export function WindowLevelActionMenu({ viewportId, - element, align, side, + onVisibilityChange, }: WindowLevelActionMenuProps): ReactElement { return ( ); } @@ -35,10 +36,12 @@ export function WindowLevelActionMenuContent({ viewportId, align, side, + onVisibilityChange, }: { viewportId: string; align?: string; side?: string; + onVisibilityChange?: (isVisible: boolean) => void; }): ReactElement { const { t } = useTranslation('WindowLevelActionMenu'); // Use a stable key for the menu to avoid infinite re-renders @@ -60,6 +63,7 @@ export function WindowLevelActionMenuContent({ align={align} side={side} backLabel={i18n.t('WindowLevelActionMenu:Back to Display Options')} + onVisibilityChange={onVisibilityChange} > {!is3DVolume && } diff --git a/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenuWrapper.tsx b/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenuWrapper.tsx index b1ad429de..669876a5a 100644 --- a/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenuWrapper.tsx +++ b/extensions/cornerstone/src/components/WindowLevelActionMenu/WindowLevelActionMenuWrapper.tsx @@ -16,7 +16,6 @@ import { useViewportRendering } from '../../hooks'; export function WindowLevelActionMenuWrapper( props: withAppTypes<{ viewportId: string; - element?: HTMLElement; location?: number; isOpen?: boolean; onOpen?: () => void; @@ -28,7 +27,6 @@ export function WindowLevelActionMenuWrapper( ): ReactNode { const { viewportId, - element, location, isOpen = false, onOpen, @@ -123,9 +121,9 @@ export function WindowLevelActionMenuWrapper( > diff --git a/platform/ui-next/src/components/AllInOneMenu/Menu.tsx b/platform/ui-next/src/components/AllInOneMenu/Menu.tsx index 8fb497414..a2e75caf5 100644 --- a/platform/ui-next/src/components/AllInOneMenu/Menu.tsx +++ b/platform/ui-next/src/components/AllInOneMenu/Menu.tsx @@ -1,4 +1,4 @@ -import React, { createContext, ReactNode, useCallback, useEffect, useState } from 'react'; +import React, { createContext, ReactNode, useCallback, useEffect, useRef, useState } from 'react'; import DividerItem from './DividerItem'; import PanelSelector from './PanelSelector'; @@ -108,6 +108,20 @@ const Menu = (props: MenuProps) => { ]); const [itemPanelLabels, setItemPanelLabels] = useState>([]); + // Store latest function references in refs so we can use them in effects + // without triggering re-runs when they change + const onVisibilityChangeRef = useRef(onVisibilityChange); + const preventHideMenuRef = useRef(preventHideMenu); + + // Keep refs up to date + useEffect(() => { + onVisibilityChangeRef.current = onVisibilityChange; + }, [onVisibilityChange]); + + useEffect(() => { + preventHideMenuRef.current = preventHideMenu; + }, [preventHideMenu]); + // If the props change for the this top level menu then we have to update the menu path // because the props to be rendered are maintained in the state. useEffect(() => { @@ -118,23 +132,25 @@ const Menu = (props: MenuProps) => { }, [activePanelIndex, props]); const hideMenu = useCallback(() => { - if (preventHideMenu) { + if (preventHideMenuRef.current) { return; } setMenuPath(path => [path[0]]); setItemPanelLabels([]); setIsMenuVisible(false); - onVisibilityChange?.(false); - }, [preventHideMenu, onVisibilityChange]); + onVisibilityChangeRef.current?.(false); + }, []); + // Only run this effect when isVisible changes, not when functions change. + // Note that hideMenu is stable with no dependencies and thus will not trigger the useEffect to run. useEffect(() => { if (isVisible) { setIsMenuVisible(isVisible); - onVisibilityChange?.(isVisible); + onVisibilityChangeRef.current?.(isVisible); } else { hideMenu(); } - }, [hideMenu, isVisible, onVisibilityChange]); + }, [isVisible, hideMenu]); const showSubMenu = useCallback((subMenuProps: MenuProps) => { setMenuPath(path => {