From 1c97331092471c9669495c6843880ebfee015396 Mon Sep 17 00:00:00 2001 From: akshayaqburst <123446355+akshayaqburst@users.noreply.github.com> Date: Wed, 9 Aug 2023 17:45:30 +0530 Subject: [PATCH] Fix #4704: Image preview popup not closing on escape button click (#4705) * Fix primfaces#4704 Image preview popup not closing on escape button click * Fix primfaces#4704 Resolved PR comments to run prettier * Fix primfaces#4704 Added useOnEscape hook in Dialog to resolve PR comments * Fix primfaces#4704 Reused the hook on dialog and sidebar components to resolve pr comments * Fix primfaces#4704 Added params code to dialog escape event click to resolve pr comments * renamed escape hook to resolve pr comments * fix: #4750, Added closeOnEscape to Overlay panel --------- Co-authored-by: akshayantony55 --- components/lib/dialog/Dialog.js | 21 ++++++++--- components/lib/hooks/Hooks.js | 4 ++- components/lib/hooks/hooks.d.ts | 6 ++++ components/lib/hooks/useOnEscapeKey.js | 36 +++++++++++++++++++ components/lib/image/Image.js | 4 +++ components/lib/image/ImageBase.js | 3 +- components/lib/image/image.d.ts | 5 +++ components/lib/overlaypanel/OverlayPanel.js | 5 +++ .../lib/overlaypanel/OverlayPanelBase.js | 3 +- components/lib/overlaypanel/overlaypanel.d.ts | 5 +++ components/lib/sidebar/Sidebar.js | 17 +++------ 11 files changed, 88 insertions(+), 21 deletions(-) create mode 100644 components/lib/hooks/useOnEscapeKey.js diff --git a/components/lib/dialog/Dialog.js b/components/lib/dialog/Dialog.js index c6c6574bf5..23de891576 100644 --- a/components/lib/dialog/Dialog.js +++ b/components/lib/dialog/Dialog.js @@ -10,6 +10,7 @@ import { Portal } from '../portal/Portal'; import { Ripple } from '../ripple/Ripple'; import { DomHandler, IconUtils, ObjectUtils, UniqueComponentId, ZIndexUtils, mergeProps } from '../utils/Utils'; import { DialogBase } from './DialogBase'; +import { useOnEscapeKey } from '../../lib/hooks/Hooks'; export const Dialog = React.forwardRef((inProps, ref) => { const context = React.useContext(PrimeReactContext); @@ -47,6 +48,20 @@ 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); + }); + const [bindDocumentKeyDownListener, unbindDocumentKeyDownListener] = useEventListener({ type: 'keydown', listener: (event) => onKeyDown(event) }); const [bindDocumentResizeListener, unbindDocumentResizeListener] = useEventListener({ type: 'mousemove', target: () => window.document, listener: (event) => onResize(event) }); const [bindDocumentResizeEndListener, unbindDocumentResizEndListener] = useEventListener({ type: 'mouseup', target: () => window.document, listener: (event) => onResizeEnd(event) }); @@ -111,11 +126,7 @@ export const Dialog = React.forwardRef((inProps, ref) => { const dialog = document.getElementById(dialogId); - if (props.closable && props.closeOnEscape && event.key === 'Escape') { - onClose(event); - event.stopImmediatePropagation(); - params.splice(paramLength - 1, 1); - } else if (event.key === 'Tab') { + if (event.key === 'Tab') { event.preventDefault(); const focusableElements = DomHandler.getFocusableElements(dialog); diff --git a/components/lib/hooks/Hooks.js b/components/lib/hooks/Hooks.js index 28e6b2875c..857a3ca542 100644 --- a/components/lib/hooks/Hooks.js +++ b/components/lib/hooks/Hooks.js @@ -18,6 +18,7 @@ import { useTimeout } from './useTimeout'; import { useUnmountEffect } from './useUnmountEffect'; import { useUpdateEffect } from './useUpdateEffect'; import { useStyle } from './useStyle'; +import { useOnEscapeKey } from './useOnEscapeKey'; export { usePrevious, @@ -41,5 +42,6 @@ export { useMove, useClickOutside, useDebounce, - useMatchMedia + useMatchMedia, + useOnEscapeKey }; diff --git a/components/lib/hooks/hooks.d.ts b/components/lib/hooks/hooks.d.ts index 84cc4c2a08..234b3df12a 100644 --- a/components/lib/hooks/hooks.d.ts +++ b/components/lib/hooks/hooks.d.ts @@ -291,3 +291,9 @@ export declare function useClickOutside(ref: React.RefObject, callback: * @param {boolean} when - Whether to listen to the event or not. */ export declare function useMatchMedia(query: string, when?: boolean): boolean; +/** + * Custom hook to use detect escape button click. + * @param {React.RefObject} ref - The ref of the element to detect escape button click. + * @param {*} callback - The callback to run when escape button clicked. + */ +export declare function useOnEscapeKey(ref: React.RefObject, callback: any): void; diff --git a/components/lib/hooks/useOnEscapeKey.js b/components/lib/hooks/useOnEscapeKey.js new file mode 100644 index 0000000000..9f5cd2d874 --- /dev/null +++ b/components/lib/hooks/useOnEscapeKey.js @@ -0,0 +1,36 @@ +import * as React from 'react'; +import { useEventListener } from './useEventListener'; + +export const useOnEscapeKey = (ref, condition, callback) => { + const handleEsc = (event) => { + if (event.key === 'Esc' || event.key === 'Escape') { + event.stopImmediatePropagation(); + callback(event); + } + + return; + }; + + const [bindKeyDownListener, unbindKeyDownListener] = useEventListener({ + type: 'keydown', + listener: handleEsc + }); + + React.useEffect(() => { + if (!condition) { + return; + } + + if (!ref.current) { + return; + } + + bindKeyDownListener(); + + return () => { + unbindKeyDownListener(); + }; + }); + + return [ref, callback]; +}; diff --git a/components/lib/image/Image.js b/components/lib/image/Image.js index 1b7e2ea8cd..cebc1fbbfe 100644 --- a/components/lib/image/Image.js +++ b/components/lib/image/Image.js @@ -13,6 +13,7 @@ import { UndoIcon } from '../icons/undo'; import { Portal } from '../portal/Portal'; import { DomHandler, IconUtils, ObjectUtils, ZIndexUtils, classNames, mergeProps } from '../utils/Utils'; import { ImageBase } from './ImageBase'; +import { useOnEscapeKey } from '../../lib/hooks/Hooks'; export const Image = React.memo( React.forwardRef((inProps, ref) => { @@ -29,6 +30,9 @@ export const Image = React.memo( const previewRef = React.useRef(null); const previewClick = React.useRef(false); + useOnEscapeKey(maskRef, props.closeOnEscape, () => { + hide(); + }); const { ptm, cx, sx, isUnstyled } = ImageBase.setMetaData({ props, state: { diff --git a/components/lib/image/ImageBase.js b/components/lib/image/ImageBase.js index 393a77d4c4..cf5b816325 100644 --- a/components/lib/image/ImageBase.js +++ b/components/lib/image/ImageBase.js @@ -140,7 +140,8 @@ export const ImageBase = ComponentBase.extend({ zoomInIcon: null, zoomOutIcon: null, zoomSrc: null, - children: undefined + children: undefined, + closeOnEscape: true }, css: { classes, diff --git a/components/lib/image/image.d.ts b/components/lib/image/image.d.ts index 11cbc732fd..6f42495573 100644 --- a/components/lib/image/image.d.ts +++ b/components/lib/image/image.d.ts @@ -247,6 +247,11 @@ export interface ImageProps extends Omit { const context = React.useContext(PrimeReactContext); @@ -44,6 +45,10 @@ export const OverlayPanel = React.forwardRef((inProps, ref) => { when: visibleState }); + useOnEscapeKey(overlayRef, props.dismissable && props.closeOnEscape, () => { + hide(); + }); + const isOutsideClicked = (target) => { return overlayRef && overlayRef.current && !(overlayRef.current.isSameNode(target) || overlayRef.current.contains(target)); }; diff --git a/components/lib/overlaypanel/OverlayPanelBase.js b/components/lib/overlaypanel/OverlayPanelBase.js index cda38f4a6b..73c2d09416 100644 --- a/components/lib/overlaypanel/OverlayPanelBase.js +++ b/components/lib/overlaypanel/OverlayPanelBase.js @@ -109,7 +109,8 @@ export const OverlayPanelBase = ComponentBase.extend({ transitionOptions: null, onShow: null, onHide: null, - children: undefined + children: undefined, + closeOnEscape: true }, css: { classes, diff --git a/components/lib/overlaypanel/overlaypanel.d.ts b/components/lib/overlaypanel/overlaypanel.d.ts index adb12ce5c6..cb188c6acf 100644 --- a/components/lib/overlaypanel/overlaypanel.d.ts +++ b/components/lib/overlaypanel/overlaypanel.d.ts @@ -125,6 +125,11 @@ export interface OverlayPanelProps extends Omit { const context = React.useContext(PrimeReactContext); @@ -28,14 +29,9 @@ export const Sidebar = React.forwardRef((inProps, ref) => { const maskRef = React.useRef(null); const closeIconRef = React.useRef(null); - const [bindDocumentEscapeListener, unbindDocumentEscapeListener] = useEventListener({ - type: 'keydown', - listener: (event) => { - if (event.key === 'Escape') { - if (ZIndexUtils.get(maskRef.current) === ZIndexUtils.getCurrent('modal', (context && context.autoZIndex) || PrimeReact.autoZIndex)) { - onClose(event); - } - } + useOnEscapeKey(maskRef, props.closeOnEscape, (event) => { + if (ZIndexUtils.get(maskRef.current) === ZIndexUtils.getCurrent('modal', (context && context.autoZIndex) || PrimeReact.autoZIndex)) { + onClose(event); } }); @@ -103,10 +99,6 @@ export const Sidebar = React.forwardRef((inProps, ref) => { }; const enableDocumentSettings = () => { - if (props.closeOnEscape) { - bindDocumentEscapeListener(); - } - if (props.dismissable && !props.modal) { bindDocumentClickListener(); } @@ -117,7 +109,6 @@ export const Sidebar = React.forwardRef((inProps, ref) => { }; const disableDocumentSettings = () => { - unbindDocumentEscapeListener(); unbindDocumentClickListener(); if (props.blockScroll) {