diff --git a/packages/app/e2e/browser/chat-find.spec.ts b/packages/app/e2e/browser/chat-find.spec.ts index 2a223f37e2..a9f1fbdff5 100644 --- a/packages/app/e2e/browser/chat-find.spec.ts +++ b/packages/app/e2e/browser/chat-find.spec.ts @@ -1,3 +1,4 @@ +import type { TestInfo } from "@playwright/test"; import { expect, test, type Page } from "../support/fixtures"; import { openAgentRoute, seedMockAgentWorkspace } from "../support/helpers/mock-agent"; import { installDaemonWebSocketGate } from "../support/helpers/daemon-websocket-gate"; @@ -15,6 +16,9 @@ import { import { openCommandCenter, closeCommandCenter } from "../support/helpers/command-center"; +const MAC_USER_AGENT = + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/130.0.0.0 Safari/537.36"; + const RESPONSE = 'İ😀hello **world**; hello *world*.\n\n[hello & world](https://example.com/hidden-destination)\n\n```ts\nconst a = "a.b";\n```\n\n- first cell\n- second cell'; @@ -212,30 +216,94 @@ test("finds text beyond the normal render cap and restores the cap when Find clo } }); -for (const shortcut of ["Control+f", "Meta+f"]) { - test(`opens and refocuses Find with ${shortcut} from the composer`, async ({ - page, - }, testInfo) => { +async function findFromComposer(page: Page, shortcut: string, testInfo: TestInfo) { + const agent = await seedMockAgentWorkspace({ + repoPrefix: "chat-find-composer-", + title: "Chat Find from composer", + initialPrompt: "Render the response", + featureValues: { mockAssistantResponse: RESPONSE }, + }); + try { + await agent.client.waitForFinish(agent.agentId, 15_000); + await openAgentRoute(page, agent); + await searchFromComposerWithoutLosingDraft(page, shortcut); + await refocusFindAndReplaceQuery(page, shortcut); + await page.screenshot({ path: testInfo.outputPath("find-from-composer.png") }); + await closeFind(page); + await expectCommandCenterOwnsFindShortcut(page, shortcut); + await fillComposerDraft(page, "Draft after closing Find"); + await expectComposerDraft(page, "Draft after closing Find"); + } finally { + await agent.cleanup(); + } +} + +test("opens and refocuses Find with Control+f from the composer", async ({ page }, testInfo) => { + await findFromComposer(page, "Control+f", testInfo); +}); + +test.describe("macOS", () => { + test.use({ userAgent: MAC_USER_AGENT }); + + test("opens and refocuses Find with Meta+f from the composer", async ({ page }, testInfo) => { + await findFromComposer(page, "Meta+f", testInfo); + }); + + // Control+F moves the caret forward on macOS, so chat Find must leave it alone. + test("leaves Control+f to the composer", async ({ page }) => { const agent = await seedMockAgentWorkspace({ - repoPrefix: "chat-find-composer-", - title: "Chat Find from composer", + repoPrefix: "chat-find-mac-control-f-", + title: "Chat Find macOS Control+f", initialPrompt: "Render the response", featureValues: { mockAssistantResponse: RESPONSE }, }); try { await agent.client.waitForFinish(agent.agentId, 15_000); await openAgentRoute(page, agent); - await searchFromComposerWithoutLosingDraft(page, shortcut); - await refocusFindAndReplaceQuery(page, shortcut); - await page.screenshot({ path: testInfo.outputPath("find-from-composer.png") }); - await closeFind(page); - await expectCommandCenterOwnsFindShortcut(page, shortcut); - await fillComposerDraft(page, "Draft after closing Find"); - await expectComposerDraft(page, "Draft after closing Find"); + await fillComposerDraft(page, "Keep this draft"); + await expectComposerFocused(page); + await recordComposerKeydown(page); + await page.keyboard.press("Control+f"); + await expect + .poll(() => recordedComposerFindKeydown(page)) + .toEqual([{ key: "f", defaultPrevented: false }]); + await expect(query(page)).toHaveCount(0); + await expectComposerFocused(page); + await expectComposerDraft(page, "Keep this draft"); } finally { await agent.cleanup(); } }); +}); + +interface RecordedKeydown { + key: string; + defaultPrevented: boolean; +} + +/** + * Chat Find listens on `document` in the capture phase, so the composer only ever + * sees a Find keystroke after Find decided about it. That makes `defaultPrevented` + * on the composer the evidence of who owns the key. + */ +async function recordComposerKeydown(page: Page) { + await composerLocator(page).evaluate((element) => { + const store = window as unknown as { __findKeydown: RecordedKeydown[] }; + store.__findKeydown = []; + element.addEventListener("keydown", (event) => { + store.__findKeydown.push({ + key: (event as KeyboardEvent).key, + defaultPrevented: event.defaultPrevented, + }); + }); + }); +} + +async function recordedComposerFindKeydown(page: Page): Promise { + const recorded = await page.evaluate( + () => (window as unknown as { __findKeydown: RecordedKeydown[] }).__findKeydown, + ); + return recorded.filter((entry) => entry.key.toLowerCase() === "f"); } async function openFindFromComposer(page: Page, shortcut: string) { diff --git a/packages/app/e2e/browser/pane-find.spec.ts b/packages/app/e2e/browser/pane-find.spec.ts index 45ba51cc2d..113a582e0e 100644 --- a/packages/app/e2e/browser/pane-find.spec.ts +++ b/packages/app/e2e/browser/pane-find.spec.ts @@ -25,14 +25,21 @@ async function findInSource(page: Page, text: string) { await expect(query(page)).toBeFocused(); await query(page).fill(text); } +function querySelection(page: Page) { + return query(page).evaluate((input: HTMLInputElement) => [ + input.selectionStart, + input.selectionEnd, + ]); +} +/** The query box keeps the caret where the user left it, with nothing reselected. */ +async function expectQueryCaretKept(page: Page, text: string) { + await expect.poll(() => querySelection(page)).toEqual([text.length, text.length]); + await expect(query(page)).toHaveValue(text); +} async function expectQuerySelected(page: Page, text: string) { await expect(query(page)).toBeFocused(); await expect(query(page)).toHaveValue(text); - await expect - .poll(() => - query(page).evaluate((input: HTMLInputElement) => [input.selectionStart, input.selectionEnd]), - ) - .toEqual([0, text.length]); + await expect.poll(() => querySelection(page)).toEqual([0, text.length]); } async function closeFind(page: Page) { await query(page).press("Escape"); @@ -428,3 +435,41 @@ test("Go to line keeps its dialog with Find closed and open", async ({ await expect(status(page)).toHaveText("11 matches"); }); }); + +test.describe("macOS", () => { + test.use({ + userAgent: + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/130.0.0.0 Safari/537.36", + }); + + // Control+F moves the caret forward on macOS, so Find must only claim Command+F. + test("opens Find with Meta+f and leaves Control+f to text editing", async ({ + page, + withWorkspace, + }, testInfo) => { + const workspace = await withWorkspace({ prefix: "pane-find-mac-" }); + await writeFile(path.join(workspace.repoPath, "source.txt"), "needle first\nneedle second\n"); + await workspace.navigateTo(); + await openSource(page, "source.txt"); + + await source(page).focus(); + await source(page).press("Control+f"); + await expect(query(page)).toHaveCount(0); + + await source(page).press("Meta+f"); + await expect(query(page)).toBeFocused(); + await query(page).fill("needle"); + await expect(status(page)).toHaveText("1 of 2"); + + await test.step("Control+f inside the query box does not reclaim the field", async () => { + await query(page).press("End"); + await query(page).press("Control+f"); + await expectQueryCaretKept(page, "needle"); + }); + + await page.screenshot({ path: testInfo.outputPath("mac-find.png") }); + await query(page).press("Meta+f"); + await expectQuerySelected(page, "needle"); + await closeFind(page); + }); +}); diff --git a/packages/app/src/agent-stream/chat-find/index.web.tsx b/packages/app/src/agent-stream/chat-find/index.web.tsx index 19539cff77..bbe9a69930 100644 --- a/packages/app/src/agent-stream/chat-find/index.web.tsx +++ b/packages/app/src/agent-stream/chat-find/index.web.tsx @@ -11,7 +11,7 @@ import { import { View, Text } from "react-native"; import { StyleSheet } from "react-native-unistyles"; import { useTranslation } from "react-i18next"; -import { PaneFind, type PaneFindHandle } from "@/pane-find"; +import { PaneFind, findShortcutPlatform, isFindShortcut, type PaneFindHandle } from "@/pane-find"; import { Button } from "@/components/ui/button"; import { usePaneFocus } from "@/panels/pane-context"; import { useRetainedPanelActive } from "@/components/retained-panel"; @@ -92,12 +92,7 @@ export function ChatFind({ const onKeyDown = (event: KeyboardEvent) => { if (event.defaultPrevented || hasActiveWebOverlay() || isImeComposingKeyboardEvent(event)) return; - if ( - (event.metaKey || event.ctrlKey) && - !event.altKey && - !event.shiftKey && - event.key.toLowerCase() === "f" - ) { + if (isFindShortcut(event, findShortcutPlatform())) { event.preventDefault(); model.open(); widget.current?.focus(); diff --git a/packages/app/src/file-pane/find/index.web.tsx b/packages/app/src/file-pane/find/index.web.tsx index 7136542415..dedf58389f 100644 --- a/packages/app/src/file-pane/find/index.web.tsx +++ b/packages/app/src/file-pane/find/index.web.tsx @@ -10,7 +10,7 @@ import { EditorView } from "@codemirror/view"; import { View, type View as ViewInstance } from "react-native"; import { StyleSheet } from "react-native-unistyles"; import { useTranslation } from "react-i18next"; -import { PaneFind, type PaneFindHandle } from "@/pane-find"; +import { PaneFind, findShortcutPlatform, isFindShortcut, type PaneFindHandle } from "@/pane-find"; import { usePaneFocus } from "@/panels/pane-context"; import { hasActiveWebOverlay } from "@/lib/overlay-root"; import { useRetainedPanelActive } from "@/components/retained-panel"; @@ -41,12 +41,7 @@ export function FileFind({ function onKeyDown(event: KeyboardEvent) { if (event.defaultPrevented || hasActiveWebOverlay() || isImeComposingKeyboardEvent(event)) return; - if ( - (event.metaKey || event.ctrlKey) && - !event.altKey && - !event.shiftKey && - event.key.toLowerCase() === "f" - ) { + if (isFindShortcut(event, findShortcutPlatform())) { event.preventDefault(); model.open(editor.current); widget.current?.focus(); diff --git a/packages/app/src/pane-find/find-shortcut.test.ts b/packages/app/src/pane-find/find-shortcut.test.ts new file mode 100644 index 0000000000..5d64bdee8d --- /dev/null +++ b/packages/app/src/pane-find/find-shortcut.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it } from "vitest"; +import { isFindShortcut } from "./find-shortcut"; + +const MAC = { isMac: true }; +const NON_MAC = { isMac: false }; + +function keydown( + overrides: Partial>, +) { + return { + key: "f", + metaKey: false, + ctrlKey: false, + shiftKey: false, + altKey: false, + ...overrides, + }; +} + +describe("on macOS", () => { + it("opens Find with Command+F", () => { + expect(isFindShortcut(keydown({ metaKey: true }), MAC)).toBe(true); + }); + + it("leaves Control+F to text editing", () => { + expect(isFindShortcut(keydown({ ctrlKey: true }), MAC)).toBe(false); + }); + + it("ignores Control+Command+F", () => { + expect(isFindShortcut(keydown({ metaKey: true, ctrlKey: true }), MAC)).toBe(false); + }); + + it("ignores Shift and Alt variants", () => { + expect(isFindShortcut(keydown({ metaKey: true, shiftKey: true }), MAC)).toBe(false); + expect(isFindShortcut(keydown({ metaKey: true, altKey: true }), MAC)).toBe(false); + }); +}); + +describe("off macOS", () => { + it("opens Find with Control+F", () => { + expect(isFindShortcut(keydown({ ctrlKey: true }), NON_MAC)).toBe(true); + }); + + it("ignores Meta+F", () => { + expect(isFindShortcut(keydown({ metaKey: true }), NON_MAC)).toBe(false); + }); + + it("ignores Control+Meta+F", () => { + expect(isFindShortcut(keydown({ ctrlKey: true, metaKey: true }), NON_MAC)).toBe(false); + }); + + it("ignores Shift and Alt variants", () => { + expect(isFindShortcut(keydown({ ctrlKey: true, shiftKey: true }), NON_MAC)).toBe(false); + expect(isFindShortcut(keydown({ ctrlKey: true, altKey: true }), NON_MAC)).toBe(false); + }); +}); + +it("matches an uppercase F from a shifted keyboard layout", () => { + expect(isFindShortcut(keydown({ key: "F", metaKey: true }), MAC)).toBe(true); + expect(isFindShortcut(keydown({ key: "F", ctrlKey: true }), NON_MAC)).toBe(true); +}); + +it("ignores other keys", () => { + expect(isFindShortcut(keydown({ key: "g", metaKey: true }), MAC)).toBe(false); + expect(isFindShortcut(keydown({ key: "g", ctrlKey: true }), NON_MAC)).toBe(false); +}); diff --git a/packages/app/src/pane-find/find-shortcut.ts b/packages/app/src/pane-find/find-shortcut.ts new file mode 100644 index 0000000000..f4c1c2b57b --- /dev/null +++ b/packages/app/src/pane-find/find-shortcut.ts @@ -0,0 +1,24 @@ +export interface FindShortcutPlatform { + isMac: boolean; +} + +/** + * The one Find keystroke policy: Command+F on macOS, Control+F everywhere else. + * + * Control+F is a macOS text-editing binding that moves the caret forward, so a + * surface that also claims it there eats a keystroke the text editor owns. + * + * The platform is a parameter rather than a lookup because this module is + * imported by the terminal emulator runtime, which is bundled on its own into + * the terminal WebView and cannot load React Native. App surfaces pass + * `findShortcutPlatform()` from `@/pane-find`. + */ +export function isFindShortcut( + event: Pick, + platform: FindShortcutPlatform, +): boolean { + const primaryModifier = platform.isMac + ? event.metaKey && !event.ctrlKey + : event.ctrlKey && !event.metaKey; + return primaryModifier && !event.shiftKey && !event.altKey && event.key.toLowerCase() === "f"; +} diff --git a/packages/app/src/pane-find/index.tsx b/packages/app/src/pane-find/index.tsx index dac417023d..761ac187e1 100644 --- a/packages/app/src/pane-find/index.tsx +++ b/packages/app/src/pane-find/index.tsx @@ -33,6 +33,15 @@ import { paneContentToolbarIconSize, ToolbarButton } from "@/components/ui/pane- import { useIsCompactFormFactor } from "@/constants/layout"; import { isWeb } from "@/constants/platform"; import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime"; +import { getShortcutOs } from "@/utils/shortcut-platform"; +import { isFindShortcut, type FindShortcutPlatform } from "./find-shortcut"; + +export { isFindShortcut, type FindShortcutPlatform } from "./find-shortcut"; + +/** The platform every Find surface judges the shortcut against. */ +export function findShortcutPlatform(): FindShortcutPlatform { + return { isMac: getShortcutOs() === "mac" }; +} const TextInput = withUnistyles(EditingTextInput, (theme) => ({ placeholderTextColor: theme.colors.foregroundMuted, @@ -166,12 +175,14 @@ export const PaneFind = forwardRef(function PaneF keyCode?: number; }; if (isImeComposingKeyboardEvent(key)) return; - if ( - (key.metaKey || key.ctrlKey) && - !key.altKey && - !key.shiftKey && - key.key.toLowerCase() === "f" - ) { + const shortcut = { + key: key.key, + metaKey: key.metaKey === true, + ctrlKey: key.ctrlKey === true, + shiftKey: key.shiftKey === true, + altKey: key.altKey === true, + }; + if (isFindShortcut(shortcut, findShortcutPlatform())) { // RN Web inputs stop keydown before the pane's document listener. event.preventDefault(); event.stopPropagation(); diff --git a/packages/app/src/terminal/find/index.tsx b/packages/app/src/terminal/find/index.tsx index 6184317ad7..c8ecab5a04 100644 --- a/packages/app/src/terminal/find/index.tsx +++ b/packages/app/src/terminal/find/index.tsx @@ -10,12 +10,11 @@ import { import { View, type LayoutChangeEvent } from "react-native"; import { StyleSheet } from "react-native-unistyles"; import { useTranslation } from "react-i18next"; -import { PaneFind, type PaneFindHandle } from "@/pane-find"; +import { PaneFind, findShortcutPlatform, isFindShortcut, type PaneFindHandle } from "@/pane-find"; import { isWeb } from "@/constants/platform"; import { hasActiveWebOverlay } from "@/lib/overlay-root"; import { isImeComposingKeyboardEvent } from "@/utils/keyboard-ime"; import type { TerminalEmulatorHandle } from "@/components/terminal-emulator-contract"; -import { isFindShortcut } from "../runtime/terminal-find-shortcut"; import type { TerminalFindResult } from "../runtime/terminal-emulator-runtime"; export interface TerminalPaneFindHandle { @@ -65,7 +64,7 @@ export const TerminalFind = forwardRef< function onKeyDown(event: KeyboardEvent) { if (event.defaultPrevented || hasActiveWebOverlay() || isImeComposingKeyboardEvent(event)) return; - if (isFindShortcut(event) && terminal.current?.find) { + if (isFindShortcut(event, findShortcutPlatform()) && terminal.current?.find) { event.preventDefault(); show(); } diff --git a/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts b/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts index 094c059231..efa6048dee 100644 --- a/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts +++ b/packages/app/src/terminal/runtime/terminal-emulator-runtime.ts @@ -28,7 +28,8 @@ import { type TerminalLocalFileLinkSource, type TerminalLocalFileLinkTarget, } from "../local-links/terminal-local-link-provider"; -import { isMac, isFindShortcut } from "./terminal-find-shortcut"; +import { isFindShortcut, type FindShortcutPlatform } from "@/pane-find/find-shortcut"; +import { isMacUserAgent } from "@/utils/mac-user-agent"; import { resolveTerminalFontFamily, resolveTerminalFontSize } from "./terminal-font"; export type TerminalOutputData = Uint8Array; @@ -178,7 +179,7 @@ function withOverviewRulerBorderHidden(theme: ITheme): ITheme { } export class TerminalEmulatorRuntime { - constructor(private readonly options: { isMac: boolean } = { isMac }) {} + constructor(private readonly options: FindShortcutPlatform = { isMac: isMacUserAgent() }) {} private callbacks: TerminalEmulatorRuntimeCallbacks = {}; private pendingModifiers: PendingTerminalModifiers = { diff --git a/packages/app/src/terminal/runtime/terminal-find-shortcut.ts b/packages/app/src/terminal/runtime/terminal-find-shortcut.ts deleted file mode 100644 index 64753b6b2c..0000000000 --- a/packages/app/src/terminal/runtime/terminal-find-shortcut.ts +++ /dev/null @@ -1,14 +0,0 @@ -export const isMac = - typeof navigator !== "undefined" && - (/Macintosh|Mac OS/i.test(navigator.userAgent ?? "") || - /Mac/i.test((navigator as Navigator & { platform?: string }).platform ?? "")); - -export function isFindShortcut( - event: Pick, - platform = { isMac }, -): boolean { - const primaryModifier = platform.isMac - ? event.metaKey && !event.ctrlKey - : event.ctrlKey && !event.metaKey; - return primaryModifier && !event.shiftKey && !event.altKey && event.key.toLowerCase() === "f"; -} diff --git a/packages/app/src/terminal/webview/terminal-emulator-webview-html.ts b/packages/app/src/terminal/webview/terminal-emulator-webview-html.ts index 804131db2f..efa77dda73 100644 --- a/packages/app/src/terminal/webview/terminal-emulator-webview-html.ts +++ b/packages/app/src/terminal/webview/terminal-emulator-webview-html.ts @@ -2,4 +2,4 @@ // Do not edit by hand. export const terminalEmulatorWebViewHtml = - '\n\n \n \n \n \n \n \n \n'; + '\n\n \n \n \n \n \n \n \n'; diff --git a/packages/app/src/utils/mac-user-agent.ts b/packages/app/src/utils/mac-user-agent.ts new file mode 100644 index 0000000000..45fb0b104c --- /dev/null +++ b/packages/app/src/utils/mac-user-agent.ts @@ -0,0 +1,14 @@ +/** + * What the browser says about the platform. `getShortcutOs()` is the app's + * answer for keyboard policy and consults native and Electron first; this is the + * browser half of it, kept free of React Native imports so the terminal WebView + * bundle can use it too. + */ +export function isMacUserAgent(): boolean { + if (typeof navigator === "undefined") return false; + const userAgent = navigator.userAgent ?? ""; + const platform = (navigator as Navigator & { platform?: string }).platform ?? ""; + return ( + /Macintosh|Mac OS|iPhone|iPad|iPod/i.test(userAgent) || /Mac|iPhone|iPad|iPod/i.test(platform) + ); +} diff --git a/packages/app/src/utils/shortcut-platform.ts b/packages/app/src/utils/shortcut-platform.ts index 6c14983411..41d55796eb 100644 --- a/packages/app/src/utils/shortcut-platform.ts +++ b/packages/app/src/utils/shortcut-platform.ts @@ -2,16 +2,12 @@ import { Platform } from "react-native"; import { getIsElectronRuntimeMac } from "@/constants/layout"; import type { ShortcutOs } from "@/utils/format-shortcut"; import { isNative } from "@/constants/platform"; +import { isMacUserAgent } from "@/utils/mac-user-agent"; export function getShortcutOs(): ShortcutOs { if (isNative) { return Platform.OS === "ios" ? "mac" : "non-mac"; } if (getIsElectronRuntimeMac()) return "mac"; - if (typeof navigator === "undefined") return "non-mac"; - const ua = navigator.userAgent ?? ""; - const platform = (navigator as Navigator & { platform?: string }).platform ?? ""; - const isApple = - /Macintosh|Mac OS|iPhone|iPad|iPod/i.test(ua) || /Mac|iPhone|iPad|iPod/i.test(platform); - return isApple ? "mac" : "non-mac"; + return isMacUserAgent() ? "mac" : "non-mac"; }