From 575f70a9e29e8355c4d47f729bc5b05e9abcbd53 Mon Sep 17 00:00:00 2001
From: Tom Moor <tom.moor@gmail.com>
Date: Sat, 31 Dec 2022 10:34:11 -0500
Subject: [PATCH] fix: useMousePosition can set state after component unmounted
 fix: Clicking item in SubMenu does not close parent menu

---
 app/components/ContextMenu/MenuItem.tsx |  5 +---
 app/components/ContextMenu/Template.tsx | 26 +++++++++++++-------
 app/components/ContextMenu/index.tsx    | 32 ++++++++++++-------------
 app/hooks/useMousePosition.ts           |  8 +++++--
 app/menus/RevisionMenu.tsx              |  1 +
 5 files changed, 41 insertions(+), 31 deletions(-)

diff --git a/app/components/ContextMenu/MenuItem.tsx b/app/components/ContextMenu/MenuItem.tsx
index 20b042b182..704e9563ab 100644
--- a/app/components/ContextMenu/MenuItem.tsx
+++ b/app/components/ContextMenu/MenuItem.tsx
@@ -41,13 +41,10 @@ const MenuItem = (
     (ev) => {
       if (onClick) {
         ev.preventDefault();
-        ev.stopPropagation();
         onClick(ev);
       }
 
-      if (hide) {
-        hide();
-      }
+      hide?.();
     },
     [onClick, hide]
   );
diff --git a/app/components/ContextMenu/Template.tsx b/app/components/ContextMenu/Template.tsx
index 6cdbb5320d..84cb0053c1 100644
--- a/app/components/ContextMenu/Template.tsx
+++ b/app/components/ContextMenu/Template.tsx
@@ -6,6 +6,7 @@ import {
   useMenuState,
   MenuButton,
   MenuItem as BaseMenuItem,
+  MenuStateReturn,
 } from "reakit/Menu";
 import styled, { useTheme } from "styled-components";
 import Flex from "~/components/Flex";
@@ -25,7 +26,7 @@ import MouseSafeArea from "./MouseSafeArea";
 import Separator from "./Separator";
 import ContextMenu from ".";
 
