From a3dfd2439386cc4675598d9854da5052bfce0fbb Mon Sep 17 00:00:00 2001 From: Jiayuan Zhang Date: Thu, 13 Aug 2026 01:47:20 +0800 Subject: [PATCH] MUL-6082: fix Board assignee avatar interactions (#6854) * fix(issues): route board avatars to actor profiles Co-authored-by: multica-agent * fix(issues): cancel deferred card navigation Co-authored-by: multica-agent * fix(issues): open assignee picker from board avatars Co-authored-by: multica-agent --------- Co-authored-by: Lambda Co-authored-by: multica-agent --- .../common/actor-avatar-profile-link.test.tsx | 63 +++++++ packages/views/common/actor-avatar.tsx | 9 +- packages/views/common/deferred-popup.test.tsx | 39 +++++ packages/views/common/deferred-popup.tsx | 8 +- .../board-card-assignee-picker.test.tsx | 163 ++++++++++++++++++ .../views/issues/components/board-card.tsx | 1 + 6 files changed, 274 insertions(+), 9 deletions(-) create mode 100644 packages/views/common/deferred-popup.test.tsx create mode 100644 packages/views/issues/components/board-card-assignee-picker.test.tsx diff --git a/packages/views/common/actor-avatar-profile-link.test.tsx b/packages/views/common/actor-avatar-profile-link.test.tsx index 8e27cfba7..cac7b90e8 100644 --- a/packages/views/common/actor-avatar-profile-link.test.tsx +++ b/packages/views/common/actor-avatar-profile-link.test.tsx @@ -10,8 +10,10 @@ */ import { afterEach, describe, expect, it, vi } from "vitest"; import { fireEvent, render, screen } from "@testing-library/react"; +import { AppLink } from "../navigation/app-link"; import { NavigationProvider } from "../navigation/context"; import type { NavigationAdapter } from "../navigation/types"; +import { DeferredPopup } from "./deferred-popup"; vi.mock("@multica/core/workspace/hooks", () => ({ useActorName: () => ({ @@ -72,6 +74,47 @@ function renderAvatar(adapter: NavigationAdapter) { ); } +function PickerWrapper({ children }: { children: React.ReactNode }) { + const stop = (event: React.SyntheticEvent) => { + event.stopPropagation(); + event.preventDefault(); + }; + return ( +
+ {children} +
+ ); +} + +function renderCardPicker(adapter: NavigationAdapter) { + return render( + + + + + + Change assignee + + } + > + {(open) => ( + + Picker mounted + + )} + + + + , + ); +} + describe("ActorAvatar profile link", () => { afterEach(() => { vi.restoreAllMocks(); @@ -88,6 +131,26 @@ describe("ActorAvatar profile link", () => { expect(open).not.toHaveBeenCalled(); }); + it("opens the deferred picker while cancelling the enclosing card link", () => { + const push = vi.fn(); + renderCardPicker(makeAdapter({ push })); + + expect(fireEvent.click(screen.getByText("Change assignee"))).toBe(false); + expect(screen.getByTestId("mounted-picker")).toHaveAttribute("data-open", "true"); + expect(push).not.toHaveBeenCalled(); + }); + + it("yields to the picker by default without leaking native card navigation", () => { + const push = vi.fn(); + const { container } = renderCardPicker(makeAdapter({ push })); + const avatar = container.querySelector('[data-slot="avatar"]'); + + expect(avatar).not.toBeNull(); + expect(fireEvent.click(avatar!)).toBe(false); + expect(screen.getByTestId("mounted-picker")).toHaveAttribute("data-open", "true"); + expect(push).not.toHaveBeenCalled(); + }); + it("uses openInNewTab for cmd/ctrl click when available (desktop)", () => { const push = vi.fn(); const openInNewTab = vi.fn(); diff --git a/packages/views/common/actor-avatar.tsx b/packages/views/common/actor-avatar.tsx index ac8cb4b75..4f9560f54 100644 --- a/packages/views/common/actor-avatar.tsx +++ b/packages/views/common/actor-avatar.tsx @@ -150,12 +150,9 @@ export function ActorAvatar({ /** * Not an `` on purpose: the avatar is often composed inside menu items, - * options and buttons, where it must yield the click to the surrounding - * control untouched (no preventDefault — a real anchor would need one to - * suppress its own navigation, and a pre-prevented click can change how the - * host control behaves). The cost is no native context menu; modifier and - * middle clicks are implemented here instead, with the same intent semantics - * as AppLink. + * options and buttons, where it yields the click to the surrounding control. + * The cost is no native context menu, so modifier and middle clicks are + * implemented here with the same intent semantics as AppLink. */ function ActorAvatarProfileLink({ href, diff --git a/packages/views/common/deferred-popup.test.tsx b/packages/views/common/deferred-popup.test.tsx new file mode 100644 index 000000000..480d5608e --- /dev/null +++ b/packages/views/common/deferred-popup.test.tsx @@ -0,0 +1,39 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import { DeferredPopup } from "./deferred-popup"; + +function popupContent(open: boolean) { + return ( + + Popup mounted + + ); +} + +describe("DeferredPopup", () => { + it("opens its default trigger and cancels an enclosing link's native navigation", () => { + render( + + Open picker}> + {popupContent} + + , + ); + + expect(fireEvent.click(screen.getByText("Open picker"))).toBe(false); + expect(screen.getByTestId("mounted-popup")).toHaveAttribute("data-open", "true"); + }); + + it("does the same for a custom trigger element", () => { + render( + + Open priority}> + {popupContent} + + , + ); + + expect(fireEvent.click(screen.getByText("Open priority"))).toBe(false); + expect(screen.getByTestId("mounted-popup")).toHaveAttribute("data-open", "true"); + }); +}); diff --git a/packages/views/common/deferred-popup.tsx b/packages/views/common/deferred-popup.tsx index b4db1b23e..6cf25fcc4 100644 --- a/packages/views/common/deferred-popup.tsx +++ b/packages/views/common/deferred-popup.tsx @@ -19,9 +19,10 @@ import { cloneElement, useState, type ReactElement, type ReactNode } from "react * swaps the element mid-gesture (no pointerenter/pointerdown warming), * every event of the gesture lands on a node that is still attached — * both for real pointers and for synthetic sequences (tests, assistive - * tech). The in-flight click is stopped from propagating so the popup's - * just-mounted outside-press dismissal doesn't treat it as an outside - * click and close the popover in the same breath. + * tech). The in-flight click is prevented and stopped so an enclosing link + * cannot perform native navigation, and the popup's just-mounted + * outside-press dismissal doesn't treat it as an outside click and close + * the popover in the same breath. * - `Enter`/`Space` do the same for keyboard. * * Only for uncontrolled usages: callers that pass `open`/`onOpenChange`/ @@ -70,6 +71,7 @@ export function DeferredPopup({ }; const handlers = { onClick: (e: React.MouseEvent) => { + e.preventDefault(); e.stopPropagation(); mountOpen(); }, diff --git a/packages/views/issues/components/board-card-assignee-picker.test.tsx b/packages/views/issues/components/board-card-assignee-picker.test.tsx new file mode 100644 index 000000000..50e7f6f60 --- /dev/null +++ b/packages/views/issues/components/board-card-assignee-picker.test.tsx @@ -0,0 +1,163 @@ +import { fireEvent, render, screen } from "@testing-library/react"; +import { describe, expect, it, vi } from "vitest"; +import type { Issue, IssueAssigneeType } from "@multica/core/types"; +import { AppLink, NavigationProvider, type NavigationAdapter } from "../../navigation"; +import { + IssueSurfaceActionsProvider, + type IssueSurfaceActions, +} from "../surface/actions-context"; +import { BoardCardContent } from "./board-card"; + +vi.mock("@tanstack/react-query", async (importOriginal) => ({ + ...(await importOriginal()), + useQuery: () => ({ data: [] }), +})); + +vi.mock("@multica/core/hooks", () => ({ + useWorkspaceId: () => "ws-1", +})); + +vi.mock("@multica/core/properties", () => ({ + propertyListOptions: () => ({ queryKey: ["properties"] }), +})); + +vi.mock("@multica/core/auth", () => ({ + useAuthStore: (selector: (state: { user: { id: string } }) => unknown) => + selector({ user: { id: "viewer-1" } }), +})); + +vi.mock("@multica/core/agents", () => ({ + isAgentRuntimeBound: () => true, + useAgentPresenceDetail: () => ({ availability: "offline", workload: null }), +})); + +vi.mock("@multica/core/paths", () => ({ + useCurrentWorkspace: () => ({ id: "ws-1", slug: "acme" }), + useWorkspacePaths: () => ({ + memberDetail: (id: string) => `/acme/members/${id}`, + agentDetail: (id: string) => `/acme/agents/${id}`, + squadDetail: (id: string) => `/acme/squads/${id}`, + }), +})); + +const viewState = vi.hoisted(() => ({ + cardProperties: { + priority: false, + description: false, + assignee: true, + startDate: true, + dueDate: true, + project: false, + childProgress: false, + labels: false, + }, + cardPropertyIds: [], +})); + +vi.mock("@multica/core/issues/stores/view-store-context", () => ({ + useViewStore: (selector: (state: typeof viewState) => unknown) => selector(viewState), +})); + +vi.mock("@multica/core/workspace/hooks", () => ({ + useActorName: () => ({ + getActorName: (type: string) => `Assigned ${type}`, + getActorInitials: () => "AA", + getActorAvatarUrl: () => null, + }), +})); + +vi.mock("../../i18n", () => ({ + useT: () => ({ t: () => "Translated" }), + useTimeAgo: () => () => "now", +})); + +vi.mock("../../agents/components/agent-profile-card", () => ({ + AgentProfileCard: () => null, +})); + +vi.mock("../../agents/components/agent-live-peek-card", () => ({ + AgentLivePeekCard: () => null, +})); + +vi.mock("../../members/member-profile-card", () => ({ + MemberProfileCard: () => null, +})); + +vi.mock("../../squads/components/squad-profile-card", () => ({ + SquadProfileCard: () => null, +})); + +vi.mock("./issue-agent-activity-indicator", () => ({ + IssueAgentActivityIndicator: () => null, +})); + +const navigation: NavigationAdapter = { + push: vi.fn(), + replace: vi.fn(), + back: vi.fn(), + pathname: "/acme/issues", + searchParams: new URLSearchParams(), + getShareableUrl: (path) => `https://app.example${path}`, +}; + +const actions: IssueSurfaceActions = { + isPending: false, + createIssue: vi.fn(), + updateIssue: vi.fn(), + moveIssue: vi.fn(), + batchUpdate: vi.fn().mockResolvedValue(undefined), + batchDelete: vi.fn().mockResolvedValue(undefined), +}; + +function makeIssue(assigneeType: IssueAssigneeType): Issue { + return { + id: `issue-${assigneeType}`, + workspace_id: "ws-1", + number: 6082, + identifier: "MUL-6082", + title: "Fix Board assignee interaction", + description: null, + status: "todo", + priority: "none", + assignee_type: assigneeType, + assignee_id: `${assigneeType}-1`, + creator_type: "member", + creator_id: "member-1", + parent_issue_id: null, + project_id: null, + position: 1, + stage: null, + start_date: "2026-08-12", + due_date: "2026-08-13", + metadata: {}, + properties: {}, + labels: [], + created_at: "2026-08-12T00:00:00Z", + updated_at: "2026-08-12T00:00:00Z", + }; +} + +describe("BoardCardContent assignee picker", () => { + it.each(["member", "agent", "squad"])( + "opens the picker from an avatar-only %s assignee without navigating the card", + (assigneeType) => { + const issue = makeIssue(assigneeType); + const { container } = render( + + + + + + + , + ); + const avatar = container.querySelector('[data-slot="avatar"]'); + + expect(avatar).not.toBeNull(); + expect(avatar!.closest('[role="link"]')).toBeNull(); + expect(fireEvent.click(avatar!)).toBe(false); + expect(screen.getByRole("textbox")).toBeInTheDocument(); + expect(navigation.push).not.toHaveBeenCalled(); + }, + ); +}); diff --git a/packages/views/issues/components/board-card.tsx b/packages/views/issues/components/board-card.tsx index 45fe5e8cc..5bffb35c0 100644 --- a/packages/views/issues/components/board-card.tsx +++ b/packages/views/issues/components/board-card.tsx @@ -141,6 +141,7 @@ export const BoardCardContent = memo(function BoardCardContent({ actorId={issue.assignee_id!} size="sm" enableHoverCard + profileLink={false} className="shrink-0" /> {assigneeName && (