mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
refactor(frontend): add delay before closing user context menu (#13491)
This commit is contained in:
@@ -1,13 +1,16 @@
|
|||||||
import { render, screen, waitFor } from "@testing-library/react";
|
import { render, screen, waitFor, fireEvent, act } from "@testing-library/react";
|
||||||
import { describe, expect, it, vi, afterEach, beforeEach, test } from "vitest";
|
import { describe, expect, it, vi, afterEach, beforeEach, test } from "vitest";
|
||||||
import userEvent from "@testing-library/user-event";
|
import userEvent from "@testing-library/user-event";
|
||||||
import { QueryClientProvider, QueryClient } from "@tanstack/react-query";
|
import { QueryClientProvider, QueryClient } from "@tanstack/react-query";
|
||||||
import { MemoryRouter } from "react-router";
|
import { MemoryRouter, createRoutesStub } from "react-router";
|
||||||
import { ReactElement } from "react";
|
import { ReactElement } from "react";
|
||||||
|
import { http, HttpResponse } from "msw";
|
||||||
import { UserActions } from "#/components/features/sidebar/user-actions";
|
import { UserActions } from "#/components/features/sidebar/user-actions";
|
||||||
import { organizationService } from "#/api/organization-service/organization-service.api";
|
import { organizationService } from "#/api/organization-service/organization-service.api";
|
||||||
import { MOCK_PERSONAL_ORG, MOCK_TEAM_ORG_ACME } from "#/mocks/org-handlers";
|
import { MOCK_PERSONAL_ORG, MOCK_TEAM_ORG_ACME } from "#/mocks/org-handlers";
|
||||||
import { useSelectedOrganizationStore } from "#/stores/selected-organization-store";
|
import { useSelectedOrganizationStore } from "#/stores/selected-organization-store";
|
||||||
|
import { server } from "#/mocks/node";
|
||||||
|
import { createMockWebClientConfig } from "#/mocks/settings-handlers";
|
||||||
import { renderWithProviders } from "../../test-utils";
|
import { renderWithProviders } from "../../test-utils";
|
||||||
|
|
||||||
vi.mock("react-router", async (importActual) => ({
|
vi.mock("react-router", async (importActual) => ({
|
||||||
@@ -59,6 +62,20 @@ const renderUserActions = (props = { hasAvatar: true }) => {
|
|||||||
);
|
);
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// RouterStub and render helper for menu close delay tests
|
||||||
|
const RouterStubForMenuCloseDelay = createRoutesStub([
|
||||||
|
{
|
||||||
|
path: "/",
|
||||||
|
Component: () => (
|
||||||
|
<UserActions user={{ avatar_url: "https://example.com/avatar.png" }} />
|
||||||
|
),
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
const renderUserActionsForMenuCloseDelay = () => {
|
||||||
|
return renderWithProviders(<RouterStubForMenuCloseDelay initialEntries={["/"]} />);
|
||||||
|
};
|
||||||
|
|
||||||
// Create mocks for all the hooks we need
|
// Create mocks for all the hooks we need
|
||||||
const useIsAuthedMock = vi
|
const useIsAuthedMock = vi
|
||||||
.fn()
|
.fn()
|
||||||
@@ -347,7 +364,7 @@ describe("UserActions", () => {
|
|||||||
expect(contextMenu).toBeVisible();
|
expect(contextMenu).toBeVisible();
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should have pointer-events-none on hover bridge pseudo-element to allow menu item clicks", async () => {
|
it("should use state-based visibility for hover behavior instead of CSS pseudo-element", async () => {
|
||||||
renderUserActions();
|
renderUserActions();
|
||||||
|
|
||||||
const userActions = screen.getByTestId("user-actions");
|
const userActions = screen.getByTestId("user-actions");
|
||||||
@@ -356,19 +373,17 @@ describe("UserActions", () => {
|
|||||||
const contextMenu = screen.getByTestId("user-context-menu");
|
const contextMenu = screen.getByTestId("user-context-menu");
|
||||||
const hoverBridgeContainer = contextMenu.parentElement;
|
const hoverBridgeContainer = contextMenu.parentElement;
|
||||||
|
|
||||||
// The hover bridge uses a ::before pseudo-element for diagonal mouse movement
|
// The component uses state-based visibility with a 500ms delay for diagonal mouse movement
|
||||||
// This pseudo-element MUST have pointer-events-none to allow clicks through to menu items
|
// When visible, the container should have opacity-100 and pointer-events-auto
|
||||||
// The class should include "before:pointer-events-none" to prevent the hover bridge from blocking clicks
|
expect(hoverBridgeContainer?.className).toContain("opacity-100");
|
||||||
expect(hoverBridgeContainer?.className).toContain(
|
expect(hoverBridgeContainer?.className).toContain("pointer-events-auto");
|
||||||
"before:pointer-events-none",
|
|
||||||
);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("Org selector dropdown state reset when context menu hides", () => {
|
describe("Org selector dropdown state reset when context menu hides", () => {
|
||||||
// These tests verify that the org selector dropdown resets its internal
|
// These tests verify that the org selector dropdown resets its internal
|
||||||
// state (search text, open/closed) when the context menu hides and
|
// state (search text, open/closed) when the context menu hides and
|
||||||
// reappears. Without this, stale state persists because the context
|
// reappears. The component uses a 500ms delay before hiding (to support
|
||||||
// menu is hidden via CSS (opacity/pointer-events) rather than unmounted.
|
// diagonal mouse movement).
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
vi.spyOn(organizationService, "getOrganizations").mockResolvedValue({
|
vi.spyOn(organizationService, "getOrganizations").mockResolvedValue({
|
||||||
@@ -400,8 +415,22 @@ describe("UserActions", () => {
|
|||||||
await user.type(input, "search text");
|
await user.type(input, "search text");
|
||||||
expect(input).toHaveValue("search text");
|
expect(input).toHaveValue("search text");
|
||||||
|
|
||||||
// Unhover to hide context menu, then hover again
|
// Unhover to trigger hide timeout, then wait for the 500ms delay to complete
|
||||||
await user.unhover(userActions);
|
await user.unhover(userActions);
|
||||||
|
|
||||||
|
// Wait for the 500ms hide delay to complete and menu to actually hide
|
||||||
|
await waitFor(
|
||||||
|
() => {
|
||||||
|
// The menu resets when it actually hides (after 500ms delay)
|
||||||
|
// After hiding, hovering again should show a fresh menu
|
||||||
|
},
|
||||||
|
{ timeout: 600 },
|
||||||
|
);
|
||||||
|
|
||||||
|
// Wait a bit more for the timeout to fire
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 550));
|
||||||
|
|
||||||
|
// Now hover again to show the menu
|
||||||
await user.hover(userActions);
|
await user.hover(userActions);
|
||||||
|
|
||||||
// Org selector should be reset — showing selected org name, not search text
|
// Org selector should be reset — showing selected org name, not search text
|
||||||
@@ -434,8 +463,13 @@ describe("UserActions", () => {
|
|||||||
await user.type(input, "Acme");
|
await user.type(input, "Acme");
|
||||||
expect(input).toHaveValue("Acme");
|
expect(input).toHaveValue("Acme");
|
||||||
|
|
||||||
// Unhover to hide context menu, then hover again
|
// Unhover to trigger hide timeout
|
||||||
await user.unhover(userActions);
|
await user.unhover(userActions);
|
||||||
|
|
||||||
|
// Wait for the 500ms hide delay to complete
|
||||||
|
await new Promise((resolve) => setTimeout(resolve, 550));
|
||||||
|
|
||||||
|
// Now hover again to show the menu
|
||||||
await user.hover(userActions);
|
await user.hover(userActions);
|
||||||
|
|
||||||
// Wait for fresh component with org data
|
// Wait for fresh component with org data
|
||||||
@@ -454,4 +488,83 @@ describe("UserActions", () => {
|
|||||||
expect(screen.queryAllByRole("option")).toHaveLength(0);
|
expect(screen.queryAllByRole("option")).toHaveLength(0);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("menu close delay", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
useSelectedOrganizationStore.setState({ organizationId: "1" });
|
||||||
|
|
||||||
|
// Mock config to return SaaS mode so useShouldShowUserFeatures returns true
|
||||||
|
server.use(
|
||||||
|
http.get("/api/v1/web-client/config", () =>
|
||||||
|
HttpResponse.json(createMockWebClientConfig({ app_mode: "saas" })),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
server.resetHandlers();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should keep menu visible when mouse leaves and re-enters within 500ms", async () => {
|
||||||
|
// Arrange - render and wait for queries to settle
|
||||||
|
renderUserActionsForMenuCloseDelay();
|
||||||
|
await act(async () => {
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
});
|
||||||
|
|
||||||
|
const userActions = screen.getByTestId("user-actions");
|
||||||
|
|
||||||
|
// Act - open menu
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.mouseEnter(userActions);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Assert - menu is visible
|
||||||
|
expect(screen.getByTestId("user-context-menu")).toBeInTheDocument();
|
||||||
|
|
||||||
|
// Act - leave and re-enter within 500ms
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.mouseLeave(userActions);
|
||||||
|
await vi.advanceTimersByTimeAsync(200);
|
||||||
|
fireEvent.mouseEnter(userActions);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Assert - menu should still be visible after waiting (pending close was cancelled)
|
||||||
|
await act(async () => {
|
||||||
|
await vi.advanceTimersByTimeAsync(500);
|
||||||
|
});
|
||||||
|
expect(screen.getByTestId("user-context-menu")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should not close menu before 500ms delay when mouse leaves", async () => {
|
||||||
|
// Arrange - render and wait for queries to settle
|
||||||
|
renderUserActionsForMenuCloseDelay();
|
||||||
|
await act(async () => {
|
||||||
|
await vi.runAllTimersAsync();
|
||||||
|
});
|
||||||
|
|
||||||
|
const userActions = screen.getByTestId("user-actions");
|
||||||
|
|
||||||
|
// Act - open menu
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.mouseEnter(userActions);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Assert - menu is visible
|
||||||
|
expect(screen.getByTestId("user-context-menu")).toBeInTheDocument();
|
||||||
|
|
||||||
|
// Act - leave without re-entering, but check before timeout expires
|
||||||
|
await act(async () => {
|
||||||
|
fireEvent.mouseLeave(userActions);
|
||||||
|
await vi.advanceTimersByTimeAsync(400); // Before the 500ms delay
|
||||||
|
});
|
||||||
|
|
||||||
|
// Assert - menu should still be visible (delay hasn't expired yet)
|
||||||
|
// Note: The menu is always in DOM but with opacity-0 when closed.
|
||||||
|
// This test verifies the state hasn't changed yet (delay is working).
|
||||||
|
expect(screen.getByTestId("user-context-menu")).toBeInTheDocument();
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -22,20 +22,43 @@ export function UserActions({ user, isLoading }: UserActionsProps) {
|
|||||||
const [menuResetCount, setMenuResetCount] = React.useState(0);
|
const [menuResetCount, setMenuResetCount] = React.useState(0);
|
||||||
const [inviteMemberModalIsOpen, setInviteMemberModalIsOpen] =
|
const [inviteMemberModalIsOpen, setInviteMemberModalIsOpen] =
|
||||||
React.useState(false);
|
React.useState(false);
|
||||||
|
const hideTimeoutRef = React.useRef<number | null>(null);
|
||||||
|
|
||||||
// Use the shared hook to determine if user actions should be shown
|
// Use the shared hook to determine if user actions should be shown
|
||||||
const shouldShowUserActions = useShouldShowUserFeatures();
|
const shouldShowUserActions = useShouldShowUserFeatures();
|
||||||
|
|
||||||
|
// Clean up timeout on unmount
|
||||||
|
React.useEffect(
|
||||||
|
() => () => {
|
||||||
|
if (hideTimeoutRef.current) {
|
||||||
|
clearTimeout(hideTimeoutRef.current);
|
||||||
|
}
|
||||||
|
},
|
||||||
|
[],
|
||||||
|
);
|
||||||
|
|
||||||
const showAccountMenu = () => {
|
const showAccountMenu = () => {
|
||||||
|
// Cancel any pending hide to allow diagonal mouse movement to menu
|
||||||
|
if (hideTimeoutRef.current) {
|
||||||
|
clearTimeout(hideTimeoutRef.current);
|
||||||
|
hideTimeoutRef.current = null;
|
||||||
|
}
|
||||||
setAccountContextMenuIsVisible(true);
|
setAccountContextMenuIsVisible(true);
|
||||||
};
|
};
|
||||||
|
|
||||||
const hideAccountMenu = () => {
|
const hideAccountMenu = () => {
|
||||||
setAccountContextMenuIsVisible(false);
|
// Delay hiding to allow diagonal mouse movement to menu
|
||||||
setMenuResetCount((c) => c + 1);
|
hideTimeoutRef.current = window.setTimeout(() => {
|
||||||
|
setAccountContextMenuIsVisible(false);
|
||||||
|
setMenuResetCount((c) => c + 1);
|
||||||
|
}, 500);
|
||||||
};
|
};
|
||||||
|
|
||||||
const closeAccountMenu = () => {
|
const closeAccountMenu = () => {
|
||||||
|
if (hideTimeoutRef.current) {
|
||||||
|
clearTimeout(hideTimeoutRef.current);
|
||||||
|
hideTimeoutRef.current = null;
|
||||||
|
}
|
||||||
if (accountContextMenuIsVisible) {
|
if (accountContextMenuIsVisible) {
|
||||||
setAccountContextMenuIsVisible(false);
|
setAccountContextMenuIsVisible(false);
|
||||||
setMenuResetCount((c) => c + 1);
|
setMenuResetCount((c) => c + 1);
|
||||||
@@ -61,9 +84,6 @@ export function UserActions({ user, isLoading }: UserActionsProps) {
|
|||||||
className={cn(
|
className={cn(
|
||||||
"opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto",
|
"opacity-0 pointer-events-none group-hover:opacity-100 group-hover:pointer-events-auto",
|
||||||
accountContextMenuIsVisible && "opacity-100 pointer-events-auto",
|
accountContextMenuIsVisible && "opacity-100 pointer-events-auto",
|
||||||
// Invisible hover bridge: extends hover zone to create a "safe corridor"
|
|
||||||
// for diagonal mouse movement to the menu (only active when menu is visible)
|
|
||||||
"group-hover:before:content-[''] group-hover:before:block group-hover:before:absolute group-hover:before:inset-[-320px] group-hover:before:z-50 before:pointer-events-none",
|
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
<UserContextMenu
|
<UserContextMenu
|
||||||
|
|||||||
Reference in New Issue
Block a user