diff --git a/packages/views/editor/title-editor.shortcut.test.tsx b/packages/views/editor/title-editor.shortcut.test.tsx new file mode 100644 index 00000000000..402f2310135 --- /dev/null +++ b/packages/views/editor/title-editor.shortcut.test.tsx @@ -0,0 +1,87 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { render, screen } from "@testing-library/react"; +import { + configureShortcutPlatform, + createShortcutChord, + useShortcutStore, +} from "@multica/core/shortcuts"; +import { TitleEditor } from "./title-editor"; + +// Every other editor test mocks `@tiptap/react`, which means nothing verifies +// that the submit-shortcut extension actually wins over the title keymap in a +// real ProseMirror instance — the one genuinely new interaction in MUL-4931. +// This file deliberately runs the real editor to pin that ordering down. + +vi.mock("../i18n", () => ({ + useT: () => ({ t: () => "" }), +})); + +function pressEnter(options: KeyboardEventInit = {}) { + const target = document.querySelector(".title-editor") as HTMLElement; + target.dispatchEvent( + new KeyboardEvent("keydown", { + key: "Enter", + bubbles: true, + cancelable: true, + ...options, + }), + ); +} + +describe("TitleEditor send chord (real editor)", () => { + beforeEach(() => { + configureShortcutPlatform("macos"); + useShortcutStore.setState({ overrides: {} }); + }); + + afterEach(() => { + useShortcutStore.setState({ overrides: {} }); + }); + + it("fires onSubmitShortcut on the default Cmd+Enter chord", async () => { + const onSubmitShortcut = vi.fn(); + render(); + await screen.findByText("A title"); + + pressEnter({ metaKey: true }); + + expect(onSubmitShortcut).toHaveBeenCalledTimes(1); + }); + + it("leaves plain Enter to the keymap, so it never creates", async () => { + const onSubmitShortcut = vi.fn(); + render(); + await screen.findByText("A title"); + + pressEnter(); + + expect(onSubmitShortcut).not.toHaveBeenCalled(); + }); + + it("still refuses plain Enter when the user binds send to Enter", async () => { + useShortcutStore.setState({ overrides: { send: createShortcutChord("Enter") } }); + const onSubmitShortcut = vi.fn(); + render(); + await screen.findByText("A title"); + + pressEnter(); + + // The single-line keymap owns Enter; inheriting a chat-shaped binding here + // would create from a half-typed title. + expect(onSubmitShortcut).not.toHaveBeenCalled(); + }); + + it("does not fire the chord for hosts that only pass onSubmit", async () => { + const onSubmit = vi.fn(); + render(); + await screen.findByText("A title"); + + // create-project / autopilot-dialog rely on plain Enter submitting and must + // not gain a second trigger from this change. + pressEnter({ metaKey: true }); + expect(onSubmit).not.toHaveBeenCalled(); + + pressEnter(); + expect(onSubmit).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/views/editor/title-editor.test.tsx b/packages/views/editor/title-editor.test.tsx index ce3232ba3a4..f2f40904149 100644 --- a/packages/views/editor/title-editor.test.tsx +++ b/packages/views/editor/title-editor.test.tsx @@ -39,7 +39,8 @@ vi.mock("@tiptap/react", () => ({ EditorContent: () =>
, })); -import { TitleEditor } from "./title-editor"; +import { createShortcutChord } from "@multica/core/shortcuts"; +import { TitleEditor, titleShortcutSubmitAllowed } from "./title-editor"; describe("TitleEditor", () => { beforeEach(() => { @@ -136,3 +137,23 @@ describe("TitleEditor", () => { expect(mockSetContent).toHaveBeenCalledWith("", { emitUpdate: false }); }); }); + +// MUL-4931 — which `send` bindings the title's shortcut-submit path honors. +describe("titleShortcutSubmitAllowed", () => { + it("allows the default Mod+Enter chord", () => { + expect( + titleShortcutSubmitAllowed(createShortcutChord("Enter", { primary: true })), + ).toBe(true); + }); + + it("refuses plain Enter, which the single-line keymap already owns", () => { + // A user may bind `send` to plain Enter for chat. The title must not + // inherit it — Enter there means "done", and creating from a half-typed + // title is the misfire #5532 removed. + expect(titleShortcutSubmitAllowed(createShortcutChord("Enter"))).toBe(false); + }); + + it("refuses an unbound send action", () => { + expect(titleShortcutSubmitAllowed(null)).toBe(false); + }); +}); diff --git a/packages/views/editor/title-editor.tsx b/packages/views/editor/title-editor.tsx index 5f70c9616e5..bdb1fca31b9 100644 --- a/packages/views/editor/title-editor.tsx +++ b/packages/views/editor/title-editor.tsx @@ -7,8 +7,14 @@ import { Document } from "@tiptap/extension-document"; import { Paragraph } from "@tiptap/extension-paragraph"; import { Text } from "@tiptap/extension-text"; import Placeholder from "@tiptap/extension-placeholder"; +import { + getShortcut, + isPlainShortcut, + type ShortcutChord, +} from "@multica/core/shortcuts"; import { cn } from "@multica/ui/lib/utils"; import { useT } from "../i18n"; +import { createSubmitShortcutExtension } from "./extensions/submit-shortcut"; import "./title-editor.css"; // --------------------------------------------------------------------------- @@ -21,6 +27,18 @@ interface TitleEditorProps { className?: string; autoFocus?: boolean; onSubmit?: () => void; + /** + * Fires on the configured `send` chord, independent of `onSubmit`'s plain + * Enter path. Hosts that submit on plain Enter pass `onSubmit`; hosts that + * want an explicit chord (create-issue, MUL-4931) pass this instead. + * + * Plain Enter is deliberately never a trigger here even when `send` is + * configured as plain Enter: the keymap below already owns that key for + * "finish editing", and this single-line editor has no newline to trade it + * against. Shadowing it would silently create from a half-typed title — + * exactly what #5532 removed. + */ + onSubmitShortcut?: () => void; onBlur?: (value: string) => void; onChange?: (value: string) => void; /** @@ -55,6 +73,23 @@ const SingleLineDocument = Document.extend({ // Keyboard shortcuts: Enter → submit, Escape → blur // --------------------------------------------------------------------------- +/** + * Whether `onSubmitShortcut` may fire for the configured `send` chord. + * + * Plain Enter is excluded on purpose. The keymap below already owns that key + * ("finish editing"), and unlike a prose editor a single-line title has no + * newline to trade it against — so a `send` configured as plain Enter would + * turn every Enter into a create from a half-typed title. That is exactly the + * misfire #5532 removed, so the title keeps plain Enter inert and only honors + * an explicit chord. Hosts wanting plain-Enter submit still pass `onSubmit`. + */ +export function titleShortcutSubmitAllowed( + sendShortcut: ShortcutChord | null, +): boolean { + if (!sendShortcut) return false; + return !isPlainShortcut(sendShortcut, "Enter"); +} + function createTitleKeymap(opts: { onSubmitRef: React.RefObject<(() => void) | undefined>; }) { @@ -89,6 +124,7 @@ const TitleEditor = forwardRef( className, autoFocus = false, onSubmit, + onSubmitShortcut, onBlur, onChange, onReady, @@ -97,15 +133,22 @@ const TitleEditor = forwardRef( ) { const { t } = useT("editor"); const onSubmitRef = useRef(onSubmit); + const onSubmitShortcutRef = useRef(onSubmitShortcut); const onBlurRef = useRef(onBlur); const onChangeRef = useRef(onChange); const onReadyRef = useRef(onReady); onSubmitRef.current = onSubmit; + onSubmitShortcutRef.current = onSubmitShortcut; onBlurRef.current = onBlur; onChangeRef.current = onChange; onReadyRef.current = onReady; + // `useEditor` reads `extensions` once at mount and no host toggles this + // prop over its lifetime, so pin the mount-time answer rather than let the + // extension list depend on whichever render happened to create the editor. + const shortcutSubmitEnabled = useRef(onSubmitShortcut !== undefined).current; + const editor = useEditor({ immediatelyRender: false, content: defaultValue @@ -120,6 +163,21 @@ const TitleEditor = forwardRef( showOnlyCurrent: false, }), createTitleKeymap({ onSubmitRef }), + // Added last so its ProseMirror plugin sits ahead of the keymap above + // — same placement the ContentEditor extension list relies on. It + // brings the configured `send` chord, the IME guards, and key-repeat + // protection with it, none of which a hand-rolled keydown would have. + ...(shortcutSubmitEnabled + ? [ + createSubmitShortcutExtension(() => { + const fn = onSubmitShortcutRef.current; + if (!fn) return false; + if (!titleShortcutSubmitAllowed(getShortcut("send"))) return false; + fn(); + return true; + }), + ] + : []), ], editorProps: { attributes: { diff --git a/packages/views/locales/en/settings.json b/packages/views/locales/en/settings.json index 1da9dc61fc1..2447dce9330 100644 --- a/packages/views/locales/en/settings.json +++ b/packages/views/locales/en/settings.json @@ -70,7 +70,7 @@ "createIssue": { "label": "Create issue", "description": "Open your preferred issue creation flow." }, "toggleSidebar": { "label": "Toggle sidebar", "description": "Show or hide the main sidebar." }, "findInIssue": { "label": "Find in issue", "description": "Search within the open issue detail." }, - "send": { "label": "Send", "description": "Send chat messages, comments, replies, feedback, and prompts." }, + "send": { "label": "Send", "description": "Send chat messages, comments, replies, feedback, and prompts, and create issues in the create dialog." }, "goInbox": { "label": "Go to Inbox", "description": "Open the workspace inbox." }, "goChat": { "label": "Go to Chat", "description": "Open workspace chat." }, "goMyIssues": { "label": "Go to My Issues", "description": "Open issues assigned to you." }, diff --git a/packages/views/locales/ja/settings.json b/packages/views/locales/ja/settings.json index 9775c159410..41c23dee77e 100644 --- a/packages/views/locales/ja/settings.json +++ b/packages/views/locales/ja/settings.json @@ -67,7 +67,7 @@ "createIssue": { "label": "Issueを作成", "description": "優先するissue作成フローを開きます。" }, "toggleSidebar": { "label": "サイドバーを切り替え", "description": "メインサイドバーの表示を切り替えます。" }, "findInIssue": { "label": "Issue内を検索", "description": "開いているissueの詳細を検索します。" }, - "send": { "label": "送信", "description": "チャット、コメント、返信、フィードバック、プロンプトを送信します。" }, + "send": { "label": "送信", "description": "チャット、コメント、返信、フィードバック、プロンプトを送信し、作成ダイアログで issue を作成します。" }, "goInbox": { "label": "受信トレイへ移動", "description": "ワークスペースの受信トレイを開きます。" }, "goChat": { "label": "チャットへ移動", "description": "ワークスペースのチャットを開きます。" }, "goMyIssues": { "label": "自分のIssuesへ移動", "description": "自分に割り当てられたissueを開きます。" }, diff --git a/packages/views/locales/ko/settings.json b/packages/views/locales/ko/settings.json index f14aafa549a..715f2877f09 100644 --- a/packages/views/locales/ko/settings.json +++ b/packages/views/locales/ko/settings.json @@ -67,7 +67,7 @@ "createIssue": { "label": "Issue 만들기", "description": "선호하는 issue 만들기 흐름을 엽니다." }, "toggleSidebar": { "label": "사이드바 전환", "description": "기본 사이드바를 표시하거나 숨깁니다." }, "findInIssue": { "label": "Issue에서 찾기", "description": "열린 issue 상세 내용에서 검색합니다." }, - "send": { "label": "보내기", "description": "채팅, 댓글, 답글, 피드백 및 프롬프트를 보냅니다." }, + "send": { "label": "보내기", "description": "채팅, 댓글, 답글, 피드백 및 프롬프트를 보내고, 생성 대화상자에서 이슈를 만듭니다." }, "goInbox": { "label": "받은 편지함으로 이동", "description": "워크스페이스 받은 편지함을 엽니다." }, "goChat": { "label": "채팅으로 이동", "description": "워크스페이스 채팅을 엽니다." }, "goMyIssues": { "label": "내 Issues로 이동", "description": "나에게 할당된 issue를 엽니다." }, diff --git a/packages/views/locales/zh-Hans/settings.json b/packages/views/locales/zh-Hans/settings.json index 74dfab00b57..e80a7cc1b8d 100644 --- a/packages/views/locales/zh-Hans/settings.json +++ b/packages/views/locales/zh-Hans/settings.json @@ -70,7 +70,7 @@ "createIssue": { "label": "新建 issue", "description": "打开你常用的 issue 创建流程。" }, "toggleSidebar": { "label": "显示或隐藏侧边栏", "description": "切换主侧边栏的显示状态。" }, "findInIssue": { "label": "在 issue 中查找", "description": "搜索当前打开的 issue 详情。" }, - "send": { "label": "发送", "description": "发送聊天消息、评论、回复、反馈和提示词。" }, + "send": { "label": "发送", "description": "发送聊天消息、评论、回复、反馈和提示词，并在创建弹窗中创建 issue。" }, "goInbox": { "label": "前往收件箱", "description": "打开工作区收件箱。" }, "goChat": { "label": "前往聊天", "description": "打开工作区聊天。" }, "goMyIssues": { "label": "前往我的 issue", "description": "打开分配给你的 issue。" }, diff --git a/packages/views/modals/create-issue.test.tsx b/packages/views/modals/create-issue.test.tsx index a3b2fcdafc9..ea9bc3eaa96 100644 --- a/packages/views/modals/create-issue.test.tsx +++ b/packages/views/modals/create-issue.test.tsx @@ -243,7 +243,7 @@ vi.mock("../editor", async () => { const uploadGate = await vi.importActual( "../editor/use-upload-gate", ); - const ContentEditor = forwardRef(({ defaultValue, onUpdate, onUploadFile, onUploadingChange, placeholder, attachments }: any, ref: any) => { + const ContentEditor = forwardRef(({ defaultValue, onUpdate, onSubmit, onUploadFile, onUploadingChange, placeholder, attachments }: any, ref: any) => { const valueRef = useRef(defaultValue || ""); const [value, setValue] = useState(defaultValue || ""); // Mirrors the real editor's `uploading` node attrs: the placeholder is in @@ -279,26 +279,32 @@ vi.mock("../editor", async () => { setValue(e.target.value); onUpdate?.(e.target.value); }} + // Stands in for createSubmitShortcutExtension with the default + // `send` binding (Mod+Enter). Plain Enter stays a newline. + onKeyDown={(e) => { + if ((e.metaKey || e.ctrlKey) && e.key === "Enter") onSubmit?.(); + }} /> ); }); ContentEditor.displayName = "ContentEditor"; - return { - ...uploadGate, - useEditorUpload: () => ({ - uploadWithToast: mockUploadWithToast, - upload: vi.fn(), - uploading: false, - }), - useFileDropZone: () => ({ isDragOver: false, dropZoneProps: {} }), - FileDropOverlay: () => null, - ContentEditor, - TitleEditor: ({ defaultValue, placeholder, onChange, onSubmit }: any) => { + // Mirrors the real split: plain Enter is the keymap's `onSubmit` path, the + // configured `send` chord (default Mod+Enter) is `onSubmitShortcut`. The + // real component never routes plain Enter to onSubmitShortcut. + const TitleEditor = forwardRef( + ({ defaultValue, placeholder, onChange, onSubmit, onSubmitShortcut }: any, ref: any) => { const [value, setValue] = useState(defaultValue || ""); + const inputRef = useRef(null); + useImperativeHandle(ref, () => ({ + getText: () => value, + focus: () => inputRef.current?.focus(), + focusAtCoords: () => inputRef.current?.focus(), + })); return ( { @@ -306,11 +312,27 @@ vi.mock("../editor", async () => { onChange?.(e.target.value); }} onKeyDown={(e) => { - if (e.key === "Enter") onSubmit?.(); + if (e.key !== "Enter") return; + if (e.metaKey || e.ctrlKey) onSubmitShortcut?.(); + else onSubmit?.(); }} /> ); }, + ); + TitleEditor.displayName = "TitleEditor"; + + return { + ...uploadGate, + useEditorUpload: () => ({ + uploadWithToast: mockUploadWithToast, + upload: vi.fn(), + uploading: false, + }), + useFileDropZone: () => ({ isDragOver: false, dropZoneProps: {} }), + FileDropOverlay: () => null, + ContentEditor, + TitleEditor, }; }); @@ -1204,7 +1226,10 @@ describe("CreateIssueModal", () => { ); }); - it("never submits manual create from Enter in the title", async () => { + // Plain Enter in the title was removed as a create trigger in #5532 — it + // fired from a half-typed title. MUL-4931 adds the explicit `send` chord + // alongside it; plain Enter must stay inert. + it("never submits manual create from plain Enter in the title", async () => { const user = userEvent.setup(); renderManual(); const title = screen.getByPlaceholderText("Issue title"); @@ -1215,6 +1240,21 @@ describe("CreateIssueModal", () => { expect(mockCreateIssue).not.toHaveBeenCalled(); }); + it("blocks the title send chord while an upload is in flight", async () => { + const user = userEvent.setup(); + renderManual(); + const title = screen.getByPlaceholderText("Issue title"); + await user.type(title, "Has a screenshot"); + + startPendingUpload(); + + // The chord bypasses the button, so the handler's own gate is what stops + // this from serializing a description whose image hasn't landed yet. + fireEvent.keyDown(title, { key: "Enter", metaKey: true }); + await Promise.resolve(); + expect(mockCreateIssue).not.toHaveBeenCalled(); + }); + it("blocks Switch to Agent while an upload is in flight", async () => { const user = userEvent.setup(); const onSwitchMode = vi.fn(); @@ -1231,4 +1271,145 @@ describe("CreateIssueModal", () => { expect(onSwitchMode).not.toHaveBeenCalled(); }); }); + + // MUL-4931 — manual create had no submit shortcut at all, while agent create + // has had one all along. + describe("send shortcut", () => { + function renderManual() { + return renderModal( + , + ); + } + + it("creates from the send chord in the title", async () => { + const user = userEvent.setup(); + renderManual(); + const title = screen.getByPlaceholderText("Issue title"); + await user.type(title, "Shortcut from title"); + + fireEvent.keyDown(title, { key: "Enter", metaKey: true }); + + await waitFor(() => expect(mockCreateIssue).toHaveBeenCalledTimes(1)); + expect(mockCreateIssue).toHaveBeenCalledWith( + expect.objectContaining({ title: "Shortcut from title" }), + ); + }); + + it("creates from the send chord in the description", async () => { + const user = userEvent.setup(); + renderManual(); + await user.type(screen.getByPlaceholderText("Issue title"), "Shortcut from body"); + const description = screen.getByPlaceholderText("Add description..."); + await user.type(description, "Body text"); + + fireEvent.keyDown(description, { key: "Enter", ctrlKey: true }); + + await waitFor(() => expect(mockCreateIssue).toHaveBeenCalledTimes(1)); + expect(mockCreateIssue).toHaveBeenCalledWith( + expect.objectContaining({ + title: "Shortcut from body", + description: "Body text", + }), + ); + }); + + it("leaves plain Enter in the description as a newline, not a create", async () => { + const user = userEvent.setup(); + renderManual(); + await user.type(screen.getByPlaceholderText("Issue title"), "Still typing"); + + fireEvent.keyDown(screen.getByPlaceholderText("Add description..."), { key: "Enter" }); + await Promise.resolve(); + expect(mockCreateIssue).not.toHaveBeenCalled(); + }); + + it("focuses the title instead of silently doing nothing when it is empty", async () => { + const user = userEvent.setup(); + renderManual(); + const description = screen.getByPlaceholderText("Add description..."); + await user.type(description, "Body but no title"); + + fireEvent.keyDown(description, { key: "Enter", metaKey: true }); + + await Promise.resolve(); + expect(mockCreateIssue).not.toHaveBeenCalled(); + // The shortcut path can't rely on the button's tooltip, so it has to say + // where the problem is some other way. + expect(screen.getByPlaceholderText("Issue title")).toHaveFocus(); + }); + + it("creates once when the chord is pressed twice in the same tick", async () => { + const user = userEvent.setup(); + // Hold the create open so both presses land inside the in-flight window. + let release!: (v: unknown) => void; + mockCreateIssue.mockImplementationOnce( + () => new Promise((resolve) => { release = resolve; }), + ); + renderManual(); + const title = screen.getByPlaceholderText("Issue title"); + await user.type(title, "Double tap"); + + // Both presses are dispatched inside ONE act, so React cannot re-render + // between them and the second handler still closes over `submitting === + // false`. `fireEvent` would flush in between and hide the race — only a + // ref that flips synchronously stops the second create here. + await act(async () => { + const press = () => + title.dispatchEvent( + new KeyboardEvent("keydown", { key: "Enter", metaKey: true, bubbles: true }), + ); + press(); + press(); + }); + + await act(async () => { + release({ id: "issue-1", identifier: "MUL-1", title: "Double tap", status: "todo" }); + }); + expect(mockCreateIssue).toHaveBeenCalledTimes(1); + }); + + it("renders the send keycaps on Create without renaming the button", async () => { + const user = userEvent.setup(); + renderManual(); + + // Accessible name must stay the label alone — the keycaps are decorative. + expect(screen.getByRole("button", { name: "Create Issue" })).toBeInTheDocument(); + expect(document.querySelector("[data-slot='shortcut-keycaps']")).toBeInTheDocument(); + + // And the affordance survives the empty → filled transition. + await user.type(screen.getByPlaceholderText("Issue title"), "Now valid"); + expect(screen.getByRole("button", { name: "Create Issue" })).toBeInTheDocument(); + expect(document.querySelector("[data-slot='shortcut-keycaps']")).toBeInTheDocument(); + }); + + it("keeps Create focusable via aria-disabled while the title is empty", () => { + renderManual(); + const createButton = screen.getByRole("button", { name: "Create Issue" }); + + // Native `disabled` would drop it out of the tab order, hiding the + // "Enter a title to create" tooltip from keyboard and SR users. + expect(createButton).toHaveAttribute("aria-disabled", "true"); + expect(createButton).not.toBeDisabled(); + createButton.focus(); + expect(createButton).toHaveFocus(); + }); + + it("carries its own disabled visuals, since the Button base only styles native disabled", () => { + renderManual(); + const createButton = screen.getByRole("button", { name: "Create Issue" }); + + // Without these the control reads as a live primary button while + // aria-disabled. `pointer-events-none` is deliberately absent: it would + // kill the tooltip hover and the click that focuses the title. + expect(createButton.className).toContain("aria-disabled:opacity-50"); + expect(createButton.className).toContain("aria-disabled:cursor-not-allowed"); + expect(createButton.className).toContain("aria-disabled:active:translate-y-0"); + expect(createButton.className).not.toContain("aria-disabled:pointer-events-none"); + }); + }); }); diff --git a/packages/views/modals/create-issue.tsx b/packages/views/modals/create-issue.tsx index faf6cac1a49..208bf4789ec 100644 --- a/packages/views/modals/create-issue.tsx +++ b/packages/views/modals/create-issue.tsx @@ -50,7 +50,9 @@ import { import { Tooltip, TooltipTrigger, TooltipContent, TooltipProvider } from "@multica/ui/components/ui/tooltip"; import { Button } from "@multica/ui/components/ui/button"; import { Switch } from "@multica/ui/components/ui/switch"; -import { ContentEditor, type ContentEditorRef, TitleEditor, useFileDropZone, FileDropOverlay, useUploadGate, useEditorUpload } from "../editor"; +import { ContentEditor, type ContentEditorRef, TitleEditor, type TitleEditorRef, useFileDropZone, FileDropOverlay, useUploadGate, useEditorUpload } from "../editor"; +import { useShortcut } from "@multica/core/shortcuts"; +import { ShortcutKeycaps } from "../common/shortcut-keycaps"; import { StatusIcon, StatusPicker, PriorityIcon, PriorityPicker, StagePicker, AssigneePicker, StartDatePicker, DueDatePicker, LabelPicker } from "../issues/components"; import { maxSiblingStage } from "../issues/components/pickers/stage-picker"; import { ProjectPicker } from "../projects/components/project-picker"; @@ -230,8 +232,10 @@ export function ManualCreatePanel({ const setKeepOpen = useQuickCreateStore((s) => s.setKeepOpen); const manualFields = useIssueCreateSettingsStore((s) => s.manualCreateFields); + const sendShortcut = useShortcut("send"); const [title, setTitle] = useState(draft.title); const [formResetKey, setFormResetKey] = useState(0); + const titleEditorRef = useRef(null); const descEditorRef = useRef(null); const { isDragOver: descDragOver, dropZoneProps: descDropZoneProps } = useFileDropZone({ onDrop: (files) => files.forEach((f) => descEditorRef.current?.uploadFile(f)), @@ -241,6 +245,7 @@ export function ManualCreatePanel({ (data?.priority as IssuePriority | undefined) ?? draft.priority, ); const [submitting, setSubmitting] = useState(false); + const submittingRef = useRef(false); const [assigneeType, setAssigneeType] = useState(() => { if (data && "assignee_type" in data) { return (data.assignee_type as IssueAssigneeType | null) ?? undefined; @@ -421,8 +426,19 @@ export function ManualCreatePanel({ }; const handleSubmit = async () => { - if (!title.trim() || submitting) return; + // Single-flight on a ref, not `submitting`: two shortcut presses in one + // tick both read the pre-update state and would each fire a create. The + // ref flips synchronously, so the second press loses the race. + if (submittingRef.current) return; + if (!title.trim()) { + // The shortcut paths bypass the button entirely, so an empty title would + // otherwise be a silent no-op. Put the caret where the fix is; the + // button's tooltip says the rest. + titleEditorRef.current?.focus(); + return; + } if (uploadGate.isBlocked()) return; + submittingRef.current = true; setSubmitting(true); try { const description = descEditorRef.current?.getMarkdown()?.trim() || undefined; @@ -627,6 +643,7 @@ export function ManualCreatePanel({ : t(($) => $.create_issue.toast_failed), ); } finally { + submittingRef.current = false; setSubmitting(false); } }; @@ -682,6 +699,59 @@ export function ManualCreatePanel({ }); }; + // One state for the button and the keyboard paths, so a rendered affordance + // can never disagree with what `handleSubmit` will actually do. + const submitState: "submitting" | "uploading" | "missing_title" | "ready" = + submitting + ? "submitting" + : uploadGate.uploading + ? "uploading" + : !title.trim() + ? "missing_title" + : "ready"; + const submitBusy = submitState === "submitting" || submitState === "uploading"; + + // Built once and reused by both footer branches: rendering a separate Button + // per branch is how the keycaps drifted out of one of them before. + const createButton = ( + + ); + return ( <> {t(($) => $.create_issue.sr_manual)} @@ -733,11 +803,14 @@ export function ManualCreatePanel({
$.create_issue.title_placeholder)} className="text-lg font-semibold" onChange={(v) => updateTitle(v)} + // Chord only — plain Enter still just ends title editing (#5532). + onSubmitShortcut={handleSubmit} />
@@ -748,6 +821,7 @@ export function ManualCreatePanel({ defaultValue={draft.description} placeholder={t(($) => $.create_issue.description_placeholder)} onUpdate={(md) => setDraft({ description: md })} + onSubmit={handleSubmit} onUploadFile={handleUpload} onUploadingChange={uploadGate.onUploadingChange} debounceMs={500} @@ -1137,27 +1211,18 @@ export function ManualCreatePanel({ /> {t(($) => $.create_issue.create_another)} - {!title.trim() ? ( + {submitState === "missing_title" ? ( - } /> + {/* No `` wrapper needed now: aria-disabled leaves the + button focusable and hoverable, so it can anchor its own + tooltip. */} + {t(($) => $.create_issue.title_required)} ) : ( - + createButton )}
diff --git a/packages/views/modals/quick-create-issue.test.tsx b/packages/views/modals/quick-create-issue.test.tsx index a32d285a6c8..58cad86d4da 100644 --- a/packages/views/modals/quick-create-issue.test.tsx +++ b/packages/views/modals/quick-create-issue.test.tsx @@ -1,6 +1,6 @@ import { forwardRef, useImperativeHandle, useRef, useState, type ReactNode } from "react"; import { describe, it, expect, vi, beforeEach } from "vitest"; -import { fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { act, fireEvent, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; const mockQuickCreateIssue = vi.hoisted(() => vi.fn()); @@ -709,4 +709,40 @@ describe("AgentCreatePanel", () => { expect(screen.getByRole("button", { name: "Upload file" })).not.toBeDisabled(); }); }); + + // MUL-4931 — this path files a real issue, so a double-fire is a duplicate + // issue, not a cosmetic glitch. `submitting` is state: two chords landing in + // one tick both read the pre-update value, so only a synchronously-flipped + // ref can gate it. Mirrors the manual-create regression. + describe("send shortcut single-flight", () => { + it("creates once when the send chord fires twice in the same tick", async () => { + // Hold the request open so both presses land inside the in-flight window. + let release!: (v: unknown) => void; + mockQuickCreateIssue.mockImplementationOnce( + () => new Promise((resolve) => { release = resolve; }), + ); + + renderPanel({ onClose: vi.fn(), isExpanded: false, setIsExpanded: vi.fn() }); + + const editor = screen.getByPlaceholderText( + 'Tell the agent what to do, e.g. "let Bohan fix the inbox loading slowness in the Web project"', + ); + + // Both presses inside ONE act: React cannot re-render between them, so + // the second handler still closes over `submitting === false`. fireEvent + // would flush in between and hide the race entirely. + await act(async () => { + const press = () => + editor.dispatchEvent( + new KeyboardEvent("keydown", { key: "Enter", metaKey: true, bubbles: true }), + ); + press(); + press(); + }); + + await act(async () => { release(undefined); }); + + expect(mockQuickCreateIssue).toHaveBeenCalledTimes(1); + }); + }); }); diff --git a/packages/views/modals/quick-create-issue.tsx b/packages/views/modals/quick-create-issue.tsx index 6d0870a5a82..546c996d628 100644 --- a/packages/views/modals/quick-create-issue.tsx +++ b/packages/views/modals/quick-create-issue.tsx @@ -306,6 +306,9 @@ export function AgentCreatePanel({ const editorRef = useRef(null); const [hasContent, setHasContent] = useState(initialPrompt.trim().length > 0); const [submitting, setSubmitting] = useState(false); + // See create-issue's handleSubmit: `submitting` state can't gate two presses + // landing in the same tick, and ⌘+Enter makes that trivial to hit. + const submittingRef = useRef(false); const [justSent, setJustSent] = useState(false); const [sentCount, setSentCount] = useState(0); const [error, setError] = useState(null); @@ -342,7 +345,7 @@ export function AgentCreatePanel({ const submit = async () => { const md = editorRef.current?.getMarkdown()?.trim() ?? ""; - if (!md || !actor || submitting || versionBlocked) return; + if (!md || !actor || submittingRef.current || versionBlocked) return; // Submit-time re-read of the queue. Blocking here is what guarantees // `getMarkdown()`'s blob-url strip never erases a pasted/dropped image // whose attachment id hasn't reached `pendingAttachments` yet — the @@ -351,6 +354,7 @@ export function AgentCreatePanel({ const activeAttachmentIds = pendingAttachments .filter((a) => contentReferencesAttachment(md, a)) .map((a) => a.id); + submittingRef.current = true; setSubmitting(true); setError(null); try { @@ -424,6 +428,7 @@ export function AgentCreatePanel({ : t(($) => $.create_issue.agent.error_unknown), ); } finally { + submittingRef.current = false; setSubmitting(false); } }; @@ -713,7 +718,8 @@ export function AgentCreatePanel({ onClick={submit} disabled={!hasContent || !actor || submitting || versionBlocked || uploadGate.uploading} aria-disabled={uploadGate.uploading || undefined} - aria-busy={uploadGate.uploading || undefined} + // Sending is a busy state too, not just uploading. + aria-busy={uploadGate.uploading || submitting || undefined} title={ versionBlocked ? t(($) => $.create_issue.agent.version_blocked_tooltip, { min: versionCheck.min })