diff --git a/packages/ui/src/components/chat/composer/ui/DraftTargetSelectors.tsx b/packages/ui/src/components/chat/composer/ui/DraftTargetSelectors.tsx index 32c5dbbb..b2fd3f93 100644 --- a/packages/ui/src/components/chat/composer/ui/DraftTargetSelectors.tsx +++ b/packages/ui/src/components/chat/composer/ui/DraftTargetSelectors.tsx @@ -12,6 +12,7 @@ import React from 'react'; import { Icon } from '@/components/icon/Icon'; import { Input } from '@/components/ui/input'; import { MobileOverlayPanel } from '@/components/ui/MobileOverlayPanel'; +import { shouldDismissDropdown } from '@/components/ui/dropdown-navigation'; import { Select, SelectContent, @@ -107,6 +108,12 @@ export function DraftTargetSelectors(props: DraftTargetProps) { const [openPicker, setOpenPicker] = React.useState<'project' | 'worktree' | null>(null); const projectTriggerRef = React.useRef(null); const worktreeTriggerRef = React.useRef(null); + const handlePickerKeyDown = (event: React.KeyboardEvent) => { + if (openPicker === null || !shouldDismissDropdown(event)) return; + event.preventDefault(); + event.stopPropagation(); + setOpenPicker(null); + }; useKeybind('open_draft_project_picker', () => { projectTriggerRef.current?.focus(); @@ -139,6 +146,7 @@ export function DraftTargetSelectors(props: DraftTargetProps) { > @@ -146,7 +154,7 @@ export function DraftTargetSelectors(props: DraftTargetProps) { {} - + {projects.map((project) => ( {} @@ -165,6 +173,7 @@ export function DraftTargetSelectors(props: DraftTargetProps) { > @@ -172,7 +181,7 @@ export function DraftTargetSelectors(props: DraftTargetProps) { {selectedBranchLabel ?? t('chat.chatInput.branch')} - + {projectRootBranchOption ? ( {t('chat.chatInput.projectRoot')} diff --git a/packages/ui/src/components/chat/message/TextSelectionMenu.tsx b/packages/ui/src/components/chat/message/TextSelectionMenu.tsx index c8ecc9f4..a57192aa 100644 --- a/packages/ui/src/components/chat/message/TextSelectionMenu.tsx +++ b/packages/ui/src/components/chat/message/TextSelectionMenu.tsx @@ -17,6 +17,7 @@ import { isVSCodeRuntime } from '@/lib/desktop'; import { useI18n } from '@/lib/i18n'; import { rangeToMarkdown, trimSelectionValue, wrapMarkdownSelectionForChat } from './selectionMarkdown'; import { focusChatInput } from '@/components/chat/composer/editor/dom'; +import { registerActiveSelectionToolbar } from '@/lib/addSelectionToChat'; interface TextSelectionMenuProps { containerRef: React.RefObject; @@ -60,6 +61,7 @@ export const TextSelectionMenu: React.FC = ({ containerR const openRafRef = React.useRef(null); const mouseUpTimeoutRef = React.useRef(null); const isMenuVisibleRef = React.useRef(false); + const activeAddToChatCleanupRef = React.useRef<(() => void) | null>(null); const createSession = useSessionUIStore((state) => state.createSession); const currentSessionId = useSessionUIStore((state) => state.currentSessionId); const setPendingInputText = useInputStore((state) => state.setPendingInputText); @@ -75,6 +77,8 @@ export const TextSelectionMenu: React.FC = ({ containerR React.useEffect(() => { return () => { + activeAddToChatCleanupRef.current?.(); + activeAddToChatCleanupRef.current = null; if (openRafRef.current !== null) { window.cancelAnimationFrame(openRafRef.current); openRafRef.current = null; @@ -88,6 +92,8 @@ export const TextSelectionMenu: React.FC = ({ containerR const hideMenu = React.useCallback(() => { pendingSelectionRef.current = null; + activeAddToChatCleanupRef.current?.(); + activeAddToChatCleanupRef.current = null; if (!isMenuVisibleRef.current) { return; @@ -123,12 +129,30 @@ export const TextSelectionMenu: React.FC = ({ containerR return Math.min(Math.max(anchorX, minX), maxX); }, []); + const addMarkdownToChat = React.useCallback((markdownText: string) => { + const markdownBlock = wrapMarkdownSelectionForChat(markdownText); + setPendingInputText(markdownBlock, 'append'); + + hideMenu(); + + window.getSelection()?.removeAllRanges(); + queueMicrotask(() => { + focusChatInput(); + }); + }, [hideMenu, setPendingInputText]); + const showMenu = React.useCallback(() => { if (!pendingSelectionRef.current) return; const { plainText, markdownText, rect } = pendingSelectionRef.current; const shouldAnimateIn = !position.show; + activeAddToChatCleanupRef.current?.(); + activeAddToChatCleanupRef.current = registerActiveSelectionToolbar({ + addToChat: () => addMarkdownToChat(markdownText), + dismiss: hideMenu, + }); + // Position menu above the selection const menuX = isMobile ? rect.left + rect.width / 2 @@ -154,7 +178,7 @@ export const TextSelectionMenu: React.FC = ({ containerR openRafRef.current = null; }); } - }, [getDesktopClampedX, isMobile, position.show]); + }, [addMarkdownToChat, getDesktopClampedX, hideMenu, isMobile, position.show]); React.useLayoutEffect(() => { if (!position.show || isMobile || !menuRef.current) { @@ -302,18 +326,8 @@ export const TextSelectionMenu: React.FC = ({ containerR const handleAddToChat = React.useCallback(() => { if (!selectedTextMarkdown) return; - - const markdownBlock = wrapMarkdownSelectionForChat(selectedTextMarkdown); - setPendingInputText(markdownBlock, 'append'); - - hideMenu(); - - // Clear selection - window.getSelection()?.removeAllRanges(); - queueMicrotask(() => { - focusChatInput(); - }); - }, [selectedTextMarkdown, setPendingInputText, hideMenu]); + addMarkdownToChat(selectedTextMarkdown); + }, [addMarkdownToChat, selectedTextMarkdown]); const handleCreateNewSession = React.useCallback(async () => { if (!selectedText) return; diff --git a/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.test.ts b/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.test.ts index 962d42f8..256383a1 100644 --- a/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.test.ts +++ b/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from 'bun:test'; -import { updateShortcutRecordingState } from './ShortcutRecordingDialog'; +import { settleShortcutRecordingState, updateShortcutRecordingState } from './ShortcutRecordingDialog'; -const emptyState = { chords: [], livePreview: null }; +const emptyState = { chords: [], livePreview: null, settled: false }; function keyEvent(key: string, modifiers: Partial> = {}) { return { key, repeat: false, isComposing: false, altKey: false, ctrlKey: false, metaKey: false, shiftKey: false, ...modifiers }; @@ -14,13 +14,38 @@ describe('ShortcutRecordingDialog recording state', () => { expect(updateShortcutRecordingState(pressed, keyEvent('Control'), 'keyup').livePreview).toBeNull(); }); - test('records up to two chords', () => { - const first = updateShortcutRecordingState(emptyState, keyEvent('k', { ctrlKey: true }), 'keydown'); - const second = updateShortcutRecordingState(first, keyEvent('p', { ctrlKey: true }), 'keydown'); - const third = updateShortcutRecordingState(second, keyEvent('x', { ctrlKey: true }), 'keydown'); - expect(first.chords).toEqual(['mod+k']); - expect(second.chords).toEqual(['mod+k', 'mod+p']); - expect(third.chords).toEqual(['mod+k', 'mod+p']); + test('waits after the first chord and settles when a second chord is recorded', () => { + const first = updateShortcutRecordingState(emptyState, keyEvent('s', { ctrlKey: true }), 'keydown'); + const second = updateShortcutRecordingState(first, keyEvent('p'), 'keydown'); + const third = updateShortcutRecordingState(second, keyEvent('x'), 'keydown'); + expect(first.chords).toEqual(['mod+s']); + expect(first.settled).toBe(false); + expect(second.chords).toEqual(['mod+s', 'p']); + expect(second.settled).toBe(true); + expect(third.chords).toEqual(['x']); + expect(third.settled).toBe(false); + }); + + test('settles a single chord for timeout and Confirm validation', () => { + const waiting = updateShortcutRecordingState(emptyState, keyEvent('s', { ctrlKey: true }), 'keydown'); + expect(settleShortcutRecordingState(waiting)).toEqual({ chords: ['mod+s'], livePreview: null, settled: true }); + }); + + test('records at most three simultaneous keys', () => { + const previous = { chords: ['mod+k'], livePreview: null, settled: false }; + const threeKeys = updateShortcutRecordingState( + previous, + keyEvent('s', { ctrlKey: true, shiftKey: true }), + 'keydown', + ); + const fourKeys = updateShortcutRecordingState( + previous, + keyEvent('s', { ctrlKey: true, metaKey: true, shiftKey: true }), + 'keydown', + ); + + expect(threeKeys.chords).toEqual(['mod+k', 'mod+shift+s']); + expect(fourKeys.chords).toEqual(['mod+k']); }); test('ignores repeat and IME events', () => { @@ -29,9 +54,11 @@ describe('ShortcutRecordingDialog recording state', () => { }); test('records Enter and Escape while Backspace removes the final chord', () => { - const state = { chords: ['mod+k', 'mod+p'], livePreview: null }; + const state = { chords: ['mod+k', 'mod+p'], livePreview: null, settled: true }; expect(updateShortcutRecordingState(emptyState, keyEvent('Enter'), 'keydown').chords).toEqual(['enter']); expect(updateShortcutRecordingState(emptyState, keyEvent('Escape'), 'keydown').chords).toEqual(['escape']); expect(updateShortcutRecordingState(state, keyEvent('Backspace'), 'keydown').chords).toEqual(['mod+k']); + expect(updateShortcutRecordingState(state, keyEvent('Backspace'), 'keydown').settled).toBe(false); + expect(updateShortcutRecordingState({ chords: ['mod+k'], livePreview: null, settled: false }, keyEvent('Backspace'), 'keydown')).toEqual(emptyState); }); }); diff --git a/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.tsx b/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.tsx index 68353904..a04fb8a1 100644 --- a/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.tsx +++ b/packages/ui/src/components/sections/openchamber/ShortcutRecordingDialog.tsx @@ -22,6 +22,8 @@ import { import { useI18n } from '@/lib/i18n'; const MODIFIER_KEYS = new Set(['shift', 'control', 'alt', 'meta']); +const MAX_SHORTCUT_KEY_COUNT = 3; +const SECOND_CHORD_TIMEOUT_MS = 3000; interface RecordingKeyboardEvent { altKey: boolean; @@ -36,6 +38,7 @@ interface RecordingKeyboardEvent { interface ShortcutRecordingState { chords: ShortcutCombo[]; livePreview: ShortcutCombo | null; + settled: boolean; } interface ShortcutRecordingDialogProps { @@ -49,6 +52,19 @@ interface ShortcutRecordingDialogProps { onOpenChange: (open: boolean) => void; } +function getPhysicalKeyCount( + event: Pick, + includeEventKey = false, +): number { + const keys = new Set(); + if (event.altKey) keys.add('alt'); + if (event.ctrlKey) keys.add('control'); + if (event.metaKey) keys.add('meta'); + if (event.shiftKey) keys.add('shift'); + if (includeEventKey) keys.add(event.key.toLowerCase()); + return keys.size; +} + function isCustomizableConflict( conflict: ShortcutBindingConflict, ): conflict is ShortcutBindingConflict & { action: CustomizableShortcutAction } { @@ -56,6 +72,7 @@ function isCustomizableConflict( } function getModifierPreview(event: RecordingKeyboardEvent): ShortcutCombo | null { + if (getPhysicalKeyCount(event) > MAX_SHORTCUT_KEY_COUNT) return null; const parts: string[] = []; if (event.metaKey || event.ctrlKey) parts.push('mod'); if (event.shiftKey) parts.push('shift'); @@ -65,6 +82,7 @@ function getModifierPreview(event: RecordingKeyboardEvent): ShortcutCombo | null function keyboardEventToCombo(event: RecordingKeyboardEvent): ShortcutCombo | null { if (MODIFIER_KEYS.has(event.key.toLowerCase())) return null; + if (getPhysicalKeyCount(event, true) > MAX_SHORTCUT_KEY_COUNT) return null; const key = keyToShortcutToken(event.key); if (!key) return null; @@ -80,6 +98,7 @@ function keyboardEventToCombo(event: RecordingKeyboardEvent): ShortcutCombo | nu function modifierKeyUpToCombo(event: React.KeyboardEvent): ShortcutCombo | null { const key = event.key.toLowerCase(); if (!MODIFIER_KEYS.has(key)) return null; + if (getPhysicalKeyCount(event, true) > MAX_SHORTCUT_KEY_COUNT) return null; const parts: string[] = []; if (event.metaKey || event.ctrlKey || key === 'meta' || key === 'control') parts.push('mod'); @@ -88,6 +107,11 @@ function modifierKeyUpToCombo(event: React.KeyboardEvent): Short return parts.length > 0 ? normalizeCombo(parts.join('+')) : null; } +// eslint-disable-next-line react-refresh/only-export-components -- tested pure recording state transition +export function settleShortcutRecordingState(state: ShortcutRecordingState): ShortcutRecordingState { + return state.chords.length > 0 ? { ...state, livePreview: null, settled: true } : state; +} + // eslint-disable-next-line react-refresh/only-export-components -- tested pure recording state transition export function updateShortcutRecordingState( state: ShortcutRecordingState, @@ -100,14 +124,19 @@ export function updateShortcutRecordingState( } if (event.key === 'Backspace') { - return { chords: state.chords.slice(0, -1), livePreview: null }; + return { chords: state.chords.slice(0, -1), livePreview: null, settled: false }; } const chord = keyboardEventToCombo(event); if (chord) { + if (state.settled) { + return { chords: [chord], livePreview: null, settled: false }; + } + const chords = state.chords.length < 2 ? [...state.chords, chord] : state.chords; return { - chords: state.chords.length < 2 ? [...state.chords, chord] : state.chords, + chords, livePreview: null, + settled: chords.length === 2, }; } @@ -122,27 +151,47 @@ export const ShortcutRecordingDialog: React.FC = ( }) => { const { t } = useI18n(); const actionLabel = (shortcut: CustomizableShortcutAction) => t(shortcut.settingsLabelKey); - const [recording, setRecording] = React.useState({ chords: [], livePreview: null }); + const conflictActionLabel = (conflict: ShortcutBindingConflict) => ( + conflict.action.customizable + ? actionLabel(conflict.action) + : formatShortcutForDisplay(conflict.action.defaultBinding) + ); + const [recording, setRecording] = React.useState({ chords: [], livePreview: null, settled: false }); const recordingRef = React.useRef(null); React.useEffect(() => { if (!action) return; - setRecording({ chords: [], livePreview: null }); + setRecording({ chords: [], livePreview: null, settled: false }); recordingRef.current?.focus(); }, [action]); + const waitingForSecondChord = recording.chords.length === 1 && !recording.settled; + + React.useEffect(() => { + if (!waitingForSecondChord) return; + const timeout = window.setTimeout( + () => setRecording(settleShortcutRecordingState), + SECOND_CHORD_TIMEOUT_MS, + ); + return () => window.clearTimeout(timeout); + }, [waitingForSecondChord]); + const combo = normalizeCombo(recording.chords.join(' ')); const conflicts = React.useMemo( () => action && combo ? getShortcutBindingConflicts(action.id, combo, overrides) : [], [action, combo, overrides], ); - const protectedConflict = conflicts.find((conflict) => !conflict.action.customizable); + const protectedConflict = conflicts.find((conflict) => ( + !conflict.action.customizable && conflict.kind !== 'contextual-prefix' + )); const customizableConflicts = conflicts.filter(isCustomizableConflict); const prefixConflict = customizableConflicts.find((conflict) => conflict.kind === 'prefix'); const exactConflict = customizableConflicts.find((conflict) => conflict.kind === 'exact'); + const contextualPrefixConflict = conflicts.find((conflict) => conflict.kind === 'contextual-prefix'); const close = () => onOpenChange(false); const confirm = () => { + if (!recording.settled) setRecording(settleShortcutRecordingState); if (!action || !combo || protectedConflict || prefixConflict) return; onSave(action.id, combo, exactConflict?.action.id); close(); @@ -150,10 +199,11 @@ export const ShortcutRecordingDialog: React.FC = ( const handleRecordingEvent = (event: React.KeyboardEvent, phase: 'keydown' | 'keyup') => { event.preventDefault(); event.stopPropagation(); + if (phase === 'keyup' && action?.id === 'switch_context_surface' && recording.chords.length === 0) { const modifierCombo = modifierKeyUpToCombo(event); if (modifierCombo) { - setRecording({ chords: [modifierCombo], livePreview: null }); + setRecording({ chords: [modifierCombo], livePreview: null, settled: true }); return; } } @@ -167,7 +217,7 @@ export const ShortcutRecordingDialog: React.FC = ( shiftKey: event.shiftKey, }, phase); setRecording(action?.id === 'switch_context_surface' && nextRecording.chords.length > 1 - ? { ...nextRecording, chords: nextRecording.chords.slice(0, 1) } + ? recording : nextRecording); }; @@ -215,21 +265,28 @@ export const ShortcutRecordingDialog: React.FC = ( - {protectedConflict ? ( + {recording.settled && protectedConflict ? (

{t('settings.openchamber.keyboardShortcuts.error.internalConflict')}

- ) : prefixConflict ? ( + ) : recording.settled && prefixConflict ? (

{t('settings.openchamber.keyboardShortcuts.error.prefixConflict', { action: actionLabel(prefixConflict.action) })}

) : null} - {exactConflict && !protectedConflict && !prefixConflict ? ( + {recording.settled && exactConflict && !protectedConflict && !prefixConflict ? (

{t('settings.openchamber.keyboardShortcuts.error.exactConflict', { action: actionLabel(exactConflict.action) })}

) : null} - {combo && isRiskyBrowserShortcut(combo) ? ( + {recording.settled && contextualPrefixConflict && !protectedConflict && !prefixConflict ? ( +

+ {t('settings.openchamber.keyboardShortcuts.warning.contextualPrefix', { + action: conflictActionLabel(contextualPrefixConflict), + })} +

+ ) : null} + {recording.settled && combo && isRiskyBrowserShortcut(combo) ? (

{t('settings.openchamber.keyboardShortcuts.warning.riskyBrowserShortcut')}

@@ -242,7 +299,7 @@ export const ShortcutRecordingDialog: React.FC = (