diff --git a/apps/desktop/src/renderer/src/components/pageview-tracker.test.tsx b/apps/desktop/src/renderer/src/components/pageview-tracker.test.tsx index cadefdb04..0d6c35a7e 100644 --- a/apps/desktop/src/renderer/src/components/pageview-tracker.test.tsx +++ b/apps/desktop/src/renderer/src/components/pageview-tracker.test.tsx @@ -116,7 +116,7 @@ describe("PageviewTracker", () => { expect(state.capturePageview).not.toHaveBeenCalled(); }); - it("fires pageview when a new tab is opened (openInNewTab / addTab)", () => { + it("fires pageview when a foreground tab is added (addTab path)", () => { state.byWorkspace = { acme: { activeTabId: "tA", @@ -128,7 +128,10 @@ describe("PageviewTracker", () => { const { rerender } = render(); state.capturePageview.mockClear(); - // Simulate openInNewTab("/acme/agents") → new tab tC added and activated. + // Simulate a foreground new-tab action (e.g. cmd+k → "Open in new tab" + // that explicitly switches focus) — tC is appended AND becomes active. + // Note: `openInNewTab` itself opens tabs in the background and does NOT + // change `activeTabId`, so it does not trigger this path on its own. state.byWorkspace = { acme: { activeTabId: "tC", diff --git a/apps/desktop/src/renderer/src/platform/navigation.test.tsx b/apps/desktop/src/renderer/src/platform/navigation.test.tsx new file mode 100644 index 000000000..a4d0c6e6e --- /dev/null +++ b/apps/desktop/src/renderer/src/platform/navigation.test.tsx @@ -0,0 +1,178 @@ +import { describe, expect, it, vi, beforeEach } from "vitest"; +import { render } from "@testing-library/react"; +import { useEffect } from "react"; + +// Shared in-memory state that the mocked tab store reads / mutates. The test +// records every method call so we can assert openInNewTab does NOT activate +// the new tab (i.e. setActiveTab is never invoked on the same-workspace path). +const state = vi.hoisted(() => ({ + activeWorkspaceSlug: "acme" as string | null, + byWorkspace: { + acme: { + activeTabId: "tA", + tabs: [{ id: "tA", path: "/acme/issues" }], + }, + } as Record< + string, + { activeTabId: string; tabs: { id: string; path: string }[] } + >, + openTab: vi.fn<(path: string, title?: string, icon?: string) => string>(), + setActiveTab: vi.fn<(tabId: string) => void>(), + switchWorkspace: vi.fn<(slug: string, openPath?: string) => void>(), +})); + +vi.mock("@/stores/tab-store", () => { + const store = { + get activeWorkspaceSlug() { + return state.activeWorkspaceSlug; + }, + get byWorkspace() { + return state.byWorkspace; + }, + openTab: state.openTab, + setActiveTab: state.setActiveTab, + switchWorkspace: state.switchWorkspace, + }; + const useTabStore = Object.assign( + (selector?: (s: typeof store) => unknown) => + selector ? selector(store) : store, + { getState: () => store }, + ); + const getActiveTab = () => { + const slug = state.activeWorkspaceSlug; + if (!slug) return null; + const group = state.byWorkspace[slug]; + if (!group) return null; + return group.tabs.find((t) => t.id === group.activeTabId) ?? null; + }; + const useActiveTabIdentity = () => ({ + slug: state.activeWorkspaceSlug, + tabId: state.activeWorkspaceSlug + ? (state.byWorkspace[state.activeWorkspaceSlug]?.activeTabId ?? null) + : null, + }); + const useActiveTabRouter = () => null; + const resolveRouteIcon = () => "File"; + return { + useTabStore, + getActiveTab, + useActiveTabIdentity, + useActiveTabRouter, + resolveRouteIcon, + }; +}); + +vi.mock("@/stores/window-overlay-store", () => ({ + useWindowOverlayStore: Object.assign( + () => null, + { getState: () => ({ overlay: null, open: vi.fn(), close: vi.fn() }) }, + ), +})); + +vi.mock("@multica/core/auth", () => ({ + useAuthStore: Object.assign( + () => null, + { getState: () => ({ logout: vi.fn() }) }, + ), +})); + +vi.mock("@multica/core/paths", () => ({ + isReservedSlug: (s: string) => + ["login", "workspaces", "invite", "onboarding", "invitations"].includes(s), +})); + +// DesktopNavigationProvider reads window.desktopAPI.runtimeConfig synchronously. +beforeEach(() => { + state.openTab.mockReset(); + state.setActiveTab.mockReset(); + state.switchWorkspace.mockReset(); + state.openTab.mockImplementation(() => "tNew"); + state.activeWorkspaceSlug = "acme"; + state.byWorkspace = { + acme: { + activeTabId: "tA", + tabs: [{ id: "tA", path: "/acme/issues" }], + }, + }; + Object.defineProperty(window, "desktopAPI", { + configurable: true, + value: { + runtimeConfig: { ok: true, config: { appUrl: "https://app.example" } }, + }, + }); +}); + +import { + DesktopNavigationProvider, + TabNavigationProvider, +} from "./navigation"; +import { useNavigation } from "@multica/views/navigation"; + +function captureAdapter(onAdapter: (adapter: ReturnType) => void) { + function Probe() { + const nav = useNavigation(); + useEffect(() => { + onAdapter(nav); + }, [nav]); + return null; + } + return Probe; +} + +describe("DesktopNavigationProvider.openInNewTab", () => { + it("opens a background tab (no setActiveTab) for a same-workspace path", () => { + let adapter: ReturnType | null = null; + const Probe = captureAdapter((a) => { + adapter = a; + }); + render( + + + , + ); + expect(adapter).not.toBeNull(); + adapter!.openInNewTab!("/acme/agents", "Agents"); + expect(state.openTab).toHaveBeenCalledWith("/acme/agents", "Agents", "File"); + expect(state.setActiveTab).not.toHaveBeenCalled(); + expect(state.switchWorkspace).not.toHaveBeenCalled(); + }); + + it("delegates to switchWorkspace for a cross-workspace path", () => { + let adapter: ReturnType | null = null; + const Probe = captureAdapter((a) => { + adapter = a; + }); + render( + + + , + ); + adapter!.openInNewTab!("/butter/inbox"); + expect(state.switchWorkspace).toHaveBeenCalledWith("butter", "/butter/inbox"); + expect(state.openTab).not.toHaveBeenCalled(); + expect(state.setActiveTab).not.toHaveBeenCalled(); + }); +}); + +describe("TabNavigationProvider.openInNewTab", () => { + it("opens a background tab (no setActiveTab) for a same-workspace path", () => { + let adapter: ReturnType | null = null; + const Probe = captureAdapter((a) => { + adapter = a; + }); + const fakeRouter = { + state: { location: { pathname: "/acme/issues", search: "" } }, + subscribe: () => () => {}, + navigate: vi.fn(), + } as unknown as Parameters[0]["router"]; + render( + + + , + ); + adapter!.openInNewTab!("/acme/agents", "Agents"); + expect(state.openTab).toHaveBeenCalledWith("/acme/agents", "Agents", "File"); + expect(state.setActiveTab).not.toHaveBeenCalled(); + expect(state.switchWorkspace).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/desktop/src/renderer/src/platform/navigation.tsx b/apps/desktop/src/renderer/src/platform/navigation.tsx index bc78278dd..4b23a55f4 100644 --- a/apps/desktop/src/renderer/src/platform/navigation.tsx +++ b/apps/desktop/src/renderer/src/platform/navigation.tsx @@ -180,7 +180,9 @@ export function DesktopNavigationProvider({ searchParams: new URLSearchParams(location.search), openInNewTab: (path: string, title?: string) => { // Cross-workspace "open in new tab" switches workspace and opens - // the path there; same-workspace just adds a tab in the current group. + // the path there; same-workspace just adds a background tab. The + // type contract (NavigationAdapter.openInNewTab) is explicitly a + // background-tab operation — focus stays on the current tab. const slug = extractWorkspaceSlug(path); const store = useTabStore.getState(); if (slug && slug !== store.activeWorkspaceSlug) { @@ -188,8 +190,7 @@ export function DesktopNavigationProvider({ return; } const icon = resolveRouteIcon(path); - const tabId = store.openTab(path, title ?? path, icon); - if (tabId) store.setActiveTab(tabId); + store.openTab(path, title ?? path, icon); }, getShareableUrl: (path: string) => `${appUrl}${path}`, }), @@ -249,8 +250,7 @@ export function TabNavigationProvider({ return; } const icon = resolveRouteIcon(path); - const tabId = store.openTab(path, title ?? path, icon); - if (tabId) store.setActiveTab(tabId); + store.openTab(path, title ?? path, icon); }, getShareableUrl: (path: string) => `${appUrl}${path}`, }), diff --git a/packages/views/editor/attachment-preview-modal.test.tsx b/packages/views/editor/attachment-preview-modal.test.tsx index 37b3bf8d3..03e006393 100644 --- a/packages/views/editor/attachment-preview-modal.test.tsx +++ b/packages/views/editor/attachment-preview-modal.test.tsx @@ -398,7 +398,7 @@ describe("AttachmentPreviewModal — open-in-new-tab (HTML only)", () => { expect(screen.getByTitle("Open in new tab")).toBeTruthy(); }); - it("invokes navigation.openInNewTab with the preview path when available (desktop)", async () => { + it("invokes navigation.openInNewTab with the preview path and closes the modal (desktop)", async () => { getAttachmentTextContentMock.mockResolvedValueOnce({ text: "

hi

", originalContentType: "text/html", @@ -407,11 +407,12 @@ describe("AttachmentPreviewModal — open-in-new-tab (HTML only)", () => { filename: "report.html", content_type: "text/html", }); + const onClose = vi.fn(); render( {}} + onClose={onClose} />, ); fireEvent.click(screen.getByTitle("Open in new tab")); @@ -419,9 +420,10 @@ describe("AttachmentPreviewModal — open-in-new-tab (HTML only)", () => { "/acme/attachments/att-1/preview?name=report.html", "report.html", ); + expect(onClose).toHaveBeenCalledTimes(1); }); - it("falls back to window.open against the shareable URL on web", async () => { + it("falls back to window.open against the shareable URL and closes the modal (web)", async () => { navState.hasOpenInNewTab = false; getAttachmentTextContentMock.mockResolvedValueOnce({ text: "

hi

", @@ -434,11 +436,12 @@ describe("AttachmentPreviewModal — open-in-new-tab (HTML only)", () => { filename: "report.html", content_type: "text/html", }); + const onClose = vi.fn(); render( {}} + onClose={onClose} />, ); fireEvent.click(screen.getByTitle("Open in new tab")); @@ -448,6 +451,7 @@ describe("AttachmentPreviewModal — open-in-new-tab (HTML only)", () => { "_blank", "noopener,noreferrer", ); + expect(onClose).toHaveBeenCalledTimes(1); }); it("does not render the new-tab button for non-HTML kinds", () => { diff --git a/packages/views/editor/attachment-preview-modal.tsx b/packages/views/editor/attachment-preview-modal.tsx index ed9bb863d..078be8cb0 100644 --- a/packages/views/editor/attachment-preview-modal.tsx +++ b/packages/views/editor/attachment-preview-modal.tsx @@ -222,10 +222,11 @@ export function AttachmentPreviewModal({ const path = `${paths.workspace(slug).attachmentPreview(state.attachmentId)}${nameQuery}`; if (navigation.openInNewTab) { navigation.openInNewTab(path, state.filename); - return; + } else { + const url = navigation.getShareableUrl(path); + window.open(url, "_blank", "noopener,noreferrer"); } - const url = navigation.getShareableUrl(path); - window.open(url, "_blank", "noopener,noreferrer"); + onClose(); }; if (!open || typeof document === "undefined") return null;