Skip to content

Commit

Permalink
Fix: #5019: Dialogs escaped in wrong order
Browse files Browse the repository at this point in the history
  • Loading branch information
melloware committed Jan 9, 2024
1 parent 9ec4c2b commit c219764
Show file tree
Hide file tree
Showing 18 changed files with 152 additions and 123 deletions.
12 changes: 10 additions & 2 deletions components/lib/cascadeselect/CascadeSelect.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as React from 'react';
import PrimeReact, { PrimeReactContext } from '../api/Api';
import { useHandleStyle } from '../componentbase/ComponentBase';
import { CSSTransition } from '../csstransition/CSSTransition';
import { useMountEffect, useOnEscapeKey, useOverlayListener, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks';
import { ESC_KEY_HANDLING_PRIORITIES, useDisplayOrder, useGlobalOnEscapeKey, useMountEffect, useOverlayListener, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks';
import { ChevronDownIcon } from '../icons/chevrondown';
import { SpinnerIcon } from '../icons/spinner';
import { OverlayService } from '../overlayservice/OverlayService';
Expand Down Expand Up @@ -49,7 +49,15 @@ export const CascadeSelect = React.memo(
when: overlayVisibleState
});

useOnEscapeKey(overlayRef, overlayVisibleState, () => hide());
const cascadeSelectOverlayDisplayOrder = useDisplayOrder('cascade-select', overlayVisibleState);

useGlobalOnEscapeKey({
callback: () => {
hide();
},
when: overlayVisibleState,
priority: [ESC_KEY_HANDLING_PRIORITIES.CASCADE_SELECT, cascadeSelectOverlayDisplayOrder]
});

const onOptionSelect = (event) => {
if (props.onChange) {
Expand Down
1 change: 1 addition & 0 deletions components/lib/cascadeselect/cascadeselect.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
*/
import * as React from 'react';
import { CSSTransitionProps as ReactCSSTransitionProps } from 'react-transition-group/CSSTransition';
import { APIOptions } from '../api/api';
import { ComponentHooks } from '../componentbase/componentbase';
import { CSSTransitionProps } from '../csstransition';
import { PassThroughOptions } from '../passthrough';
Expand Down
1 change: 1 addition & 0 deletions components/lib/confirmdialog/confirmdialog.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*
*/
import * as React from 'react';
import { CSSTransitionProps as ReactCSSTransitionProps } from 'react-transition-group/CSSTransition';
import { ButtonPassThroughOptions } from '../button/button';
import { ComponentHooks } from '../componentbase/componentbase';
import { DialogProps } from '../dialog';
Expand Down
14 changes: 10 additions & 4 deletions components/lib/confirmpopup/ConfirmPopup.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import * as React from 'react';
import { useOnEscapeKey } from '../../lib/hooks/Hooks';
import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api';
import { Button } from '../button/Button';
import { useHandleStyle } from '../componentbase/ComponentBase';
import { CSSTransition } from '../csstransition/CSSTransition';
import { useOverlayListener, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks';
import { ESC_KEY_HANDLING_PRIORITIES, useDisplayOrder, useGlobalOnEscapeKey, useOverlayListener, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks';
import { OverlayService } from '../overlayservice/OverlayService';
import { Portal } from '../portal/Portal';
import { DomHandler, IconUtils, ObjectUtils, ZIndexUtils, classNames, mergeProps } from '../utils/Utils';
Expand Down Expand Up @@ -57,9 +56,16 @@ export const ConfirmPopup = React.memo(

const acceptLabel = getPropValue('acceptLabel') || localeOption('accept');
const rejectLabel = getPropValue('rejectLabel') || localeOption('reject');
const displayOrder = useDisplayOrder('dialog', visibleState);

useOnEscapeKey(overlayRef, props.dismissable && props.closeOnEscape, (event) => {
hide('hide');
useGlobalOnEscapeKey({
callback: () => {
if (props.dismissable && props.closeOnEscape) {
hide('hide');
}
},
when: visibleState,
priority: [ESC_KEY_HANDLING_PRIORITIES.DIALOG, displayOrder]
});

const [bindOverlayListener, unbindOverlayListener] = useOverlayListener({
Expand Down
35 changes: 16 additions & 19 deletions components/lib/dialog/Dialog.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as React from 'react';
import { useOnEscapeKey } from '../../lib/hooks/Hooks';
import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api';
import { useHandleStyle } from '../componentbase/ComponentBase';
import { CSSTransition } from '../csstransition/CSSTransition';
import { useEventListener, useMountEffect, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks';
import { ESC_KEY_HANDLING_PRIORITIES, useDisplayOrder, useEventListener, useGlobalOnEscapeKey, useMountEffect, useUnmountEffect, useUpdateEffect } from '../hooks/Hooks';
import { TimesIcon } from '../icons/times';
import { WindowMaximizeIcon } from '../icons/windowmaximize';
import { WindowMinimizeIcon } from '../icons/windowminimize';
Expand Down Expand Up @@ -37,6 +36,7 @@ export const Dialog = React.forwardRef((inProps, ref) => {
const focusElementOnHide = React.useRef(null);
const maximized = props.onMaximize ? props.maximized : maximizedState;
const shouldBlockScroll = visibleState && (props.blockScroll || (props.maximizable && maximized));
const displayOrder = useDisplayOrder('dialog', visibleState);

const { ptm, cx, sx, isUnstyled } = DialogBase.setMetaData({
props,
Expand All @@ -50,18 +50,14 @@ export const Dialog = React.forwardRef((inProps, ref) => {

useHandleStyle(DialogBase.css.styles, isUnstyled, { name: 'dialog' });

useOnEscapeKey(maskRef, props.closable && props.closeOnEscape, (event) => {
const currentTarget = event.currentTarget;

if (!currentTarget || !currentTarget.primeDialogParams) {
return;
}

const params = currentTarget.primeDialogParams;
const paramLength = params.length;

onClose(event);
params.splice(paramLength - 1, 1);
useGlobalOnEscapeKey({
callback: (event) => {
if (props.closable && props.closeOnEscape) {
onClose(event);
}
},
when: visibleState,
priority: [ESC_KEY_HANDLING_PRIORITIES.DIALOG, displayOrder]
});

const [bindDocumentKeyDownListener, unbindDocumentKeyDownListener] = useEventListener({ type: 'keydown', listener: (event) => onKeyDown(event) });
Expand Down Expand Up @@ -319,12 +315,10 @@ export const Dialog = React.forwardRef((inProps, ref) => {

const enableDocumentSettings = () => {
bindGlobalListeners();
updateGlobalDialogsRegistry(true);
};

const disableDocumentSettings = () => {
unbindGlobalListeners();
updateGlobalDialogsRegistry(false);
};

const updateScrollBlocker = () => {
Expand All @@ -339,7 +333,7 @@ export const Dialog = React.forwardRef((inProps, ref) => {
};

const updateGlobalDialogsRegistry = (isMounted) => {
// Update current dialog info in global registry if it is mounted :
// Update current dialog info in global registry if it is mounted and visible:
if (isMounted && visibleState) {
const newParam = { id: idState, hasBlockScroll: shouldBlockScroll };

Expand All @@ -356,7 +350,7 @@ export const Dialog = React.forwardRef((inProps, ref) => {
document.primeDialogParams = document.primeDialogParams.toSpliced(currentDialogIndexInRegistry, 1, newParam);
}
}
// Or remove it from global registry if unmounted:
// Or remove it from global registry if unmounted or invisible:
else {
document.primeDialogParams = document.primeDialogParams && document.primeDialogParams.filter((param) => param.id !== idState);
}
Expand Down Expand Up @@ -411,6 +405,8 @@ export const Dialog = React.forwardRef((inProps, ref) => {
};

useMountEffect(() => {
updateGlobalDialogsRegistry(true);

if (props.visible) {
setMaskVisibleState(true);
}
Expand Down Expand Up @@ -452,10 +448,11 @@ export const Dialog = React.forwardRef((inProps, ref) => {

useUpdateEffect(() => {
updateGlobalDialogsRegistry(true);
}, [shouldBlockScroll]);
}, [shouldBlockScroll, visibleState]);

useUnmountEffect(() => {
disableDocumentSettings();
updateGlobalDialogsRegistry(false);
DomHandler.removeInlineStyle(styleElement.current);
ZIndexUtils.clear(maskRef.current);
});
Expand Down
39 changes: 21 additions & 18 deletions components/lib/hooks/Hooks.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import { useClickOutside } from './useClickOutside';
import { useCounter } from './useCounter';
import { useDebounce } from './useDebounce';
import { useDisplayOrder } from './useDisplayOrder';
import { useEventListener } from './useEventListener';
import { useFavicon } from './useFavicon';
import { ESC_KEY_HANDLING_PRIORITIES, useGlobalOnEscapeKey } from './useGlobalOnEscapeKey';
import { useIntersectionObserver } from './useIntersectionObserver';
import { useInterval } from './useInterval';
import { useMatchMedia } from './useMatchMedia';
Expand All @@ -14,34 +16,35 @@ import { useOverlayScrollListener } from './useOverlayScrollListener';
import { usePrevious } from './usePrevious';
import { useResizeListener } from './useResizeListener';
import { useLocalStorage, useSessionStorage, useStorage } from './useStorage';
import { useStyle } from './useStyle';
import { useTimeout } from './useTimeout';
import { useUnmountEffect } from './useUnmountEffect';
import { useUpdateEffect } from './useUpdateEffect';
import { useStyle } from './useStyle';
import { useOnEscapeKey } from './useOnEscapeKey';

export {
usePrevious,
useMountEffect,
useUpdateEffect,
useUnmountEffect,
ESC_KEY_HANDLING_PRIORITIES,
useClickOutside,
useCounter,
useDebounce,
useDisplayOrder,
useEventListener,
useFavicon,
useGlobalOnEscapeKey,
useIntersectionObserver,
useInterval,
useLocalStorage,
useMatchMedia,
useMountEffect,
useMouse,
useMove,
useOverlayListener,
useOverlayScrollListener,
usePrevious,
useResizeListener,
useIntersectionObserver,
useInterval,
useSessionStorage,
useStorage,
useStyle,
useLocalStorage,
useSessionStorage,
useTimeout,
useCounter,
useFavicon,
useMouse,
useMove,
useClickOutside,
useDebounce,
useMatchMedia,
useOnEscapeKey
useUnmountEffect,
useUpdateEffect
};
12 changes: 8 additions & 4 deletions components/lib/hooks/hooks.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,12 @@ export declare function useClickOutside(ref: React.RefObject<Element>, callback:
*/
export declare function useMatchMedia(query: string, when?: boolean): boolean;
/**
* Custom hook to use detect escape button click.
* @param {React.RefObject<Element>} ref - The ref of the element to detect escape button click.
* @param {*} callback - The callback to run when escape button clicked.
* Custom hook to use detect global escape button click.
*/
export declare function useOnEscapeKey(ref: React.RefObject<Element>, callback: any): void;
export declare function useGlobalOnEscapeKey(props: { callback: (event: KeyboardEvent) => void; when: boolean; priority: [number, number] }): void;
/**
* Custom hook to use display order of component of one and the same group
* @param {string} group
* @param {boolean} [isVisible]
*/
export declare function useDisplayOrder(group: string, isVisible?: boolean): number | undefined;
36 changes: 0 additions & 36 deletions components/lib/hooks/useOnEscapeKey.js

This file was deleted.

23 changes: 16 additions & 7 deletions components/lib/image/Image.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as React from 'react';
import { useOnEscapeKey } from '../../lib/hooks/Hooks';
import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api';
import { useHandleStyle } from '../componentbase/ComponentBase';
import { CSSTransition } from '../csstransition/CSSTransition';
import { useUnmountEffect } from '../hooks/Hooks';
import { ESC_KEY_HANDLING_PRIORITIES, useGlobalOnEscapeKey, useUnmountEffect } from '../hooks/Hooks';
import { DownloadIcon } from '../icons/download';
import { EyeIcon } from '../icons/eye';
import { RefreshIcon } from '../icons/refresh';
Expand Down Expand Up @@ -32,11 +31,6 @@ export const Image = React.memo(
const previewButton = React.useRef(null);
const zoomOutDisabled = scaleState <= 0.5;
const zoomInDisabled = scaleState >= 1.5;

useOnEscapeKey(maskRef, props.closeOnEscape, () => {
hide();
});

const { ptm, cx, sx, isUnstyled } = ImageBase.setMetaData({
props,
state: {
Expand All @@ -47,6 +41,21 @@ export const Image = React.memo(
}
});

useGlobalOnEscapeKey({
callback: () => {
if (props.closeOnEscape) {
hide();
}
},
when: maskVisibleState,
priority: [
ESC_KEY_HANDLING_PRIORITIES.IMAGE,
// Assume that there could be only one image mask activated, so it's safe
// to provide one and the same priority all the time:
0
]
});

useHandleStyle(ImageBase.css.styles, isUnstyled, { name: 'image' });

const show = () => {
Expand Down
12 changes: 8 additions & 4 deletions components/lib/menu/Menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,11 @@ import * as React from 'react';
import PrimeReact, { PrimeReactContext } from '../api/Api';
import { useHandleStyle } from '../componentbase/ComponentBase';
import { CSSTransition } from '../csstransition/CSSTransition';
import { useMountEffect, useOverlayListener, useUnmountEffect } from '../hooks/Hooks';
import { ESC_KEY_HANDLING_PRIORITIES, useDisplayOrder, useGlobalOnEscapeKey, useMountEffect, useOverlayListener, useUnmountEffect } from '../hooks/Hooks';
import { OverlayService } from '../overlayservice/OverlayService';
import { Portal } from '../portal/Portal';
import { DomHandler, IconUtils, ObjectUtils, UniqueComponentId, ZIndexUtils, classNames, mergeProps } from '../utils/Utils';
import { MenuBase } from './MenuBase';
import { useOnEscapeKey } from '../../lib/hooks/Hooks';

export const Menu = React.memo(
React.forwardRef((inProps, ref) => {
Expand All @@ -31,9 +30,14 @@ export const Menu = React.memo(
const menuRef = React.useRef(null);
const listRef = React.useRef(null);
const targetRef = React.useRef(null);
const popupMenuDisplayOrder = useDisplayOrder('menu', !!(visibleState && props.popup));

useOnEscapeKey(targetRef, props.popup && props.closeOnEscape, (event) => {
hide(event);
useGlobalOnEscapeKey({
callback: (event) => {
hide(event);
},
when: visibleState && props.popup && props.closeOnEscape,
priority: [ESC_KEY_HANDLING_PRIORITIES.MENU, popupMenuDisplayOrder]
});

const [bindOverlayListener, unbindOverlayListener] = useOverlayListener({
Expand Down
13 changes: 9 additions & 4 deletions components/lib/overlaypanel/OverlayPanel.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
import * as React from 'react';
import { useOnEscapeKey } from '../../lib/hooks/Hooks';
import PrimeReact, { PrimeReactContext, localeOption } from '../api/Api';
import { useHandleStyle } from '../componentbase/ComponentBase';
import { CSSTransition } from '../csstransition/CSSTransition';
import { useMountEffect, useOverlayListener, useUnmountEffect } from '../hooks/Hooks';
import { ESC_KEY_HANDLING_PRIORITIES, useDisplayOrder, useGlobalOnEscapeKey, useMountEffect, useOverlayListener, useUnmountEffect } from '../hooks/Hooks';
import { TimesIcon } from '../icons/times';
import { OverlayService } from '../overlayservice/OverlayService';
import { Portal } from '../portal/Portal';
Expand Down Expand Up @@ -54,8 +53,14 @@ export const OverlayPanel = React.forwardRef((inProps, ref) => {
when: visibleState
});

useOnEscapeKey(overlayEventListener, props.closeOnEscape, () => {
hide();
const overlayPanelDisplayOrder = useDisplayOrder('overlay-panel', visibleState);

useGlobalOnEscapeKey({
callback: () => {
hide();
},
when: visibleState && props.closeOnEscape,
priority: [ESC_KEY_HANDLING_PRIORITIES.OVERLAY_PANEL, overlayPanelDisplayOrder]
});

const isOutsideClicked = (target) => {
Expand Down
Loading

0 comments on commit c219764

Please sign in to comment.