From b7e325e9986af25b548950587fbcf04a60ee82a4 Mon Sep 17 00:00:00 2001 From: FraterCCCLXIII Date: Mon, 10 Aug 2026 01:41:46 -0700 Subject: [PATCH] fix: make overflow menus usable on touch devices (#16101) Co-authored-by: Cursor Co-authored-by: hieptl --- .../conversation-card.test.tsx | 48 +++++ .../conversation-name-with-status.test.tsx | 168 ++++++++++++++++++ .../profile-actions-menu.test.tsx | 20 +++ .../hooks/use-click-outside-element.test.tsx | 34 ++++ __tests__/utils/hover-reveal-classes.test.ts | 117 ++++++++++++ .../controls/server-status-context-menu.tsx | 12 +- .../conversation-card-actions.tsx | 4 + .../conversation-card/conversation-card.tsx | 37 ++-- .../conversation-name-context-menu.tsx | 2 +- .../conversation-name-with-status.tsx | 97 ++++++++-- .../conversation/conversation-name.tsx | 5 +- .../agent-profile-actions-menu.tsx | 8 +- .../llm-profiles/profile-actions-menu.tsx | 8 +- src/utils/hover-reveal-classes.ts | 94 ++++++++++ 14 files changed, 602 insertions(+), 52 deletions(-) create mode 100644 __tests__/components/features/conversation/conversation-name-with-status.test.tsx create mode 100644 __tests__/utils/hover-reveal-classes.test.ts create mode 100644 src/utils/hover-reveal-classes.ts diff --git a/__tests__/components/features/conversation-panel/conversation-card.test.tsx b/__tests__/components/features/conversation-panel/conversation-card.test.tsx index ee97512cd1..9070ed69f7 100644 --- a/__tests__/components/features/conversation-panel/conversation-card.test.tsx +++ b/__tests__/components/features/conversation-panel/conversation-card.test.tsx @@ -312,6 +312,54 @@ describe("ConversationCard", () => { expect(onContextMenuToggle).toHaveBeenCalledWith(false); }); + it("keeps the ellipsis clickable without hover via touch-first reveal classes", () => { + renderWithProviders( + , + ); + + const ellipsisButton = screen.getByTestId("ellipsis-button"); + const actionOverlay = ellipsisButton.parentElement; + + expect(actionOverlay).toHaveClass("pointer-events-auto"); + expect(actionOverlay?.className).toContain( + "[@media(hover:hover)_and_(pointer:fine)]:pointer-events-none", + ); + }); + + it("closes the context menu when clicking outside", async () => { + const user = userEvent.setup(); + const onContextMenuToggle = vi.fn(); + + renderWithProviders( +
+
Outside
+ +
, + ); + + expect(screen.getByTestId("context-menu")).toBeInTheDocument(); + + await user.click(screen.getByTestId("outside")); + + expect(onContextMenuToggle).toHaveBeenCalledWith(false); + }); + it("should call onDelete when the delete button is clicked", async () => { const user = userEvent.setup(); const onContextMenuToggle = vi.fn(); diff --git a/__tests__/components/features/conversation/conversation-name-with-status.test.tsx b/__tests__/components/features/conversation/conversation-name-with-status.test.tsx new file mode 100644 index 0000000000..9cb41ff550 --- /dev/null +++ b/__tests__/components/features/conversation/conversation-name-with-status.test.tsx @@ -0,0 +1,168 @@ +import { fireEvent, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { renderWithProviders } from "test-utils"; +import { ConversationNameWithStatus } from "#/components/features/conversation/conversation-name-with-status"; +import { AgentState } from "#/types/agent-state"; +import { ExecutionStatus } from "#/types/agent-server/core/base/common"; +import { useAgentState } from "#/hooks/use-agent-state"; + +vi.mock("#/hooks/use-agent-state", () => ({ + useAgentState: vi.fn(), +})); + +vi.mock("#/hooks/query/use-task-polling", () => ({ + useTaskPolling: () => ({ + isTask: false, + taskStatus: null, + }), +})); + +vi.mock("#/hooks/query/use-active-conversation", () => ({ + useActiveConversation: () => ({ + data: { + id: "test-conversation-id", + title: "Test conversation", + execution_status: ExecutionStatus.RUNNING, + }, + }), +})); + +vi.mock("#/hooks/use-conversation-id", () => ({ + useConversationId: () => ({ conversationId: "test-conversation-id" }), +})); + +vi.mock("#/hooks/mutation/use-unified-stop-conversation", () => ({ + useUnifiedPauseConversation: () => ({ mutate: vi.fn() }), +})); + +vi.mock("#/hooks/mutation/use-unified-start-conversation", () => ({ + useUnifiedResumeConversation: () => ({ mutate: vi.fn() }), +})); + +vi.mock("#/hooks/use-user-providers", () => ({ + useUserProviders: () => ({ providers: [] }), +})); + +vi.mock("#/components/features/conversation/conversation-name", () => ({ + ConversationName: () =>
, +})); + +vi.mock("#/components/features/conversation/right-panel-toggle", () => ({ + RightPanelToggle: () => null, +})); + +vi.mock("react-i18next", async () => { + const actual = await vi.importActual("react-i18next"); + return { + ...actual, + useTranslation: () => ({ + t: (key: string) => key, + i18n: { changeLanguage: () => Promise.resolve() }, + }), + }; +}); + +describe("ConversationNameWithStatus", () => { + beforeEach(() => { + vi.mocked(useAgentState).mockReturnValue({ + curAgentState: AgentState.RUNNING, + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it("opens the server status menu on click and closes on outside click", async () => { + const user = userEvent.setup(); + + renderWithProviders( +
+
Outside
+ +
, + ); + + expect( + screen.queryByTestId("server-status-context-menu"), + ).not.toBeInTheDocument(); + + // fireEvent click avoids a preceding mouse pointerenter (hover-open), which + // models the touch/toggle path this assertion covers. + fireEvent.click(screen.getByTestId("server-status-menu-trigger")); + + expect( + screen.getByTestId("server-status-context-menu"), + ).toBeInTheDocument(); + + await user.click(screen.getByTestId("outside")); + + expect( + screen.queryByTestId("server-status-context-menu"), + ).not.toBeInTheDocument(); + }); + + it("toggles the server status menu closed when the trigger is clicked again", () => { + renderWithProviders(); + + const trigger = screen.getByTestId("server-status-menu-trigger"); + fireEvent.click(trigger); + expect( + screen.getByTestId("server-status-context-menu"), + ).toBeInTheDocument(); + + fireEvent.click(trigger); + expect( + screen.queryByTestId("server-status-context-menu"), + ).not.toBeInTheDocument(); + }); + + it("opens on mouse pointerenter and dismisses on a following mouse click", () => { + renderWithProviders(); + + const trigger = screen.getByTestId("server-status-menu-trigger"); + const hoverTarget = trigger.parentElement; + expect(hoverTarget).not.toBeNull(); + + // pointerover is the event React delegates for onPointerEnter synthesis, + // so it reaches the handler reliably across jsdom setups. + fireEvent.pointerOver(hoverTarget!, { pointerType: "mouse" }); + expect( + screen.getByTestId("server-status-context-menu"), + ).toBeInTheDocument(); + + fireEvent.click(trigger); + expect( + screen.queryByTestId("server-status-context-menu"), + ).not.toBeInTheDocument(); + }); + + it("keeps click-toggle open after touch/compatibility hover events on fine-hover hardware", () => { + // Device-primary pointer reports fine hover, but the interaction is touch. + vi.stubGlobal( + "matchMedia", + vi.fn().mockReturnValue({ + matches: true, + media: "(hover: hover) and (pointer: fine)", + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + }), + ); + + renderWithProviders(); + + const trigger = screen.getByTestId("server-status-menu-trigger"); + const hoverTarget = trigger.parentElement; + expect(hoverTarget).not.toBeNull(); + + // Compatibility mouseenter must not open-then-close against the tap. + fireEvent.mouseEnter(hoverTarget!); + fireEvent.pointerOver(hoverTarget!, { pointerType: "touch" }); + fireEvent.click(trigger); + + expect( + screen.getByTestId("server-status-context-menu"), + ).toBeInTheDocument(); + }); +}); diff --git a/__tests__/components/settings/llm-profiles/profile-actions-menu.test.tsx b/__tests__/components/settings/llm-profiles/profile-actions-menu.test.tsx index 04fb200c66..0aa044ca63 100644 --- a/__tests__/components/settings/llm-profiles/profile-actions-menu.test.tsx +++ b/__tests__/components/settings/llm-profiles/profile-actions-menu.test.tsx @@ -196,6 +196,26 @@ describe("ProfileActionsMenu", () => { expect(handleClose).toHaveBeenCalledTimes(1); }); + it("does not call onClose when mousedown lands on the anchor trigger", () => { + const handleClose = vi.fn(); + const anchorRef = { current: document.createElement("button") }; + anchorRef.current.setAttribute("data-testid", "profile-menu-trigger"); + document.body.appendChild(anchorRef.current); + + render( + , + ); + + fireEvent.mouseDown(anchorRef.current); + + expect(handleClose).not.toHaveBeenCalled(); + anchorRef.current.remove(); + }); + it("calls onClose when Escape key is pressed", () => { const handleClose = vi.fn(); diff --git a/__tests__/hooks/use-click-outside-element.test.tsx b/__tests__/hooks/use-click-outside-element.test.tsx index 3f9c77c0ce..b60077bda1 100644 --- a/__tests__/hooks/use-click-outside-element.test.tsx +++ b/__tests__/hooks/use-click-outside-element.test.tsx @@ -1,3 +1,4 @@ +import React from "react"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { expect, test, vi } from "vitest"; @@ -34,3 +35,36 @@ test("call the callback when the element is clicked outside", async () => { await user.click(outsideElement); expect(callback).toHaveBeenCalled(); }); + +test("does not call the callback when clicking the ignored trigger", async () => { + const user = userEvent.setup(); + const callback = vi.fn(); + + function Harness() { + const ignoreOutsideClickRef = React.useRef(null); + const ref = useClickOutsideElement( + callback, + ignoreOutsideClickRef, + ); + + return ( +
+
+ + {isMenuVisible ? ( + + ) : null}
diff --git a/src/components/features/conversation/conversation-name.tsx b/src/components/features/conversation/conversation-name.tsx index b22c28d0b0..c48a91cba7 100644 --- a/src/components/features/conversation/conversation-name.tsx +++ b/src/components/features/conversation/conversation-name.tsx @@ -176,7 +176,10 @@ export function ConversationName() { ref={ellipsisAnchorRef} className="relative flex items-center shrink-0" > - + {contextMenuOpen && ( setContextMenuOpen(false)} diff --git a/src/components/features/settings/agent-profiles/agent-profile-actions-menu.tsx b/src/components/features/settings/agent-profiles/agent-profile-actions-menu.tsx index 5dd2f09889..1f641d003d 100644 --- a/src/components/features/settings/agent-profiles/agent-profile-actions-menu.tsx +++ b/src/components/features/settings/agent-profiles/agent-profile-actions-menu.tsx @@ -78,9 +78,9 @@ export function AgentProfileActionsMenu({ useEffect(() => { const handleClickOutside = (event: MouseEvent) => { const target = event.target as Node; - if (menuRef.current && !menuRef.current.contains(target)) { - onClose(); - } + if (menuRef.current?.contains(target)) return; + if (anchorElement?.contains(target)) return; + onClose(); }; const handleEscape = (event: KeyboardEvent) => { if (event.key === "Escape") onClose(); @@ -91,7 +91,7 @@ export function AgentProfileActionsMenu({ document.removeEventListener("mousedown", handleClickOutside); document.removeEventListener("keydown", handleEscape); }; - }, [onClose]); + }, [anchorElement, onClose]); const handleAction = (action: () => void) => { action(); diff --git a/src/components/features/settings/llm-profiles/profile-actions-menu.tsx b/src/components/features/settings/llm-profiles/profile-actions-menu.tsx index 50874ce7c2..ef1106720f 100644 --- a/src/components/features/settings/llm-profiles/profile-actions-menu.tsx +++ b/src/components/features/settings/llm-profiles/profile-actions-menu.tsx @@ -85,9 +85,9 @@ export function ProfileActionsMenu({ useEffect(() => { const handleClickOutside = (event: MouseEvent) => { const target = event.target as Node; - if (menuRef.current && !menuRef.current.contains(target)) { - onClose(); - } + if (menuRef.current?.contains(target)) return; + if (anchorElement?.contains(target)) return; + onClose(); }; const handleEscape = (event: KeyboardEvent) => { @@ -103,7 +103,7 @@ export function ProfileActionsMenu({ document.removeEventListener("mousedown", handleClickOutside); document.removeEventListener("keydown", handleEscape); }; - }, [onClose]); + }, [anchorElement, onClose]); const handleAction = (action: () => void) => { action(); diff --git a/src/utils/hover-reveal-classes.ts b/src/utils/hover-reveal-classes.ts new file mode 100644 index 0000000000..d8ac8f5ccd --- /dev/null +++ b/src/utils/hover-reveal-classes.ts @@ -0,0 +1,94 @@ +import { cn } from "#/utils/utils"; + +/** + * Full Tailwind candidates for fine-hover media — devices where CSS `:hover` + * is a reliable primary interaction (mouse/trackpad). Coarse-pointer / + * touch-primary devices match the inverse and keep overflow actions always + * visible + clickable. Kept as complete string literals so the production CSS + * scanner can emit the arbitrary-variant rules (dynamic prefix concatenation + * is not discoverable). + */ +export const FINE_HOVER_ACTION_CLASSES = [ + "[@media(hover:hover)_and_(pointer:fine)]:pointer-events-none", + "[@media(hover:hover)_and_(pointer:fine)]:invisible", + "[@media(hover:hover)_and_(pointer:fine)]:opacity-0", + "[@media(hover:hover)_and_(pointer:fine)]:group-hover:pointer-events-auto", + "[@media(hover:hover)_and_(pointer:fine)]:group-hover:visible", + "[@media(hover:hover)_and_(pointer:fine)]:group-hover:opacity-100", + "[@media(hover:hover)_and_(pointer:fine)]:group-focus-within:pointer-events-auto", + "[@media(hover:hover)_and_(pointer:fine)]:group-focus-within:visible", + "[@media(hover:hover)_and_(pointer:fine)]:group-focus-within:opacity-100", +] as const; + +export const FINE_HOVER_YIELD_CLASSES = [ + "[@media(hover:hover)_and_(pointer:fine)]:opacity-100", + "[@media(hover:hover)_and_(pointer:fine)]:group-hover:opacity-0", + "[@media(hover:hover)_and_(pointer:fine)]:group-focus-within:opacity-0", +] as const; + +export const FINE_HOVER_RESERVE_CLASSES = [ + "[@media(hover:hover)_and_(pointer:fine)]:min-w-0", + "[@media(hover:hover)_and_(pointer:fine)]:group-hover:min-w-[3.75rem]", + "[@media(hover:hover)_and_(pointer:fine)]:group-focus-within:min-w-[3.75rem]", +] as const; + +export const FINE_HOVER_PINNED_TIMESTAMP_CLASSES = [ + "[@media(hover:hover)_and_(pointer:fine)]:flex", + "[@media(hover:hover)_and_(pointer:fine)]:group-hover:hidden", + "[@media(hover:hover)_and_(pointer:fine)]:group-focus-within:hidden", +] as const; + +/** + * Overlay action chrome (ellipsis, pin, etc.): always interactable on touch; + * hover/focus-reveal only on fine-pointer hover devices. + */ +export function hoverRevealActionClassName(forceVisible = false): string { + if (forceVisible) { + return "pointer-events-auto visible opacity-100"; + } + + return cn( + "pointer-events-auto visible opacity-100", + ...FINE_HOVER_ACTION_CLASSES, + ); +} + +/** + * Companion for timestamps that yield space to hover-reveal actions: + * hidden on touch (actions stay visible); on fine-pointer devices, visible + * until the row is hovered / focused / menu-open. + */ +export function hoverRevealYieldClassName(forceHidden = false): string { + if (forceHidden) { + return "opacity-0"; + } + + return cn("opacity-0", ...FINE_HOVER_YIELD_CLASSES); +} + +/** + * Reserve trailing space for hover-reveal actions. Always reserved on touch; + * on fine-pointer devices, reserved on hover / focus / open. + */ +export function hoverRevealReserveClassName(forceReserved = false): string { + if (forceReserved) { + return "min-w-[3.75rem]"; + } + + return cn("min-w-[3.75rem]", ...FINE_HOVER_RESERVE_CLASSES); +} + +/** + * Absolute-positioned timestamp that sits under a pinned-card ellipsis slot: + * shown at rest on fine-pointer devices, hidden when the row reveals actions + * (or when the menu is open / on touch where the ellipsis stays visible). + */ +export function hoverRevealPinnedTimestampClassName( + forceHidden = false, +): string { + if (forceHidden) { + return "hidden"; + } + + return cn("hidden", ...FINE_HOVER_PINNED_TIMESTAMP_CLASSES); +}