Fix primefaces#4704: Image preview popup not closing on escape button…

… click (primefaces#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: primefaces#4750, Added closeOnEscape to Overlay panel --------- Co-authored-by: akshayantony55 <akshayantony55@gmail.com>
melloware · Aug 9, 2023 · 1c97331 · 1c97331
1 parent 9dbd6b4
commit 1c97331
Show file tree

Hide file tree

Showing 11 changed files with 88 additions and 21 deletions.
diff --git 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
@@ -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
@@ -291,3 +291,9 @@ export declare function useClickOutside(ref: React.RefObject<Element>, 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<Element>} 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<Element>, callback: any): void;
diff --git a/components/lib/hooks/useOnEscapeKey.js 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
@@ -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
@@ -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
@@ -247,6 +247,11 @@ export interface ImageProps extends Omit<React.DetailedHTMLProps<React.HTMLAttri
      * @defaultValue false
      */
     unstyled?: boolean;
+    /**
+     * Specifies if pressing escape key should hide the preview.
+     * @defaultValue true
+     */
+    closeOnEscape?: boolean | undefined;
 }
 
 /**

diff --git a/components/lib/overlaypanel/OverlayPanel.js b/components/lib/overlaypanel/OverlayPanel.js
@@ -9,6 +9,7 @@ import { Portal } from '../portal/Portal';
 import { Ripple } from '../ripple/Ripple';
 import { DomHandler, IconUtils, UniqueComponentId, ZIndexUtils, mergeProps } from '../utils/Utils';
 import { OverlayPanelBase } from './OverlayPanelBase';
+import { useOnEscapeKey } from '../../lib/hooks/Hooks';
 
 export const OverlayPanel = React.forwardRef((inProps, ref) => {
     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
@@ -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
@@ -125,6 +125,11 @@ export interface OverlayPanelProps extends Omit<React.DetailedHTMLProps<React.HT
      * @defaultValue false
      */
     unstyled?: boolean;
+    /**
+     * Specifies if pressing escape key should hide the preview.
+     * @defaultValue true
+     */
+    closeOnEscape?: boolean | undefined;
 }
 
 /**

diff --git a/components/lib/sidebar/Sidebar.js b/components/lib/sidebar/Sidebar.js
@@ -8,6 +8,7 @@ import { Portal } from '../portal/Portal';
 import { Ripple } from '../ripple/Ripple';
 import { DomHandler, IconUtils, ObjectUtils, ZIndexUtils, mergeProps } from '../utils/Utils';
 import { SidebarBase } from './SidebarBase';
+import { useOnEscapeKey } from '../../lib/hooks/Hooks';
 
 export const Sidebar = React.forwardRef((inProps, ref) => {
     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) {