fix(desktop): clear stale web context menu

apps/desktop/layer/renderer/src/providers/context-menu-provider.test.tsx

@@ -0,0 +1,148 @@
+import { GlobalFocusableProvider } from "@follow/components/common/Focusable/GlobalFocusableProvider.js"
+import { Provider } from "jotai"
+import * as React from "react"
+import { act } from "react"
+import type { Root } from "react-dom/client"
+import { createRoot } from "react-dom/client"
+import { afterAll, afterEach, beforeAll, describe, expect, test, vi } from "vitest"
+
+import { contextMenuAtom, MenuItemText, useShowContextMenu } from "~/atoms/context-menu"
+import { jotaiStore } from "~/lib/jotai"
+
+import { ContextMenuProvider } from "./context-menu-provider"
+
+const { requireLoginMock } = vi.hoisted(() => ({
+  requireLoginMock: () => ({
+    withLoginGuard: <T extends (...args: never[]) => unknown>(action: T) => action,
+  }),
+}))
+
+vi.mock("~/hooks/common/useRequireLogin", () => ({
+  useRequireLogin: requireLoginMock,
+}))
+
+const waitForContextMenuEffects = async () => {
+  for (let index = 0; index < 3; index += 1) {
+    await new Promise((resolve) => setTimeout(resolve, 0))
+  }
+}
+
+const TestMenuTrigger = () => {
+  const showContextMenu = useShowContextMenu()
+
+  return (
+    <button
+      type="button"
+      onContextMenu={(event) => {
+        event.preventDefault()
+        void showContextMenu(
+          [
+            new MenuItemText({
+              label: "Archive",
+              click: () => {},
+            }),
+          ],
+          event,
+        )
+      }}
+    >
+      Open menu
+    </button>
+  )
+}
+
+const renderProvider = async () => {
+  const container = document.createElement("div")
+  document.body.append(container)
+
+  const root = createRoot(container)
+  await act(async () => {
+    root.render(
+      <Provider store={jotaiStore}>
+        <GlobalFocusableProvider>
+          <ContextMenuProvider>
+            <TestMenuTrigger />
+          </ContextMenuProvider>
+        </GlobalFocusableProvider>
+      </Provider>,
+    )
+  })
+
+  return { container, root }
+}
+
+describe("ContextMenuProvider", () => {
+  let root: Root | null = null
+  let container: HTMLElement | null = null
+
+  beforeAll(() => {
+    ;(globalThis as typeof globalThis & { React: typeof React }).React = React
+    ;(
+      globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }
+    ).IS_REACT_ACT_ENVIRONMENT = true
+    vi.spyOn(console, "info").mockImplementation(() => {})
+
+    Object.assign(window, {
+      clearTimeout,
+      Element: document.defaultView?.Element ?? Element,
+      getComputedStyle:
+        document.defaultView?.getComputedStyle.bind(document.defaultView) ?? getComputedStyle,
+      HTMLElement: document.defaultView?.HTMLElement ?? HTMLElement,
+      innerHeight: 768,
+      innerWidth: 1024,
+      Node: document.defaultView?.Node ?? Node,
+      setTimeout,
+    })
+  })
+
+  afterAll(() => {
+    vi.restoreAllMocks()
+  })
+
+  afterEach(async () => {
+    await act(async () => {
+      jotaiStore.set(contextMenuAtom, { open: false })
+      await waitForContextMenuEffects()
+    })
+
+    if (root) {
+      await act(async () => {
+        root?.unmount()
+      })
+    }
+
+    container?.remove()
+    document.body.innerHTML = ""
+    root = null
+    container = null
+    vi.clearAllMocks()
+  })
+
+  test("removes the web menu shell when the app menu state closes", async () => {
+    ;({ container, root } = await renderProvider())
+
+    const trigger = container.querySelector("button")
+    expect(trigger).not.toBeNull()
+
+    await act(async () => {
+      trigger?.dispatchEvent(
+        new MouseEvent("contextmenu", {
+          bubbles: true,
+          cancelable: true,
+          clientX: 120,
+          clientY: 80,
+        }),
+      )
+      await waitForContextMenuEffects()
+    })
+
+    expect(document.querySelector('[role="menu"]')).not.toBeNull()
+
+    await act(async () => {
+      jotaiStore.set(contextMenuAtom, { open: false })
+      await waitForContextMenuEffects()
+    })
+
+    expect(document.querySelector('[role="menu"]')).toBeNull()
+  })
+})