-type Props = {
+type Props = Omit<MenuStateReturn, "items"> & {
   actions?: (Action | MenuSeparator | MenuHeading)[];
   context?: Partial<ActionContext>;
   items?: TMenuItem[];
@@ -37,13 +38,15 @@ const Disclosure = styled(ExpandedIcon)`
   right: 8px;
 `;
 
-const Submenu = React.forwardRef(
+type SubMenuProps = MenuStateReturn & {
+  templateItems: TMenuItem[];
+  parentMenuState: Omit<MenuStateReturn, "items">;
+  title: React.ReactNode;
+};
+
+const SubMenu = React.forwardRef(
   (
-    {
-      templateItems,
-      title,
-      ...rest
-    }: { templateItems: TMenuItem[]; title: React.ReactNode },
+    { templateItems, title, parentMenuState, ...rest }: SubMenuProps,
     ref: React.LegacyRef<HTMLButtonElement>
   ) => {
     const { t } = useTranslation();
@@ -59,7 +62,11 @@ const Submenu = React.forwardRef(
             </MenuAnchor>
           )}
         </MenuButton>
-        <ContextMenu {...menu} aria-label={t("Submenu")}>
+        <ContextMenu
+          {...menu}
+          aria-label={t("Submenu")}
+          onClick={parentMenuState.hide}
+        >
           <MouseSafeArea parentRef={menu.unstable_popoverRef} />
           <Template {...menu} items={templateItems} />
         </ContextMenu>
@@ -177,8 +184,9 @@ function Template({ items, actions, context, ...menu }: Props) {
           return (
             <BaseMenuItem
               key={index}
-              as={Submenu}
+              as={SubMenu}
               templateItems={item.items}
+              parentMenuState={menu}
               title={<Title title={item.title} icon={item.icon} />}
               {...menu}
             />
diff --git a/app/components/ContextMenu/index.tsx b/app/components/ContextMenu/index.tsx
index 24c2ebc3c6..60f83c969b 100644
--- a/app/components/ContextMenu/index.tsx
+++ b/app/components/ContextMenu/index.tsx
@@ -1,7 +1,7 @@
 import { disableBodyScroll, enableBodyScroll } from "body-scroll-lock";
 import * as React from "react";
 import { useTranslation } from "react-i18next";
-import { Menu } from "reakit/Menu";
+import { Menu, MenuStateReturn } from "reakit/Menu";
 import styled, { DefaultTheme } from "styled-components";
 import breakpoint from "styled-components-breakpoint";
 import { depths } from "@shared/styles";
@@ -36,21 +36,23 @@ export type Placement =
   | "left"
   | "left-start";
 
-type Props = {
+type Props = MenuStateReturn & {
   "aria-label": string;
-  visible?: boolean;
-  placement?: Placement;
-  animating?: boolean;
-  unstable_disclosureRef?: React.RefObject<HTMLElement | null>;
+  /** The parent menu state if this is a submenu. */
+  parentMenuState?: MenuStateReturn;
+  /** Called when the context menu is opened. */
   onOpen?: () => void;
+  /** Called when the context menu is closed. */
   onClose?: () => void;
-  hide?: () => void;
+  /** Called when the context menu is clicked. */
+  onClick?: (ev: React.MouseEvent) => void;
 };
 
 const ContextMenu: React.FC<Props> = ({
   children,
   onOpen,
   onClose,
+  parentMenuState,
   ...rest
 }) => {
   const previousVisible = usePrevious(rest.visible);
@@ -67,19 +69,17 @@ const ContextMenu: React.FC<Props> = ({
 
   React.useEffect(() => {
     if (rest.visible && !previousVisible) {
-      if (onOpen) {
-        onOpen();
-      }
-      if (rest["aria-label"] !== t("Submenu")) {
+      onOpen?.();
+
+      if (!parentMenuState) {
         setIsMenuOpen(true);
       }
     }
 
     if (!rest.visible && previousVisible) {
-      if (onClose) {
-        onClose();
-      }
-      if (rest["aria-label"] !== t("Submenu")) {
+      onClose?.();
+
+      if (!parentMenuState) {
         setIsMenuOpen(false);
       }
     }
@@ -90,7 +90,7 @@ const ContextMenu: React.FC<Props> = ({
     rest.visible,
     ui.sidebarCollapsed,
     setIsMenuOpen,
-    rest,
+    parentMenuState,
     t,
   ]);
 
diff --git a/app/hooks/useMousePosition.ts b/app/hooks/useMousePosition.ts
index 6ec4936d38..475ec2b3a9 100644
--- a/app/hooks/useMousePosition.ts
+++ b/app/hooks/useMousePosition.ts
@@ -1,6 +1,7 @@
 import { throttle } from "lodash";
 import * as React from "react";
 import useEventListener from "./useEventListener";
+import useIsMounted from "./useIsMounted";
 
 /**
  * Mouse position as a tuple of [x, y]
@@ -13,6 +14,7 @@ type MousePosition = [number, number];
  * @returns Mouse position as a tuple of [x, y]
  */
 export const useMousePosition = () => {
+  const isMounted = useIsMounted();
   const [mousePosition, setMousePosition] = React.useState<MousePosition>([
     0,
     0,
@@ -21,9 +23,11 @@ export const useMousePosition = () => {
   const updateMousePosition = React.useMemo(
     () =>
       throttle((ev: MouseEvent) => {
-        setMousePosition([ev.clientX, ev.clientY]);
+        if (isMounted()) {
+          setMousePosition([ev.clientX, ev.clientY]);
+        }
       }, 200),
-    []
+    [isMounted]
   );
 
   useEventListener("mousemove", updateMousePosition);
diff --git a/app/menus/RevisionMenu.tsx b/app/menus/RevisionMenu.tsx
index e7f7903f49..f2df429aa9 100644
--- a/app/menus/RevisionMenu.tsx
+++ b/app/menus/RevisionMenu.tsx
@@ -39,6 +39,7 @@ function RevisionMenu({ document, className }: Props) {
       />
       <ContextMenu {...menu} aria-label={t("Revision options")}>
         <Template
+          {...menu}
           items={[
             actionToMenuItem(restoreRevision, context),
             separator(),