diff --git a/packages/views/autopilots/components/autopilot-dialog.tsx b/packages/views/autopilots/components/autopilot-dialog.tsx index c08fa0bdc0..00461c97d0 100644 --- a/packages/views/autopilots/components/autopilot-dialog.tsx +++ b/packages/views/autopilots/components/autopilot-dialog.tsx @@ -1,6 +1,6 @@ "use client"; -import { useMemo, useRef, useState } from "react"; +import { useId, useMemo, useRef, useState } from "react"; import { useQuery } from "@tanstack/react-query"; import { toast } from "sonner"; import { @@ -53,7 +53,7 @@ import type { AutopilotExecutionMode, AutopilotTrigger, } from "@multica/core/types"; -import { TitleEditor, ContentEditor } from "../../editor"; +import { TitleEditor, ContentEditor, type TitleEditorRef } from "../../editor"; import { ActorAvatar } from "../../common/actor-avatar"; import { SegmentedToggle } from "../../common/segmented-toggle"; import { ProjectPicker } from "../../projects/components/project-picker"; @@ -251,14 +251,32 @@ export function AutopilotDialog(props: AutopilotDialogProps) { // edits to the title, prompt or assignee. const scheduleWillBeWritten = triggerKind === "schedule" && !schedulePillDisabled && (isCreate || scheduleDirty); - const canSubmit = - title.trim().length > 0 && - assigneeId.length > 0 && - !submitting && - (!scheduleWillBeWritten || scheduleGate.scheduleValid); + + // The FIRST empty required field in reading order — the user fills one, the + // next surfaces. Only these two are answered here: a rejected schedule is + // re-checked against the server below, which toasts its actual reason. + const missingField: "title" | "assignee" | null = + title.trim().length === 0 ? "title" : assigneeId.length === 0 ? "assignee" : null; + + // Inline errors appear only after a submit attempt: a form that opens already + // shouting at the user for fields they have not reached yet is worse than the + // silence this replaces. Rendered as `showErrors && `, + // so filling the field clears its error without a second submit. + const [showErrors, setShowErrors] = useState(false); + const titleEditorRef = useRef(null); + const assigneeTriggerRef = useRef(null); + const assigneeErrorId = useId(); const handleSubmit = async () => { - if (!canSubmit) return; + if (submitting) return; + if (missingField !== null) { + // Reveal the inline errors and take the user to the field at fault; + // focusing scrolls the config column to it on its own. + setShowErrors(true); + if (missingField === "title") titleEditorRef.current?.focus(); + else assigneeTriggerRef.current?.focus(); + return; + } setSubmitting(true); try { if (scheduleWillBeWritten && !(await scheduleGate.ensureAccepted(schedule))) { @@ -519,6 +537,7 @@ export function AutopilotDialog(props: AutopilotDialogProps) {
$.dialog.title_placeholder)} @@ -526,6 +545,15 @@ export function AutopilotDialog(props: AutopilotDialogProps) { onChange={setTitle} onSubmit={handleSubmit} /> + {/* role="alert": the title is a contenteditable, so there is no + input to hang aria-describedby off — announcing the error is + the only way a screen-reader user learns why Create did + nothing. */} + {showErrors && title.trim().length === 0 && ( +

+ {t(($) => $.dialog.error_title_required)} +

+ )}
@@ -553,11 +581,14 @@ export function AutopilotDialog(props: AutopilotDialogProps) { {/* Right: Configuration */}
@@ -646,42 +689,71 @@ export function AutopilotDialog(props: AutopilotDialogProps) { // Right column sections // --------------------------------------------------------------------------- -function SectionLabel({ children }: { children: React.ReactNode }) { +function SectionLabel({ + children, + required, +}: { + children: React.ReactNode; + // Purely the sighted user's advance warning. `aria-required` is not supported + // on `role="button"`, so the picker cannot carry it; what a screen reader + // gets instead is the blocked submit's error, wired to the trigger through + // aria-describedby and announced by its own role="alert". + required?: boolean; +}) { return (
{children} + {required === true && ( + + * + + )}
); } function AgentSection({ + ref, selectedType, selectedId, onChange, selectedName, selectedDescription, + invalid, + errorId, }: { + ref: React.Ref; selectedType: AutopilotAssigneeType; selectedId: string; onChange: (next: AssigneeSelection) => void; selectedName?: string; selectedDescription?: string; + /** A submit was attempted with no assignee picked. */ + invalid: boolean; + errorId: string; }) { const { t } = useT("autopilots"); const hasSelection = selectedId.length > 0; return (
- {t(($) => $.dialog.section_assignee)} + {/* Marked required, unlike the Project and Subscribers pickers below it: + the three look identical, and nothing else told the user that only + this one blocks Create (#6231). */} + {t(($) => $.dialog.section_assignee)} {hasSelection ? ( @@ -710,6 +782,11 @@ function AgentSection({ } /> + {invalid && ( + + )}
); } diff --git a/packages/views/autopilots/components/autopilot-dialog.validation.test.tsx b/packages/views/autopilots/components/autopilot-dialog.validation.test.tsx new file mode 100644 index 0000000000..a55c66a75a --- /dev/null +++ b/packages/views/autopilots/components/autopilot-dialog.validation.test.tsx @@ -0,0 +1,187 @@ +import { useImperativeHandle, useRef, useState } from "react"; +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { renderWithI18n } from "../../test/i18n"; + +// Regression cover for GitHub #6231: "Create Autopilot" was disabled whenever +// a required field was empty, so a user who had not picked an assignee saw a +// dead control and no reason for it. The button is now live whenever a save +// isn't already in flight, and the click it accepts is what surfaces an inline +// error on the field at fault. + +const mockCreateAutopilot = vi.hoisted(() => vi.fn()); +const mockCreateTrigger = vi.hoisted(() => vi.fn()); + +vi.mock("@multica/core/hooks", () => ({ useWorkspaceId: () => "ws-test" })); +vi.mock("@multica/core/paths", () => ({ useCurrentWorkspace: () => ({ name: "Acme" }) })); + +vi.mock("@multica/core/workspace/queries", () => ({ + agentListOptions: (wsId: string) => ({ + queryKey: ["agents", wsId], + queryFn: async () => [ + { id: "agent-1", name: "Scout", description: "Researches things", archived_at: null }, + ], + }), + squadListOptions: (wsId: string) => ({ + queryKey: ["squads", wsId], + queryFn: async () => [], + }), +})); + +vi.mock("@multica/core/projects/queries", () => ({ + projectListOptions: (wsId: string) => ({ + queryKey: ["projects", wsId], + queryFn: async () => [], + }), +})); + +vi.mock("@multica/core/autopilots/queries", () => ({ + cronPreviewOptions: (wsId: string, expr: string, tz: string) => ({ + queryKey: ["cron-preview", wsId, expr, tz], + queryFn: async () => ({ next_runs: ["2126-07-14T01:00:00Z"] }), + retry: false, + }), +})); + +vi.mock("@multica/core/autopilots/mutations", () => ({ + useCreateAutopilot: () => ({ mutateAsync: mockCreateAutopilot }), + useCreateAutopilotTrigger: () => ({ mutateAsync: mockCreateTrigger }), + useUpdateAutopilot: () => ({ mutateAsync: vi.fn() }), + useUpdateAutopilotTrigger: () => ({ mutateAsync: vi.fn() }), +})); + +vi.mock("sonner", () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +// Tiptap in jsdom is neither cheap nor the subject here: the title is a plain +// input whose ref honours focus(), which is all the dialog asks of it. +vi.mock("../../editor", () => ({ + TitleEditor: ({ ref, defaultValue, placeholder, onChange, onSubmit }: any) => { + const [value, setValue] = useState(defaultValue ?? ""); + const inputRef = useRef(null); + useImperativeHandle(ref, () => ({ + getText: () => value, + focus: () => inputRef.current?.focus(), + focusAtCoords: () => inputRef.current?.focus(), + })); + return ( + { + setValue(e.target.value); + onChange?.(e.target.value); + }} + onKeyDown={(e) => { + if (e.key === "Enter") onSubmit?.(); + }} + /> + ); + }, + ContentEditor: ({ placeholder }: any) =>