diff --git a/apps/code/src/main/platform-adapters/electron-dialog.test.ts b/apps/code/src/main/platform-adapters/electron-dialog.test.ts new file mode 100644 index 0000000000..32175b7982 --- /dev/null +++ b/apps/code/src/main/platform-adapters/electron-dialog.test.ts @@ -0,0 +1,124 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const mockGetFocusedWindow = vi.hoisted(() => vi.fn()); +const mockGetAllWindows = vi.hoisted(() => vi.fn()); +const mockShowOpenDialog = vi.hoisted(() => vi.fn()); +const mockShowMessageBox = vi.hoisted(() => vi.fn()); + +vi.mock("inversify", () => ({ + injectable: () => (target: unknown) => target, +})); + +vi.mock("electron", () => ({ + BrowserWindow: { + getFocusedWindow: mockGetFocusedWindow, + getAllWindows: mockGetAllWindows, + }, + dialog: { + showOpenDialog: mockShowOpenDialog, + showMessageBox: mockShowMessageBox, + }, +})); + +import { ElectronDialog } from "./electron-dialog"; + +function makeWindow(overrides: { visible?: boolean } = {}) { + return { + isVisible: vi.fn(() => overrides.visible ?? true), + focus: vi.fn(), + }; +} + +describe("ElectronDialog.pickFile", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockShowOpenDialog.mockResolvedValue({ + canceled: false, + filePaths: ["/repo"], + }); + }); + + it("parents the picker to the focused window when one exists", async () => { + const focused = makeWindow(); + mockGetFocusedWindow.mockReturnValue(focused); + + const result = await new ElectronDialog().pickFile({ directories: true }); + + expect(result).toEqual(["/repo"]); + expect(mockShowOpenDialog).toHaveBeenCalledWith( + focused, + expect.any(Object), + ); + // A focused window is already on top, so we never reach the fallback windows. + expect(mockGetAllWindows).not.toHaveBeenCalled(); + }); + + it("falls back to a visible window and focuses it when nothing is focused", async () => { + mockGetFocusedWindow.mockReturnValue(null); + const hidden = makeWindow({ visible: false }); + const visible = makeWindow({ visible: true }); + mockGetAllWindows.mockReturnValue([hidden, visible]); + + await new ElectronDialog().pickFile({ directories: true }); + + expect(visible.focus).toHaveBeenCalledTimes(1); + // The dialog must be parented to the visible window, not opened unparented + // (which is what previously left it stuck behind/off-screen). + expect(mockShowOpenDialog).toHaveBeenCalledWith( + visible, + expect.any(Object), + ); + }); + + it("opens unparented only when there are no windows at all", async () => { + mockGetFocusedWindow.mockReturnValue(null); + mockGetAllWindows.mockReturnValue([]); + + await new ElectronDialog().pickFile({ directories: true }); + + expect(mockShowOpenDialog).toHaveBeenCalledWith(expect.any(Object)); + expect(mockShowOpenDialog.mock.calls[0]).toHaveLength(1); + }); +}); + +describe("ElectronDialog.confirm", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockShowMessageBox.mockResolvedValue({ response: 0 }); + }); + + it("parents the message box to the focused window when one exists", async () => { + const focused = makeWindow(); + mockGetFocusedWindow.mockReturnValue(focused); + + await new ElectronDialog().confirm({ + title: "Discard?", + message: "Discard changes?", + options: ["Cancel", "Discard"], + }); + + expect(mockShowMessageBox).toHaveBeenCalledWith( + focused, + expect.any(Object), + ); + }); + + it("falls back to a visible window and focuses it when nothing is focused", async () => { + mockGetFocusedWindow.mockReturnValue(null); + const hidden = makeWindow({ visible: false }); + const visible = makeWindow({ visible: true }); + mockGetAllWindows.mockReturnValue([hidden, visible]); + + await new ElectronDialog().confirm({ + title: "Discard?", + message: "Discard changes?", + options: ["Cancel", "Discard"], + }); + + expect(visible.focus).toHaveBeenCalledTimes(1); + expect(mockShowMessageBox).toHaveBeenCalledWith( + visible, + expect.any(Object), + ); + }); +}); diff --git a/apps/code/src/main/platform-adapters/electron-dialog.ts b/apps/code/src/main/platform-adapters/electron-dialog.ts index de1fd8f810..6a42dd37f1 100644 --- a/apps/code/src/main/platform-adapters/electron-dialog.ts +++ b/apps/code/src/main/platform-adapters/electron-dialog.ts @@ -18,6 +18,25 @@ function severityToType(severity?: DialogSeverity): MessageBoxOptions["type"] { return severity ?? "none"; } +/** + * Resolve the window to parent a native dialog to. + * + * `BrowserWindow.getFocusedWindow()` returns `null` whenever the app has no + * focused window — which happens during onboarding flows that hand focus to + * external windows (OS auth, the GitHub device-login browser tab, etc.). An + * unparented dialog can then open behind the app window or off-screen, looking + * completely unresponsive to the user. Fall back to a visible window and focus + * it so the dialog is always parented to and rendered on top of a real window. + */ +function resolveDialogParent(): BrowserWindow | null { + const focused = BrowserWindow.getFocusedWindow(); + if (focused) return focused; + const windows = BrowserWindow.getAllWindows(); + const parent = windows.find((w) => w.isVisible()) ?? windows[0] ?? null; + parent?.focus(); + return parent; +} + function buildProperties(options: PickFileOptions): OpenDialogProperty[] { const properties: OpenDialogProperty[] = ["treatPackageAsDirectory"]; if (options.filesAndDirectories) { @@ -37,7 +56,7 @@ function buildProperties(options: PickFileOptions): OpenDialogProperty[] { @injectable() export class ElectronDialog implements IDialog { public async confirm(options: ConfirmOptions): Promise { - const parent = BrowserWindow.getFocusedWindow(); + const parent = resolveDialogParent(); const electronOptions: MessageBoxOptions = { type: severityToType(options.severity), title: options.title, @@ -54,7 +73,7 @@ export class ElectronDialog implements IDialog { } public async pickFile(options: PickFileOptions): Promise { - const parent = BrowserWindow.getFocusedWindow(); + const parent = resolveDialogParent(); const electronOptions: OpenDialogOptions = { title: options.title, properties: buildProperties(options),