From f2f830c3c336516d88a28f98cf2ff62258736c6f Mon Sep 17 00:00:00 2001 From: Peter Makowski Date: Mon, 27 Nov 2023 10:46:46 +0100 Subject: [PATCH] revert changes to relative position - update react-useportal to v1.0.19 - add options to useListener --- package.json | 2 +- .../ContextualMenu/ContextualMenu.stories.mdx | 28 ++-- .../ContextualMenu/ContextualMenu.tsx | 137 ++++++++++-------- .../ContextualMenuDropdown.tsx | 123 ++++++++++------ src/hooks/useListener.test.ts | 44 +++++- src/hooks/useListener.ts | 23 ++- yarn.lock | 8 +- 7 files changed, 238 insertions(+), 127 deletions(-) diff --git a/package.json b/package.json index 053d6d3b3..c4c7315a8 100644 --- a/package.json +++ b/package.json @@ -94,7 +94,7 @@ "prop-types": "15.8.1", "react-router-dom": "6.17.0", "react-table": "7.8.0", - "react-useportal": "1.0.18" + "react-useportal": "1.0.19" }, "resolutions": { "@types/react": "18.2.28", diff --git a/src/components/ContextualMenu/ContextualMenu.stories.mdx b/src/components/ContextualMenu/ContextualMenu.stories.mdx index 62a02ab71..690f580d0 100644 --- a/src/components/ContextualMenu/ContextualMenu.stories.mdx +++ b/src/components/ContextualMenu/ContextualMenu.stories.mdx @@ -20,13 +20,23 @@ import ContextualMenu from "./ContextualMenu"; /> export const ScrollTemplate = (args) => ( -
+
-

Lorem ipsum dolor sit amet, consectetur adipisicing elit. - Consequuntur cum dicta beatae nostrum eligendi similique - earum, dolorem fuga quis, sequi voluptates architecto ipsa - dolorum eaque rem expedita inventore voluptas odit - aspernatur alias molestias facere.

+ {Array(3).fill( +

+ Lorem ipsum dolor sit amet, consectetur adipisicing elit. Consequuntur + cum dicta beatae nostrum eligendi similique earum, dolorem fuga quis, + sequi voluptates architecto ipsa dolorum eaque rem expedita inventore + voluptas odit aspernatur alias molestias facere. +

+ )}
); @@ -99,11 +109,11 @@ The contextual menu will provide a visual wrapper to any provided children. Visi links: [ { children: "Link 1", - onClick: () => { }, + onClick: () => {}, }, { children: "Long Link 2", - onClick: () => { }, + onClick: () => {}, }, ], hasToggleIcon: true, @@ -112,7 +122,7 @@ The contextual menu will provide a visual wrapper to any provided children. Visi }} > {ScrollTemplate.bind({})} - + ### Child function diff --git a/src/components/ContextualMenu/ContextualMenu.tsx b/src/components/ContextualMenu/ContextualMenu.tsx index 503eac599..ae5f89a2b 100644 --- a/src/components/ContextualMenu/ContextualMenu.tsx +++ b/src/components/ContextualMenu/ContextualMenu.tsx @@ -1,8 +1,9 @@ import classNames from "classnames"; import React, { useCallback, useEffect, useRef, useState } from "react"; import type { HTMLProps } from "react"; +import usePortal from "react-useportal"; -import { useListener, useOnEscapePressed, usePrevious } from "hooks"; +import { useListener, usePrevious } from "hooks"; import Button from "../Button"; import type { ButtonProps } from "../Button"; import ContextualMenuDropdown from "./ContextualMenuDropdown"; @@ -10,7 +11,6 @@ import type { ContextualMenuDropdownProps } from "./ContextualMenuDropdown"; import type { MenuLink, Position } from "./ContextualMenuDropdown"; import { ClassName, PropsWithSpread, SubComponentProps } from "types"; import { useId } from "hooks/useId"; -import { useOnClickOutside } from "hooks/useOnClickOutside"; export enum Label { Toggle = "Toggle menu", @@ -206,19 +206,21 @@ const ContextualMenu = ({ setPositionCoords(parent.getBoundingClientRect()); }, [wrapper, positionNode]); - const [isOpen, setIsOpen] = useState(visible); - const handleOpen = useCallback(() => { - // Call the toggle callback, if supplied. - onToggleMenu && onToggleMenu(true); - // When the menu opens then update the coordinates of the parent. - updatePositionCoords(); - setIsOpen(true); - }, [onToggleMenu, updatePositionCoords]); - const handleClose = useCallback(() => { - // Call the toggle callback, if supplied. - onToggleMenu && onToggleMenu(false); - setIsOpen(false); - }, [onToggleMenu]); + const { openPortal, closePortal, isOpen, Portal } = usePortal({ + closeOnEsc, + closeOnOutsideClick, + isOpen: visible, + onOpen: () => { + // Call the toggle callback, if supplied. + onToggleMenu && onToggleMenu(true); + // When the menu opens then update the coordinates of the parent. + updatePositionCoords(); + }, + onClose: () => { + // Call the toggle callback, if supplied. + onToggleMenu && onToggleMenu(false); + }, + }); const previousVisible = usePrevious(visible); const labelNode = @@ -227,7 +229,7 @@ const ContextualMenu = ({ ) : React.isValidElement(toggleLabel) ? ( toggleLabel ) : null; - const wrapperClass = classNames(className, "p-contextual-menu", { + const contextualMenuClassName = classNames(className, "p-contextual-menu", { [`p-contextual-menu--${adjustedPosition}`]: adjustedPosition !== "right", }); @@ -249,36 +251,54 @@ const ContextualMenu = ({ useEffect(() => { if (visible !== previousVisible) { if (visible && !isOpen) { - handleOpen(); + openPortal(); } else if (!visible && isOpen) { - handleClose(); + closePortal(); } } - }, [handleClose, handleOpen, visible, isOpen, previousVisible]); + }, [closePortal, openPortal, visible, isOpen, previousVisible]); - useOnClickOutside(wrapper, handleClose, { - isEnabled: closeOnOutsideClick, - }); - useOnEscapePressed(handleClose, { isEnabled: closeOnEsc }); + const onResize = useCallback( + (evt) => { + console.log("on resize!"); + const parent = getPositionNode(wrapper.current, positionNode); + if (parent && !getPositionNodeVisible(parent)) { + // Hide the menu if the item has become hidden. This might happen in + // a responsive table when columns become hidden as the page + // becomes smaller. + closePortal(evt); + } else { + // Update the coordinates so that the menu stays relative to the + // toggle button. + updatePositionCoords(); + } + }, + [closePortal, positionNode, updatePositionCoords] + ); - const onResize = useCallback(() => { - const parent = getPositionNode(wrapper.current, positionNode); - if (parent && !getPositionNodeVisible(parent)) { - // Hide the menu if the item has become hidden. This might happen in - // a responsive table when columns become hidden as the page - // becomes smaller. - handleClose(); - } else { - // Update the coordinates so that the menu stays relative to the - // toggle button. - updatePositionCoords(); - } - }, [handleClose, positionNode, updatePositionCoords]); + const onScroll = useCallback( + (e) => { + const parent = getPositionNode(wrapper.current, positionNode); + // update position if the scroll event is triggered by the parent of the menu + if (parent && e.target.contains(parent)) { + // Update the coordinates so that the menu stays relative to the + // toggle button. + console.log("update position!"); + updatePositionCoords(); + } + }, + [positionNode, updatePositionCoords] + ); useListener(window, onResize, "resize", true, isOpen); + useListener(window, onScroll, "scroll", false, isOpen, true); return ( - + {hasToggle ? ( ) : null} {isOpen && ( - - adjustedPosition={adjustedPosition} - autoAdjust={autoAdjust} - handleClose={handleClose} - constrainPanelWidth={constrainPanelWidth} - dropdownClassName={dropdownClassName} - dropdownContent={children} - id={id} - isOpen={isOpen} - links={links} - position={position} - positionCoords={positionCoords} - positionNode={getPositionNode(wrapper.current)} - scrollOverflow={scrollOverflow} - setAdjustedPosition={setAdjustedPosition} - {...dropdownProps} - /> + + + adjustedPosition={adjustedPosition} + autoAdjust={autoAdjust} + handleClose={closePortal} + constrainPanelWidth={constrainPanelWidth} + dropdownClassName={dropdownClassName} + dropdownContent={children} + id={id} + isOpen={isOpen} + links={links} + position={position} + positionCoords={positionCoords} + contextualMenuClassName={contextualMenuClassName} + positionNode={getPositionNode(wrapper.current)} + scrollOverflow={scrollOverflow} + setAdjustedPosition={setAdjustedPosition} + {...dropdownProps} + /> + )} ); diff --git a/src/components/ContextualMenu/ContextualMenuDropdown/ContextualMenuDropdown.tsx b/src/components/ContextualMenu/ContextualMenuDropdown/ContextualMenuDropdown.tsx index 847132640..5c71191f3 100644 --- a/src/components/ContextualMenu/ContextualMenuDropdown/ContextualMenuDropdown.tsx +++ b/src/components/ContextualMenu/ContextualMenuDropdown/ContextualMenuDropdown.tsx @@ -32,7 +32,7 @@ export type Position = "left" | "center" | "right"; export type Props = { adjustedPosition?: Position; autoAdjust?: boolean; - handleClose?: () => void; + handleClose?: (evt?: MouseEvent) => void; constrainPanelWidth?: boolean; dropdownClassName?: string; dropdownContent?: ReactNode | ((close: () => void) => ReactElement); @@ -40,11 +40,11 @@ export type Props = { isOpen?: boolean; links?: MenuLink[]; position?: Position; - positionCoords?: ClientRect; + positionCoords?: DOMRect; positionNode?: HTMLElement; scrollOverflow?: boolean; setAdjustedPosition?: (position: Position) => void; - wrapperClass?: string; + contextualMenuClassName?: string; } & HTMLProps; /** @@ -54,16 +54,35 @@ export type Props = { * @param constrainPanelWidth - Whether the menu width should be constrained to the position width. */ const getPositionStyle = ( + position: Position, positionCoords: Props["positionCoords"], constrainPanelWidth: Props["constrainPanelWidth"] -) => { +): React.CSSProperties => { if (!positionCoords) { return null; } - const { height, width } = positionCoords; + const { height, left, top, width } = positionCoords; + const topPos = top + height + (window.scrollY || 0); + let leftPos = left; + + switch (position) { + case "left": + leftPos = left; + break; + case "center": + leftPos = left + width / 2; + break; + case "right": + leftPos = left + width; + break; + default: + break; + } return { - top: height, + position: "absolute", + left: leftPos, + top: topPos, // The width only needs to be set if the width is to be constrained. ...(constrainPanelWidth ? { width } : null), }; @@ -130,7 +149,7 @@ const generateLink = ( onClick={ onClick ? (evt) => { - handleClose(); + handleClose(evt.nativeEvent); onClick(evt); } : null @@ -157,19 +176,22 @@ const ContextualMenuDropdown = ({ positionNode, scrollOverflow, setAdjustedPosition, - wrapperClass, + contextualMenuClassName, ...props }: Props): JSX.Element => { const dropdown = useRef(); + const [positionStyle, setPositionStyle] = useState( - getPositionStyle(positionCoords, constrainPanelWidth) + getPositionStyle(adjustedPosition, positionCoords, constrainPanelWidth) ); const [maxHeight, setMaxHeight] = useState(); // Update the styles to position the menu. const updatePositionStyle = useCallback(() => { - setPositionStyle(getPositionStyle(positionCoords, constrainPanelWidth)); - }, [positionCoords, constrainPanelWidth]); + setPositionStyle( + getPositionStyle(adjustedPosition, positionCoords, constrainPanelWidth) + ); + }, [adjustedPosition, positionCoords, constrainPanelWidth]); // Update the position when the window fitment info changes. const onUpdateWindowFitment = useCallback( @@ -199,43 +221,48 @@ const ContextualMenuDropdown = ({ }, [adjustedPosition, updatePositionStyle]); return ( - - {dropdownContent - ? typeof dropdownContent === "function" - ? dropdownContent(handleClose) - : dropdownContent - : links.map((item, i) => { - if (Array.isArray(item)) { - return ( - - {item.map((link, j) => generateLink(link, j, handleClose))} - - ); - } else if (typeof item === "string") { - return ( -
- {item} -
- ); - } - return generateLink(item, i, handleClose); - })} + // Vanilla Framework uses .p-contextual-menu parent modifier classnames to determine the correct position of the .p-contextual-menu__dropdown dropdown (left, center, right). + // Extra span wrapper is required as the dropdown is rendered in a portal. + + + {dropdownContent + ? typeof dropdownContent === "function" + ? dropdownContent(handleClose) + : dropdownContent + : links.map((item, i) => { + if (Array.isArray(item)) { + return ( + + {item.map((link, j) => + generateLink(link, j, handleClose) + )} + + ); + } else if (typeof item === "string") { + return ( +
+ {item} +
+ ); + } + return generateLink(item, i, handleClose); + })} + ); }; diff --git a/src/hooks/useListener.test.ts b/src/hooks/useListener.test.ts index 976f6c1ad..73a7dccc7 100644 --- a/src/hooks/useListener.test.ts +++ b/src/hooks/useListener.test.ts @@ -8,6 +8,7 @@ type Args = { eventType?: string; shouldThrottle?: boolean; shouldListen?: boolean; + options?: boolean | AddEventListenerOptions; }; describe("useListener", () => { @@ -29,6 +30,7 @@ describe("useListener", () => { eventType: "mousemove", shouldThrottle: false, shouldListen: true, + options: undefined, }; renderListener = (initialProps) => renderHook( @@ -38,7 +40,8 @@ describe("useListener", () => { args.callback, args.eventType, args.shouldThrottle, - args.shouldListen + args.shouldListen, + args.options ), { initialProps: { @@ -51,7 +54,21 @@ describe("useListener", () => { it("initially adds the listener", () => { renderListener(); - expect(addEventListener).toHaveBeenCalledWith("mousemove", callback); + expect(addEventListener).toHaveBeenCalledWith( + "mousemove", + callback, + undefined + ); + }); + + it("initially adds the listener with options", () => { + renderListener({ + eventType: "mousemove", + callback, + options: true, + }); + const args = ["mousemove", callback, true]; + expect(addEventListener).toHaveBeenCalledWith(...args); }); it("does not add the listener if it shouldn't listen", () => { @@ -124,6 +141,16 @@ describe("useListener", () => { expect(addEventListener).toHaveBeenCalled(); }); + it("re-adds the listener if options change", () => { + const { rerender } = renderListener(); + rerender({ + ...defaultArgs, + options: true, + }); + expect(removeEventListener).toHaveBeenCalled(); + expect(addEventListener).toHaveBeenCalled(); + }); + it("stops listening when unmounting", () => { const { unmount } = renderHook(() => useListener(targetNode, callback, "mousemove") @@ -132,4 +159,17 @@ describe("useListener", () => { unmount(); expect(removeEventListener).toHaveBeenCalled(); }); + + it("stops listening when unmounting with options", () => { + const callback = jest.fn(); + const { unmount } = renderListener({ + eventType: "mousemove", + callback, + options: true, + }); + const args = ["mousemove", callback, true]; + expect(addEventListener).toHaveBeenCalledWith(...args); + unmount(); + expect(removeEventListener).toHaveBeenCalledWith(...args); + }); }); diff --git a/src/hooks/useListener.ts b/src/hooks/useListener.ts index c0adf3916..8016ac7e4 100644 --- a/src/hooks/useListener.ts +++ b/src/hooks/useListener.ts @@ -11,13 +11,15 @@ import { usePrevious } from "./usePrevious"; * @param eventType The event name. * @param shouldThrottle Whether the callback calls should be throttled. * @param shouldListen When the listener should be active. + * @param options Native event listener options. */ export const useListener = ( targetNode: Window | HTMLElement, callback: (...args: any[]) => any, // eslint-disable-line @typescript-eslint/no-explicit-any eventType: string, shouldThrottle = false, - shouldListen = true + shouldListen = true, + options?: boolean | AddEventListenerOptions ): void => { const isListening = useRef(false); const throttle = useThrottle(callback); @@ -26,6 +28,7 @@ export const useListener = ( const previousShouldThrottle = usePrevious(shouldThrottle); const previousTargetNode = usePrevious(targetNode); const previousCallback = usePrevious(callback); + const previousOptions = usePrevious(options); useEffect(() => { // If any of the props related to the attached listener changed then the @@ -34,11 +37,13 @@ export const useListener = ( callback !== previousCallback || eventType !== previousEventType || shouldThrottle !== previousShouldThrottle || - targetNode !== previousTargetNode; + targetNode !== previousTargetNode || + options !== previousOptions; if (isListening.current && (!shouldListen || listenerAttributesChanged)) { previousTargetNode.removeEventListener( previousEventType, - eventListener.current + eventListener.current, + previousOptions ); isListening.current = false; } @@ -52,14 +57,16 @@ export const useListener = ( } if (targetNode && shouldListen && !isListening.current) { - targetNode.addEventListener(eventType, eventListener.current); + targetNode.addEventListener(eventType, eventListener.current, options); isListening.current = true; } }, [ callback, eventType, + options, previousCallback, previousEventType, + previousOptions, previousShouldThrottle, previousTargetNode, shouldListen, @@ -73,9 +80,13 @@ export const useListener = ( // Unattach the listener if the component gets unmounted while // listening. if (targetNode && isListening.current) { - targetNode.removeEventListener(eventType, eventListener.current); + targetNode.removeEventListener( + eventType, + eventListener.current, + options + ); } }, - [eventType, targetNode] + [eventType, targetNode, options] ); }; diff --git a/yarn.lock b/yarn.lock index ab5662417..1324e4ba9 100644 --- a/yarn.lock +++ b/yarn.lock @@ -12091,10 +12091,10 @@ react-table@7.8.0: resolved "https://registry.yarnpkg.com/react-table/-/react-table-7.8.0.tgz#07858c01c1718c09f7f1aed7034fcfd7bda907d2" integrity sha512-hNaz4ygkZO4bESeFfnfOft73iBUj8K5oKi1EcSHPAibEydfsX2MyU6Z8KCr3mv3C9Kqqh71U+DhZkFvibbnPbA== -react-useportal@1.0.18: - version "1.0.18" - resolved "https://registry.yarnpkg.com/react-useportal/-/react-useportal-1.0.18.tgz#b3baf14962f44402b2d0d6152acc2035c5342bd8" - integrity sha512-dGuT/yyE2T9RtUxRZJYkIX8tLHC7KxAxbMw/Ufjiwo8ixoDYzkk9LFKGnARtCtFz6Yd5AoP7fVymrN3eT04jiA== +react-useportal@1.0.19: + version "1.0.19" + resolved "https://registry.yarnpkg.com/react-useportal/-/react-useportal-1.0.19.tgz#473168f1a1c4009833d49a46b05d974a5850a166" + integrity sha512-gXXxBu/j+gQrghUh5l0/GJxy3MoRxQRX0P3jpNI8c+4v/ey1ZGW3Tqh/wTYgBs8NdumvaYe+IiKGMYntEv07CA== dependencies: use-ssr "^1.0.25"