apps/desktop/layer/renderer/src/providers/context-menu-provider.tsx

@@ -18,7 +18,7 @@ import {
 import { KbdCombined } from "@follow/components/ui/kbd/Kbd.js"
 import { nextFrame, preventDefault } from "@follow/utils/dom"
 import { cn } from "@follow/utils/utils"
-import { Fragment, memo, useCallback, useEffect, useRef } from "react"
+import { Fragment, memo, useCallback, useEffect, useReducer, useRef } from "react"
 import { useHotkeys } from "react-hotkeys-hook"
 
 import type { FollowMenuItem } from "~/atoms/context-menu"
@@ -40,6 +40,8 @@ export const ContextMenuProvider: Component = ({ children }) => (
 const Handler = () => {
   const ref = useRef<HTMLSpanElement>(null)
   const [contextMenuState, setContextMenuState] = useContextMenuState()
+  const wasOpenRef = useRef(false)
+  const [contextMenuKey, resetContextMenu] = useReducer((key) => key + 1, 0)
 
   useEffect(() => {
     if (!contextMenuState.open) return
@@ -55,6 +57,18 @@ const Handler = () => {
       }),
     )
   }, [contextMenuState])
+
+  useEffect(() => {
+    if (contextMenuState.open) {
+      wasOpenRef.current = true
+      return
+    }
+
+    if (!wasOpenRef.current) return
+    wasOpenRef.current = false
+    resetContextMenu()
+  }, [contextMenuState.open])
+
   const setGlobalFocusableScope = useSetGlobalFocusableScope()
 
   const handleOpenChange = useCallback(
@@ -69,11 +83,11 @@ const Handler = () => {
   )
 
   return (
-    <ContextMenu onOpenChange={handleOpenChange}>
+    <ContextMenu key={contextMenuKey} onOpenChange={handleOpenChange}>
       <ContextMenuTrigger className="hidden" ref={ref} />
-      <ContextMenuContent onContextMenu={preventDefault}>
-        {contextMenuState.open &&
-          contextMenuState.menuItems.map((item, index) => {
+      {contextMenuState.open && (
+        <ContextMenuContent onContextMenu={preventDefault}>
+          {contextMenuState.menuItems.map((item, index) => {
             const prevItem = contextMenuState.menuItems[index - 1]
             if (prevItem instanceof MenuItemSeparator && item instanceof MenuItemSeparator) {
               return null
@@ -86,13 +100,35 @@ const Handler = () => {
             if (!nextItem && item instanceof MenuItemSeparator) {
               return null
             }
-            return <Item key={index} item={item} />
+            return (
+              <Item key={getMenuItemKey(item, index, contextMenuState.menuItems)} item={item} />
+            )
           })}
-      </ContextMenuContent>
+        </ContextMenuContent>
+      )}
     </ContextMenu>
   )
 }
 
+const getMenuItemKey = (item: FollowMenuItem, index: number, items: FollowMenuItem[]) => {
+  if (item instanceof MenuItemSeparator) {
+    const previousItem = items[index - 1]
+    const nextItem = items[index + 1]
+    const previousLabel = previousItem instanceof MenuItemText ? previousItem.label : "start"
+    const nextLabel = nextItem instanceof MenuItemText ? nextItem.label : "end"
+
+    return `separator-${previousLabel}-${nextLabel}`
+  }
+
+  return [
+    item.label,
+    item.shortcut ?? "no-shortcut",
+    typeof item.checked === "boolean" ? item.checked.toString() : "unchecked",
+    item.disabled ? "disabled" : "enabled",
+    item.submenu.length.toString(),
+  ].join(":")
+}
+
 const Item = memo(({ item }: { item: FollowMenuItem }) => {
   const onClick = useCallback(() => {
     if ("click" in item) {
@@ -155,7 +191,7 @@ const Item = memo(({ item }: { item: FollowMenuItem }) => {
             <ContextMenuPortal>
               <ContextMenuSubContent>
                 {item.submenu.map((subItem, index) => (
-                  <Item key={index} item={subItem} />
+                  <Item key={getMenuItemKey(subItem, index, item.submenu)} item={subItem} />
                 ))}
               </ContextMenuSubContent>
             </ContextMenuPortal>