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.
This commit is contained in:
Joe Boccanfuso 2026-01-14 16:50:24 -05:00 committed by GitHub
parent 612ced0ea1
commit eb6b814fc9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 29 additions and 11 deletions

View File

@ -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 (
<WindowLevelActionMenuContent
viewportId={viewportId}
align={align}
side={side}
onVisibilityChange={onVisibilityChange}
/>
);
}
@ -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}
>
<AllInOneMenu.ItemPanel>
{!is3DVolume && <Colorbar viewportId={viewportId} />}

View File

@ -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(
>
<WindowLevelActionMenu
viewportId={viewportIdToUse}
element={element}
align={align}
side={side}
onVisibilityChange={handleOpenChange}
/>
</PopoverContent>
</Popover>

View File

@ -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<Array<string>>([]);
// 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 => {