From a52ff66fb3c8a7b0fb73d418ea82adba8a028075 Mon Sep 17 00:00:00 2001 From: DIYgod Date: Tue, 26 May 2026 14:31:40 +0800 Subject: [PATCH] fix(desktop): clear stale web context menu --- .../providers/context-menu-provider.test.tsx | 148 ++++++++++++++++++ .../src/providers/context-menu-provider.tsx | 52 +++++- 2 files changed, 192 insertions(+), 8 deletions(-) create mode 100644 apps/desktop/layer/renderer/src/providers/context-menu-provider.test.tsx diff --git a/apps/desktop/layer/renderer/src/providers/context-menu-provider.test.tsx b/apps/desktop/layer/renderer/src/providers/context-menu-provider.test.tsx new file mode 100644 index 000000000..e8cd7814e --- /dev/null +++ b/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: 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 ( + + ) +} + +const renderProvider = async () => { + const container = document.createElement("div") + document.body.append(container) + + const root = createRoot(container) + await act(async () => { + root.render( + + + + + + + , + ) + }) + + 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() + }) +}) diff --git a/apps/desktop/layer/renderer/src/providers/context-menu-provider.tsx b/apps/desktop/layer/renderer/src/providers/context-menu-provider.tsx index ed07908f5..a4438e161 100644 --- a/apps/desktop/layer/renderer/src/providers/context-menu-provider.tsx +++ b/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(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 ( - + - - {contextMenuState.open && - contextMenuState.menuItems.map((item, index) => { + {contextMenuState.open && ( + + {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 + return ( + + ) })} - + + )} ) } +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 }) => { {item.submenu.map((subItem, index) => ( - + ))}