diff --git a/apps/desktop/src/app/chat/composer/composer-utils.test.ts b/apps/desktop/src/app/chat/composer/composer-utils.test.ts index 4df8463ba2d..062b2567738 100644 --- a/apps/desktop/src/app/chat/composer/composer-utils.test.ts +++ b/apps/desktop/src/app/chat/composer/composer-utils.test.ts @@ -2,12 +2,14 @@ import type { Unstable_TriggerItem } from '@assistant-ui/core' import { describe, expect, it } from 'vitest' import { + acceptsTriggerCompletion, isPendingDraftPersistCurrent, type PendingDraftPersist, pickPlaceholder, slashArgStage, slashChipKindForItem, - slashCommandToken + slashCommandToken, + type TriggerAcceptInput } from './composer-utils' const item = (group: string): Unstable_TriggerItem => @@ -39,6 +41,53 @@ describe('slashChipKindForItem', () => { }) }) +describe('acceptsTriggerCompletion', () => { + const press = (key: string, overrides: Partial = {}) => + acceptsTriggerCompletion({ + activeExplicit: false, + freeTextArgStage: false, + key, + kind: '/', + query: 'personality alic', + ...overrides + }) + + it('accepts on Enter / Tab / Space for a finite option list', () => { + expect(press('Enter')).toBe(true) + expect(press('Tab')).toBe(true) + expect(press(' ')).toBe(true) + }) + + it('ignores keys that are neither navigation nor acceptance', () => { + expect(press('a')).toBe(false) + expect(press('Escape')).toBe(false) + }) + + it('lets an `@` mention take a literal space', () => { + expect(press(' ', { kind: '@', query: 'src/comp' })).toBe(false) + expect(press('Enter', { kind: '@', query: 'src/comp' })).toBe(true) + }) + + it('types a space on a bare `/ ` instead of accepting', () => { + expect(press(' ', { query: '' })).toBe(false) + }) + + // The `/goal ` class: the popover may be live over free-form text, so + // the keys that mean something else in prose must keep meaning it. + it('sends the prose rather than the unchosen first row', () => { + expect(press('Enter', { freeTextArgStage: true, query: 'goal ship the redesign' })).toBe(false) + expect(press(' ', { freeTextArgStage: true, query: 'goal ship the' })).toBe(false) + }) + + it('accepts on Enter once the user has arrowed to a row deliberately', () => { + expect(press('Enter', { activeExplicit: true, freeTextArgStage: true, query: 'goal stat' })).toBe(true) + }) + + it('keeps Tab as the explicit accept even over free text', () => { + expect(press('Tab', { freeTextArgStage: true, query: 'goal stat' })).toBe(true) + }) +}) + describe('pickPlaceholder', () => { it('returns a member of the pool', () => { const pool = ['a', 'b', 'c'] as const diff --git a/apps/desktop/src/app/chat/composer/composer-utils.ts b/apps/desktop/src/app/chat/composer/composer-utils.ts index 547a210f06f..21e3c1ac4a1 100644 --- a/apps/desktop/src/app/chat/composer/composer-utils.ts +++ b/apps/desktop/src/app/chat/composer/composer-utils.ts @@ -4,6 +4,8 @@ import type { SlashChipKind } from '@/components/assistant-ui/directive-text' import type { ComposerAttachment } from '@/store/composer' import { setSessionPickerOpen } from '@/store/session' +import type { TriggerState } from './text-utils' + export const COMPOSER_STACK_BREAKPOINT_PX = 320 // Above the stack breakpoint but still cramped: the model pill sheds its label @@ -59,6 +61,48 @@ export const slashArgStage = (query: string) => query.includes(' ') /** The `/command` token of a slash query (`personality x` → `/personality`). */ export const slashCommandToken = (query: string) => `/${query.split(/\s+/, 1)[0]?.toLowerCase() ?? ''}` +export interface TriggerAcceptInput { + /** The user moved the highlight themselves (arrow keys) rather than + * inheriting the list's default first row. */ + activeExplicit: boolean + /** The trigger is a slash command whose argument is arbitrary prose. */ + freeTextArgStage: boolean + key: string + kind: TriggerState['kind'] + query: string +} + +/** + * Whether a keypress accepts the highlighted completion while the popover is + * open. Tab is always an accept — it has no other meaning in the composer. + * + * Enter and Space are conditional, because both mean something else while a + * free-text argument is being written (`/goal ship the redesign`). Space types + * a space, and Enter sends the message; letting either take the popover's + * pre-highlighted row would swap the prose the user is mid-sentence on for a + * subcommand they never chose. Enter still accepts once the user has arrowed + * to a row deliberately, so the highlight never lies about what Enter will do. + */ +export function acceptsTriggerCompletion({ + activeExplicit, + freeTextArgStage, + key, + kind, + query +}: TriggerAcceptInput): boolean { + if (key === 'Tab') { + return true + } + + if (key === 'Enter') { + return !freeTextArgStage || activeExplicit + } + + // Space is slash-only (an `@` mention takes a literal space) and gated to a + // non-empty query so a bare `/ ` still types a space. + return key === ' ' && kind === '/' && Boolean(query.trim()) && !freeTextArgStage +} + export interface QueueEditState { attachments: ComposerAttachment[] draft: string diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts index c7a4e2f138a..a4ab38b3b3a 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.test.ts @@ -148,6 +148,30 @@ describe('useComposerTrigger — free-text slash arguments', () => { expect(editor.querySelector('[data-slash-kind]')).toBeNull() }) + it('treats the default highlight as a suggestion until the user arrows to a row', () => { + const editor = mountEditor('/goal stat') + const { hook } = mountTrigger(editor, [item('/goal status', 'Options')]) + + act(() => hook.result.current.refreshTrigger()) + expect(hook.result.current.triggerActiveExplicit).toBe(false) + + act(() => hook.result.current.moveTriggerActive(1)) + expect(hook.result.current.triggerActiveExplicit).toBe(true) + }) + + it('drops a deliberate selection once the query moves on', () => { + const editor = mountEditor('/goal stat') + const { hook } = mountTrigger(editor, [item('/goal status', 'Options')]) + + act(() => hook.result.current.refreshTrigger()) + act(() => hook.result.current.moveTriggerActive(1)) + + renderComposerContents(editor, '/goal start the migration') + act(() => hook.result.current.refreshTrigger()) + + expect(hook.result.current.triggerActiveExplicit).toBe(false) + }) + it('still commits a fully typed finite option as one directive chip', () => { const editor = mountEditor('/personality creative') const { hook } = mountTrigger(editor, []) diff --git a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts index e6ab8535124..ea968fb3643 100644 --- a/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts +++ b/apps/desktop/src/app/chat/composer/hooks/use-composer-trigger.ts @@ -53,6 +53,11 @@ export function useComposerTrigger({ }: UseComposerTriggerOptions) { const [trigger, setTrigger] = useState(null) const [triggerActive, setTriggerActive] = useState(0) + // The list highlights its first row on open, which is a suggestion rather + // than a choice. This records that the user moved the highlight themselves, + // which is what lets Enter accept a completion in a free-text argument stage + // without stealing prose from everyone who never touched the arrows. + const [triggerActiveExplicit, setTriggerActiveExplicit] = useState(false) const [triggerItems, setTriggerItems] = useState([]) // Set synchronously in keydown when the open trigger popover consumes a // navigation/control key (Arrow/Enter/Tab/Escape). The subsequent keyup must @@ -63,6 +68,11 @@ export function useComposerTrigger({ // re-rendered and the handler closure sees the post-keydown state. const triggerKeyConsumedRef = useRef(false) + const resetTriggerActive = useCallback(() => { + setTriggerActive(0) + setTriggerActiveExplicit(false) + }, []) + const refreshTrigger = useCallback(() => { const editor = editorRef.current @@ -80,7 +90,7 @@ export function useComposerTrigger({ if (!rawText.includes('@') && !rawText.includes('/')) { if (trigger) { setTrigger(null) - setTriggerActive(0) + resetTriggerActive() } return @@ -109,9 +119,9 @@ export function useComposerTrigger({ // caret move (mouseup) or a stray refresh — must preserve the user's // current selection instead of snapping back to the first item. if (detected?.kind !== trigger?.kind || detected?.query !== trigger?.query) { - setTriggerActive(0) + resetTriggerActive() } - }, [editorRef, trigger]) + }, [editorRef, resetTriggerActive, trigger]) const triggerAdapter: Unstable_TriggerAdapter | null = trigger?.kind === '@' ? at.adapter : trigger?.kind === '/' ? slash.adapter : null @@ -149,7 +159,13 @@ export function useComposerTrigger({ const closeTrigger = () => { setTrigger(null) setTriggerItems([]) - setTriggerActive(0) + resetTriggerActive() + } + + /** Step the highlight, marking it as the user's own deliberate pick. */ + const moveTriggerActive = (delta: number) => { + setTriggerActiveExplicit(true) + setTriggerActive(idx => (idx + delta + triggerItems.length) % triggerItems.length) } useEffect(() => { @@ -360,12 +376,14 @@ export function useComposerTrigger({ ascendTriggerPath, closeTrigger, commitTypedSlashDirective, + moveTriggerActive, refreshTrigger, replaceTriggerWithChip, setTriggerActive, slashFreeTextArgStage, trigger, triggerActive, + triggerActiveExplicit, triggerItems, triggerKeyConsumedRef, triggerLoading diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 6ce8289fa0a..ac4f5dd3027 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -22,7 +22,12 @@ import { $autoSpeakReplies } from '@/store/voice-prefs' import { useTheme } from '@/themes' import { AttachmentList } from './attachments' -import { COMPOSER_FADE_BACKGROUND, type QueueEditState, slashArgStage } from './composer-utils' +import { + acceptsTriggerCompletion, + COMPOSER_FADE_BACKGROUND, + type QueueEditState, + slashArgStage +} from './composer-utils' import { ContextMenu } from './context-menu' import { COMPOSER_AREAS, runComposerMiddleware } from './contrib' import { ComposerControls } from './controls' @@ -302,12 +307,14 @@ export function ChatBar({ ascendTriggerPath, closeTrigger, commitTypedSlashDirective, + moveTriggerActive, refreshTrigger, replaceTriggerWithChip, setTriggerActive, slashFreeTextArgStage, trigger, triggerActive, + triggerActiveExplicit, triggerItems, triggerKeyConsumedRef, triggerLoading @@ -528,7 +535,7 @@ export function ChatBar({ if (event.key === 'ArrowDown') { event.preventDefault() triggerKeyConsumedRef.current = true - setTriggerActive(idx => (idx + 1) % triggerItems.length) + moveTriggerActive(1) return } @@ -536,20 +543,21 @@ export function ChatBar({ if (event.key === 'ArrowUp') { event.preventDefault() triggerKeyConsumedRef.current = true - setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) + moveTriggerActive(-1) return } - // Enter / Tab / Space all accept the highlighted item: a no-arg command - // commits its directive chip, an arg-taking command expands to its - // options step, and an arg option commits the full `/cmd arg` chip. Space - // is slash-only (an `@` mention takes a literal space) and gated to a - // non-empty query so a bare `/ ` still types a space. - const acceptOnSpace = - event.key === ' ' && trigger.kind === '/' && Boolean(trigger.query.trim()) && !slashFreeTextArgStage - - const accept = event.key === 'Enter' || event.key === 'Tab' || acceptOnSpace + // Accepting the highlighted item: a no-arg command commits its directive + // chip, an arg-taking command expands to its options step, and an arg + // option commits the full `/cmd arg` chip. + const accept = acceptsTriggerCompletion({ + activeExplicit: triggerActiveExplicit, + freeTextArgStage: slashFreeTextArgStage, + key: event.key, + kind: trigger.kind, + query: trigger.query + }) if (accept) { event.preventDefault()