diff --git a/src/renderer/src/components/editor/MarkdownPreview.tsx b/src/renderer/src/components/editor/MarkdownPreview.tsx index 3b4c53c3f..6e7a80e67 100644 --- a/src/renderer/src/components/editor/MarkdownPreview.tsx +++ b/src/renderer/src/components/editor/MarkdownPreview.tsx @@ -68,6 +68,10 @@ import { isMarkdownPreviewFindShortcut, setActiveMarkdownPreviewSearchMatch } from './markdown-preview-search' +import { + getMarkdownAnnotationBlockKeyForSelection, + isMarkdownPreviewAddReviewNoteShortcut +} from './markdown-preview-annotation-shortcut' import { usePreserveSectionDuringExternalEdit } from './usePreserveSectionDuringExternalEdit' import { openHttpLink, type HttpLinkSourceOwner } from '@/lib/http-link-routing' import { getShortcutPlatform } from '@/lib/shortcut-platform' @@ -916,6 +920,20 @@ export default function MarkdownPreview({ return } + if ( + isMarkdownPreviewAddReviewNoteShortcut(event, getShortcutPlatform(), keybindings) && + targetInsidePreview && + markdownAnnotationsEnabled + ) { + const blockKey = getMarkdownAnnotationBlockKeyForSelection(root, window.getSelection()) + if (blockKey) { + event.preventDefault() + event.stopPropagation() + setActiveAnnotationBlockKey(blockKey) + } + return + } + if (!isSearchOpen) { return } @@ -930,7 +948,7 @@ export default function MarkdownPreview({ window.addEventListener('keydown', handleKeyDown, { capture: true }) return () => window.removeEventListener('keydown', handleKeyDown, { capture: true }) - }, [closeSearch, isSearchOpen, keybindings, openSearch]) + }, [closeSearch, isSearchOpen, keybindings, markdownAnnotationsEnabled, openSearch]) const handleCopyMarkdownReviewNotes = useCallback(async (): Promise => { if (markdownReviewNotes.length === 0) { @@ -1248,6 +1266,7 @@ export default function MarkdownPreview({ className={`markdown-annotation-block ${hasReviewNotes ? 'has-review-notes' : ''}`.trim()} data-source-line={range.startLine} data-source-end-line={range.endLine} + data-annotation-block-key={blockKey} onClick={(event) => handleAnnotatedMarkdownBlockClick(range, event)} > {rendered} @@ -1668,6 +1687,11 @@ export default function MarkdownPreview({ } const blockKey = `li:${range.startLine}-${range.endLine}` const hasReviewNotes = getMarkdownCommentsForRange(range).length > 0 + const controls = renderAnnotationControls( + range, + blockKey, + getMarkdownPreviewAnnotationQuote(children) + ) return (
  • handleAnnotatedMarkdownBlockClick(range, event)} > {children} - {renderAnnotationControls( - range, - blockKey, - getMarkdownPreviewAnnotationQuote(children) - )} + {controls}
    • ) diff --git a/src/renderer/src/components/editor/MonacoEditor.tsx b/src/renderer/src/components/editor/MonacoEditor.tsx index daf288d8c..a1945794b 100644 --- a/src/renderer/src/components/editor/MonacoEditor.tsx +++ b/src/renderer/src/components/editor/MonacoEditor.tsx @@ -48,7 +48,11 @@ import { getDiffCommentPopoverTop } from '../diff-comments/diff-comment-popover-position' import { isLinuxUserAgent } from '../terminal-pane/pane-helpers' -import { installEditorSaveShortcut, installMonacoEditorFindShortcut } from './editor-shortcuts' +import { + installEditorAddReviewNoteShortcut, + installEditorSaveShortcut, + installMonacoEditorFindShortcut +} from './editor-shortcuts' import { Plus } from 'lucide-react' import { getMonacoMarkdownSelectionAnnotationTarget, @@ -198,6 +202,10 @@ export default function MonacoEditor({ const [commentPopover, setCommentPopover] = useState(null) const [selectionAnnotationTarget, setSelectionAnnotationTarget] = useState(null) + // Why: the Monaco mount closure installs its keydown listeners once, so the + // add-review-note shortcut reads the live selection target through a ref. + const selectionAnnotationTargetRef = useRef(null) + selectionAnnotationTargetRef.current = selectionAnnotationTarget const isDark = settings?.theme === 'dark' || (settings?.theme === 'system' && window.matchMedia('(prefers-color-scheme: dark)').matches) @@ -413,6 +421,18 @@ export default function MonacoEditor({ propsRef.current.onSave(value) }) const cleanupFindShortcut = installMonacoEditorFindShortcut(editorInstance) + // Opens the same composer as the selection "+" button; the target ref + // mirrors the last-rendered selection target and is null unless markdown + // annotations are enabled and text is selected. + const cleanupAddReviewNoteShortcut = installEditorAddReviewNoteShortcut(editorDomNode, () => { + const target = selectionAnnotationTargetRef.current + if (!target) { + return false + } + setCommentPopover(target) + setSelectionAnnotationTarget(null) + return true + }) const searchInFilesAction = editorInstance.addAction({ id: 'orca.searchInFiles', label: translate('auto.components.editor.MonacoEditor.fd68ae03b3', 'Search in Files'), @@ -512,6 +532,7 @@ export default function MonacoEditor({ gutterMouseDownSub.dispose() cleanupSaveShortcut() cleanupFindShortcut() + cleanupAddReviewNoteShortcut() editorDomNode.removeEventListener('paste', onLargeTextPaste, { capture: true }) searchInFilesAction.dispose() autoHeightSub?.dispose() diff --git a/src/renderer/src/components/editor/RichMarkdownEditor.tsx b/src/renderer/src/components/editor/RichMarkdownEditor.tsx index fdbaee0df..2f499ba6b 100644 --- a/src/renderer/src/components/editor/RichMarkdownEditor.tsx +++ b/src/renderer/src/components/editor/RichMarkdownEditor.tsx @@ -1,4 +1,4 @@ -import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react' +import React, { useCallback, useEffect, useRef, useState } from 'react' import { useEditorState, type Editor } from '@tiptap/react' import type { DiffComment, MarkdownDocument } from '../../../../shared/types' import { useAppStore } from '@/store' @@ -10,9 +10,7 @@ import { useLinkBubble } from './useLinkBubble' import { useEditorScrollRestore } from './useEditorScrollRestore' import { useModifierHeldClass } from './useModifierHeldClass' import { registerPendingEditorFlush } from './editor-pending-flush' -import type { MarkdownTocItem } from './markdown-table-of-contents' -import { findRichMarkdownTocHeadingTarget } from './rich-markdown-toc-heading-target' -import { selectMarkdownTableOfContents } from './markdown-toc-visibility-gate' +import { useRichMarkdownTableOfContents } from './use-rich-markdown-table-of-contents' import { RichMarkdownEditorSurface } from './RichMarkdownEditorSurface' import { useRichMarkdownEditorInstance } from './useRichMarkdownEditorInstance' import { useRichMarkdownMenuController } from './useRichMarkdownMenuController' @@ -56,10 +54,6 @@ type RichMarkdownEditorProps = { headerSlot?: React.ReactNode } -function flattenMarkdownTocItems(items: MarkdownTocItem[]): MarkdownTocItem[] { - return items.flatMap((item) => [item, ...flattenMarkdownTocItems(item.children)]) -} - export default function RichMarkdownEditor({ fileId, content, @@ -112,6 +106,7 @@ export default function RichMarkdownEditor({ const onOpenDocLinkRef = useRef(onOpenDocLink) const handleLocalImagePickRef = useRef<() => void>(() => {}) const openSearchRef = useRef<() => void>(() => {}) + const openAnnotationPopoverRef = useRef<(requireLiveSelection?: boolean) => boolean>(() => false) // Why: ProseMirror keeps the initial handleKeyDown closure, so `editor` stays // stuck at the first-render null value unless we read the live instance here. const editorRef = useRef(null) @@ -145,17 +140,10 @@ export default function RichMarkdownEditor({ worktreeId, worktreeRoot }) - // Why: building the table of contents runs a full-document remark parse on - // every content change. The result is only used while the panel is open - // (closed by default), so gate the parse on visibility; including - // showTableOfContents in deps rebuilds the outline the moment it opens. - const tableOfContentsItems = useMemo( - () => selectMarkdownTableOfContents(showTableOfContents, content), - [content, showTableOfContents] - ) - const flatTableOfContentsItems = useMemo( - () => flattenMarkdownTocItems(tableOfContentsItems), - [tableOfContentsItems] + const { tableOfContentsItems, navigateToTableOfContentsItem } = useRichMarkdownTableOfContents( + showTableOfContents, + content, + scrollContainerRef ) // Why: assigning callback refs during render keeps them current before any @@ -166,6 +154,7 @@ export default function RichMarkdownEditor({ onSaveRef.current = onSave onOpenDocLinkRef.current = onOpenDocLink isEditingLinkRef.current = isEditingLink + openAnnotationPopoverRef.current = review.openAnnotationPopover const reconcileRoundTripRef = useRichMarkdownReconcileRoundTrip({ htmlSuperscriptLinkContext, filePath, @@ -258,6 +247,7 @@ export default function RichMarkdownEditor({ markdownSourceLineOffsetRef: review.markdownSourceLineOffsetRef, flushPendingSerialization, openSearchRef, + openAnnotationPopoverRef, syncAnnotationTarget: review.syncAnnotationTarget, clearAnnotationTarget: review.clearAnnotationTarget, scrollRichMarkdownReviewNoteCardIntoView: review.scrollRichMarkdownReviewNoteCardIntoView, @@ -360,18 +350,6 @@ export default function RichMarkdownEditor({ }) openSearchRef.current = openSearch - const navigateToTableOfContentsItem = useCallback( - (id: string): void => { - const container = scrollContainerRef.current - if (!container) { - return - } - const heading = findRichMarkdownTocHeadingTarget(container, flatTableOfContentsItems, id) - heading?.scrollIntoView({ block: 'center' }) - }, - [flatTableOfContentsItems] - ) - return ( ({ })) import { + installEditorAddReviewNoteShortcut, installEditorFindShortcut, installMonacoDiffChangeNavigationShortcut, installMonacoEditorFindShortcut @@ -201,6 +202,74 @@ describe('installEditorFindShortcut', () => { }) }) +describe('installEditorAddReviewNoteShortcut', () => { + it('invokes add-review-note on its default binding and honors overrides', () => { + const container = document.createElement('div') + const input = document.createElement('textarea') + const onAddReviewNote = vi.fn(() => true) + container.appendChild(input) + document.body.appendChild(container) + const dispose = installEditorAddReviewNoteShortcut(container, onAddReviewNote) + + const defaultEvent = dispatchKeyDown(input, { + key: 'n', + code: 'KeyN', + metaKey: true, + altKey: true + }) + const repeatEvent = dispatchKeyDown(input, { + key: 'n', + code: 'KeyN', + metaKey: true, + altKey: true, + repeat: true + }) + const unrelatedEvent = dispatchKeyDown(input, { key: 'n', code: 'KeyN', metaKey: true }) + + expect(defaultEvent.defaultPrevented).toBe(true) + expect(repeatEvent.defaultPrevented).toBe(false) + expect(unrelatedEvent.defaultPrevented).toBe(false) + expect(onAddReviewNote).toHaveBeenCalledTimes(1) + + shortcutState.keybindings = { 'editor.addReviewNote': ['Mod+Shift+A'] } + const overriddenEvent = dispatchKeyDown(input, { + key: 'a', + code: 'KeyA', + metaKey: true, + shiftKey: true + }) + expect(overriddenEvent.defaultPrevented).toBe(true) + expect(onAddReviewNote).toHaveBeenCalledTimes(2) + + dispose() + dispatchKeyDown(input, { key: 'a', code: 'KeyA', metaKey: true, shiftKey: true }) + expect(onAddReviewNote).toHaveBeenCalledTimes(2) + }) + + it('leaves the chord unconsumed when the handler reports it did not act', () => { + const container = document.createElement('div') + const input = document.createElement('textarea') + const onDownstreamKeyDown = vi.fn() + const onAddReviewNote = vi.fn(() => false) + container.appendChild(input) + document.body.appendChild(container) + input.addEventListener('keydown', onDownstreamKeyDown) + const dispose = installEditorAddReviewNoteShortcut(container, onAddReviewNote) + + const event = dispatchKeyDown(input, { + key: 'n', + code: 'KeyN', + metaKey: true, + altKey: true + }) + + expect(onAddReviewNote).toHaveBeenCalledTimes(1) + expect(event.defaultPrevented).toBe(false) + expect(onDownstreamKeyDown).toHaveBeenCalledTimes(1) + dispose() + }) +}) + describe('installMonacoDiffChangeNavigationShortcut', () => { function createDiffNavigationFixture(): { container: HTMLDivElement diff --git a/src/renderer/src/components/editor/editor-shortcuts.ts b/src/renderer/src/components/editor/editor-shortcuts.ts index 825caf6f1..5038498a5 100644 --- a/src/renderer/src/components/editor/editor-shortcuts.ts +++ b/src/renderer/src/components/editor/editor-shortcuts.ts @@ -79,6 +79,27 @@ export function installMonacoDiffChangeNavigationShortcut( return () => target.removeEventListener('keydown', handleKeyDown, true) } +export function installEditorAddReviewNoteShortcut( + target: HTMLElement, + onAddReviewNote: () => boolean +): () => void { + const handleKeyDown = (event: KeyboardEvent): void => { + if (event.repeat || !editorShortcutMatches('editor.addReviewNote', event)) { + return + } + // Why: only consume the chord when a composer actually opens; on files + // where review notes can never apply the key must stay available to + // whatever else the user bound it to. + if (onAddReviewNote()) { + event.preventDefault() + event.stopPropagation() + } + } + + target.addEventListener('keydown', handleKeyDown, true) + return () => target.removeEventListener('keydown', handleKeyDown, true) +} + type MonacoFindShortcutEditor = { getAction: (id: string) => { run: () => void | Promise } | null getContainerDomNode: () => HTMLElement diff --git a/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts b/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts new file mode 100644 index 000000000..ed16221df --- /dev/null +++ b/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.test.ts @@ -0,0 +1,95 @@ +// @vitest-environment happy-dom + +import { afterEach, describe, expect, it } from 'vitest' +import { + getMarkdownAnnotationBlockKeyForSelection, + isMarkdownPreviewAddReviewNoteShortcut +} from './markdown-preview-annotation-shortcut' + +function createPreviewFixture(): { + root: HTMLDivElement + block: HTMLDivElement + paragraph: HTMLParagraphElement +} { + const root = document.createElement('div') + const block = document.createElement('div') + block.className = 'markdown-annotation-block' + block.setAttribute('data-annotation-block-key', 'p:3-5') + const paragraph = document.createElement('p') + paragraph.textContent = 'Some rendered markdown text' + block.appendChild(paragraph) + root.appendChild(block) + document.body.appendChild(root) + return { root, block, paragraph } +} + +function selectTextIn(node: Node): Selection { + const selection = window.getSelection() + if (!selection) { + throw new Error('Selection API unavailable in test environment') + } + const range = document.createRange() + range.selectNodeContents(node) + selection.removeAllRanges() + selection.addRange(range) + return selection +} + +afterEach(() => { + window.getSelection()?.removeAllRanges() + document.body.replaceChildren() +}) + +describe('getMarkdownAnnotationBlockKeyForSelection', () => { + it('returns the block key for a selection inside an annotation block', () => { + const { root, paragraph } = createPreviewFixture() + const selection = selectTextIn(paragraph) + + expect(getMarkdownAnnotationBlockKeyForSelection(root, selection)).toBe('p:3-5') + }) + + it('returns null for a collapsed selection', () => { + const { root, paragraph } = createPreviewFixture() + const selection = selectTextIn(paragraph) + selection.collapseToStart() + + expect(getMarkdownAnnotationBlockKeyForSelection(root, selection)).toBeNull() + }) + + it('returns null when the selection is outside the preview root', () => { + const { root } = createPreviewFixture() + const outside = document.createElement('p') + outside.textContent = 'other text' + document.body.appendChild(outside) + const selection = selectTextIn(outside) + + expect(getMarkdownAnnotationBlockKeyForSelection(root, selection)).toBeNull() + }) + + it('returns null without a selection', () => { + const { root } = createPreviewFixture() + + expect(getMarkdownAnnotationBlockKeyForSelection(root, null)).toBeNull() + }) +}) + +describe('isMarkdownPreviewAddReviewNoteShortcut', () => { + it('matches the default binding and respects overrides', () => { + const defaultEvent = { + key: 'n', + code: 'KeyN', + metaKey: true, + ctrlKey: false, + altKey: true, + shiftKey: false + } + + expect(isMarkdownPreviewAddReviewNoteShortcut(defaultEvent, 'darwin')).toBe(true) + expect(isMarkdownPreviewAddReviewNoteShortcut(defaultEvent, 'linux')).toBe(false) + expect( + isMarkdownPreviewAddReviewNoteShortcut(defaultEvent, 'darwin', { + 'editor.addReviewNote': ['Mod+Shift+A'] + }) + ).toBe(false) + }) +}) diff --git a/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.ts b/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.ts new file mode 100644 index 000000000..b64dc4c67 --- /dev/null +++ b/src/renderer/src/components/editor/markdown-preview-annotation-shortcut.ts @@ -0,0 +1,38 @@ +import { keybindingMatchesAction, type KeybindingOverrides } from '../../../../shared/keybindings' + +export function isMarkdownPreviewAddReviewNoteShortcut( + event: Pick, + platform: NodeJS.Platform, + keybindings?: KeybindingOverrides +): boolean { + return keybindingMatchesAction('editor.addReviewNote', event, platform, keybindings) +} + +function closestAnnotationBlockKey(node: Node | null, root: HTMLElement): string | null { + const element = node instanceof Element ? node : (node?.parentElement ?? null) + const block = element?.closest('[data-annotation-block-key]') ?? null + if (!block || !root.contains(block)) { + return null + } + return block.getAttribute('data-annotation-block-key') +} + +/** + * Maps the current DOM text selection to the annotation block that should host + * the review-note composer. Returns null when the selection is collapsed or + * falls outside an annotatable block of this preview root. + */ +export function getMarkdownAnnotationBlockKeyForSelection( + root: HTMLElement, + selection: Selection | null +): string | null { + if (!selection || selection.isCollapsed || selection.rangeCount === 0) { + return null + } + // Why: a selection spanning multiple blocks anchors the composer on the + // block where the selection started, falling back to where it ended. + return ( + closestAnnotationBlockKey(selection.anchorNode, root) ?? + closestAnnotationBlockKey(selection.focusNode, root) + ) +} diff --git a/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts b/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts new file mode 100644 index 000000000..318154d41 --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-annotation-shortcut.ts @@ -0,0 +1,21 @@ +import type { KeyHandlerContext } from './rich-markdown-key-handler' +import { editorShortcutMatches } from './editor-shortcuts' + +/** + * Mod+Alt+N: open the review-note composer for the current selection. + */ +export function handleRichMarkdownAddReviewNoteShortcut( + ctx: KeyHandlerContext, + event: KeyboardEvent +): boolean { + if (!editorShortcutMatches('editor.addReviewNote', event)) { + return false + } + // Why: require the live selection so a collapsed selection cannot reopen a + // stale target; consume the chord only when a composer actually opens. + if (!ctx.openAnnotationPopoverRef.current(true)) { + return false + } + event.preventDefault() + return true +} diff --git a/src/renderer/src/components/editor/rich-markdown-editor-config.test.ts b/src/renderer/src/components/editor/rich-markdown-editor-config.test.ts index 8573ddea9..dd58ff408 100644 --- a/src/renderer/src/components/editor/rich-markdown-editor-config.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-editor-config.test.ts @@ -73,6 +73,7 @@ function createConfigParams(overrides: Partial = {}): Editor markdownSourceLineOffsetRef: ref(0), flushPendingSerialization: vi.fn(), openSearchRef: ref(vi.fn()), + openAnnotationPopoverRef: ref(vi.fn()), syncAnnotationTarget: vi.fn(), clearAnnotationTarget: vi.fn(), scrollRichMarkdownReviewNoteCardIntoView: vi.fn(), diff --git a/src/renderer/src/components/editor/rich-markdown-editor-config.ts b/src/renderer/src/components/editor/rich-markdown-editor-config.ts index ec94eda94..5877d9eb3 100644 --- a/src/renderer/src/components/editor/rich-markdown-editor-config.ts +++ b/src/renderer/src/components/editor/rich-markdown-editor-config.ts @@ -77,6 +77,7 @@ export type EditorConfigParams = { markdownSourceLineOffsetRef: MutableRefObject flushPendingSerialization: () => void openSearchRef: MutableRefObject<() => void> + openAnnotationPopoverRef: MutableRefObject<(requireLiveSelection?: boolean) => boolean> syncAnnotationTarget: (editor: Editor) => void clearAnnotationTarget: () => void scrollRichMarkdownReviewNoteCardIntoView: (commentId: string) => void @@ -109,17 +110,7 @@ export function createRichMarkdownEditorConfig(params: EditorConfigParams): UseE reconcileRoundTripRef, onContentChangeRef, onDirtyStateHintRef, - onSaveRef, onOpenDocLinkRef, - isEditingLinkRef, - slashMenuRef, - filteredSlashCommandsRef, - selectedCommandIndexRef, - docLinkMenuRef, - filteredDocLinkRowsRef, - selectedDocLinkIndexRef, - handleLocalImagePickRef, - handleEmojiPickRef, typedEmptyOrderedListMarkerRef, cancelAutoFocusRef, serializeTimerRef, @@ -127,15 +118,11 @@ export function createRichMarkdownEditorConfig(params: EditorConfigParams): UseE isApplyingProgrammaticUpdateRef, markdownCommentsRef, markdownSourceLineOffsetRef, - flushPendingSerialization, - openSearchRef, syncAnnotationTarget, clearAnnotationTarget, scrollRichMarkdownReviewNoteCardIntoView, setIsEditingLink, setLinkBubble, - setSelectedCommandIndex, - setSelectedDocLinkIndex, setSlashMenu, setDocLinkMenu } = params @@ -172,36 +159,12 @@ export function createRichMarkdownEditorConfig(params: EditorConfigParams): UseE typedEmptyOrderedListMarkerRef.current = /^\d+\.$/.test(beforeCursor) return false }, + // Why: KeyHandlerContext is a typed subset of EditorConfigParams, so the + // spread stays type-checked while new context fields avoid re-listing + // every ref here. handleKeyDown: createRichMarkdownKeyHandler({ - isMac, - editorRef, - rootRef, - lastCommittedMarkdownRef, - originalSourceRef, - baseCanonicalRef, - reconcileRoundTripRef, - onContentChangeRef, - onSaveRef, - isEditingLinkRef, - slashMenuRef, - filteredSlashCommandsRef, - selectedCommandIndexRef, - docLinkMenuRef, - filteredDocLinkRowsRef, - selectedDocLinkIndexRef, - handleLocalImagePickRef, - handleEmojiPickRef, - typedEmptyOrderedListMarkerRef, - flushPendingSerialization, - openSearchRef, + ...params, linkBubbleOwnerId: codec.transport.key, - htmlSuperscriptLinkContext, - setIsEditingLink, - setLinkBubble, - setSelectedCommandIndex, - setSelectedDocLinkIndex, - setSlashMenu, - setDocLinkMenu, openSelectedHtmlSuperscriptLink: () => openSelectedHtmlSuperscriptLink({ activateMarkdownLink, diff --git a/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts b/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts index 02d53e1b6..ebd71f84f 100644 --- a/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-key-handler.test.ts @@ -4,6 +4,12 @@ import StarterKit from '@tiptap/starter-kit' import { createIsolatedMarkdownExtensionForTests } from './isolated-markdown-extension-for-tests' import { createRichMarkdownKeyHandler, type KeyHandlerContext } from './rich-markdown-key-handler' +// Why: keybinding matching resolves the platform from navigator.userAgent, +// which is environment-dependent under vitest; pin it for determinism. +vi.mock('@/lib/shortcut-platform', () => ({ + getShortcutPlatform: () => 'darwin' as NodeJS.Platform +})) + const extensions = [StarterKit, createIsolatedMarkdownExtensionForTests()] function createEditor(content: object): Editor { @@ -82,6 +88,7 @@ function createContext(editor: Editor, typedMarker: boolean): KeyHandlerContext subscribe: () => () => {}, update: () => {} }, + openAnnotationPopoverRef: { current: vi.fn(() => true) }, setIsEditingLink: vi.fn(), setLinkBubble: vi.fn(), setSelectedCommandIndex: vi.fn(), @@ -105,6 +112,37 @@ function emptyTopLevelOrderedList(): object { } describe('rich markdown key handler', () => { + it('opens the review-note composer on the add-review-note shortcut', () => { + const editor = createEditor(emptyTopLevelOrderedList()) + + try { + const ctx = createContext(editor, false) + const event = keyEvent('n', { metaKey: true, altKey: true, code: 'KeyN' }) + + expect(createRichMarkdownKeyHandler(ctx)(null, event)).toBe(true) + expect(event.preventDefault).toHaveBeenCalled() + expect(ctx.openAnnotationPopoverRef.current).toHaveBeenCalledWith(true) + } finally { + editor.destroy() + } + }) + + it('leaves the add-review-note chord unconsumed when no composer opens', () => { + const editor = createEditor(emptyTopLevelOrderedList()) + + try { + const ctx = createContext(editor, false) + ctx.openAnnotationPopoverRef.current = vi.fn(() => false) + const event = keyEvent('n', { metaKey: true, altKey: true, code: 'KeyN' }) + + expect(createRichMarkdownKeyHandler(ctx)(null, event)).toBe(false) + expect(event.preventDefault).not.toHaveBeenCalled() + expect(ctx.openAnnotationPopoverRef.current).toHaveBeenCalledTimes(1) + } finally { + editor.destroy() + } + }) + it('preserves a typed empty ordered-list shortcut on Enter', () => { const editor = createEditor(emptyTopLevelOrderedList()) diff --git a/src/renderer/src/components/editor/rich-markdown-key-handler.ts b/src/renderer/src/components/editor/rich-markdown-key-handler.ts index d8995f3c9..b0a223887 100644 --- a/src/renderer/src/components/editor/rich-markdown-key-handler.ts +++ b/src/renderer/src/components/editor/rich-markdown-key-handler.ts @@ -3,6 +3,7 @@ import type { Editor } from '@tiptap/react' import { getShortcutPlatform } from '@/lib/shortcut-platform' import { useAppStore } from '@/store' import { isMarkdownPreviewFindShortcut } from './markdown-preview-search' +import { handleRichMarkdownAddReviewNoteShortcut } from './rich-markdown-annotation-shortcut' import type { LinkBubbleState } from './RichMarkdownLinkBubble' import { commitRow, type DocLinkMenuRow, type DocLinkMenuState } from './rich-markdown-commands' import { @@ -21,6 +22,7 @@ import { handleRichMarkdownCitationKey } from './rich-markdown-citation-keyboard import type { RichMarkdownHtmlSuperscriptLinkContext } from './rich-markdown-html-superscript-link-context' import { handleRichMarkdownLinkShortcut } from './rich-markdown-link-shortcut' import { handleRichMarkdownSaveShortcut } from './rich-markdown-save-shortcut' +import { flushPendingProseMirrorSelection } from './rich-markdown-selection-flush' export type KeyHandlerContext = { isMac: boolean @@ -44,6 +46,7 @@ export type KeyHandlerContext = { typedEmptyOrderedListMarkerRef: MutableRefObject flushPendingSerialization: () => void openSearchRef: MutableRefObject<() => void> + openAnnotationPopoverRef: MutableRefObject<(requireLiveSelection?: boolean) => boolean> setIsEditingLink: (editing: boolean) => void setLinkBubble: (bubble: LinkBubbleState | null) => void setSelectedCommandIndex: Dispatch> @@ -59,47 +62,6 @@ function isComposingMarkdownInput(event: KeyboardEvent, editor: Editor | null): return event.isComposing || editor?.view.composing === true } -type NativeSelectionSnapshot = { - anchorNode: Node | null - anchorOffset: number - focusNode: Node | null - focusOffset: number -} - -type ProseMirrorDomObserver = { - currentSelection?: { - set?: (selection: NativeSelectionSnapshot) => void - } - flush?: () => void -} - -type ProseMirrorViewWithDomObserver = Editor['view'] & { - domObserver?: ProseMirrorDomObserver -} - -function flushPendingProseMirrorSelection(editor: Editor): void { - let observer: ProseMirrorDomObserver | undefined - try { - observer = (editor.view as ProseMirrorViewWithDomObserver).domObserver - } catch { - return - } - - if (typeof observer?.flush !== 'function') { - return - } - - // Why: immediate Tab after a mouse click can run before ProseMirror has - // copied the native selection into editor state, so list commands hit stale item state. - observer.currentSelection?.set?.({ - anchorNode: null, - anchorOffset: 0, - focusNode: null, - focusOffset: 0 - }) - observer.flush() -} - /** * Why: extracted from RichMarkdownEditor to stay under the file line-limit * while keeping the keyboard handler logic co-located and testable. @@ -133,6 +95,9 @@ export function createRichMarkdownKeyHandler( if (handleRichMarkdownSaveShortcut(ctx, event)) { return true } + if (handleRichMarkdownAddReviewNoteShortcut(ctx, event)) { + return true + } // Strikethrough: Cmd/Ctrl+Shift+X (standard shortcut used by Google // Docs, Notion, etc. — supplements Tiptap's built-in Mod+Shift+S). diff --git a/src/renderer/src/components/editor/rich-markdown-selection-flush.ts b/src/renderer/src/components/editor/rich-markdown-selection-flush.ts new file mode 100644 index 000000000..a7dd336ed --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-selection-flush.ts @@ -0,0 +1,42 @@ +import type { Editor } from '@tiptap/react' + +type NativeSelectionSnapshot = { + anchorNode: Node | null + anchorOffset: number + focusNode: Node | null + focusOffset: number +} + +type ProseMirrorDomObserver = { + currentSelection?: { + set?: (selection: NativeSelectionSnapshot) => void + } + flush?: () => void +} + +type ProseMirrorViewWithDomObserver = Editor['view'] & { + domObserver?: ProseMirrorDomObserver +} + +export function flushPendingProseMirrorSelection(editor: Editor): void { + let observer: ProseMirrorDomObserver | undefined + try { + observer = (editor.view as ProseMirrorViewWithDomObserver).domObserver + } catch { + return + } + + if (typeof observer?.flush !== 'function') { + return + } + + // Why: immediate Tab after a mouse click can run before ProseMirror has + // copied the native selection into editor state, so list commands hit stale item state. + observer.currentSelection?.set?.({ + anchorNode: null, + anchorOffset: 0, + focusNode: null, + focusOffset: 0 + }) + observer.flush() +} diff --git a/src/renderer/src/components/editor/rich-markdown-tab-key-handler.test.ts b/src/renderer/src/components/editor/rich-markdown-tab-key-handler.test.ts index b9675d5e3..de6b9934d 100644 --- a/src/renderer/src/components/editor/rich-markdown-tab-key-handler.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-tab-key-handler.test.ts @@ -89,6 +89,7 @@ function createContext(editor: Editor): KeyHandlerContext { subscribe: () => () => {}, update: () => {} }, + openAnnotationPopoverRef: { current: vi.fn() }, setIsEditingLink: vi.fn(), setLinkBubble: vi.fn(), setSelectedCommandIndex: vi.fn(), diff --git a/src/renderer/src/components/editor/use-rich-markdown-table-of-contents.ts b/src/renderer/src/components/editor/use-rich-markdown-table-of-contents.ts new file mode 100644 index 000000000..de2040627 --- /dev/null +++ b/src/renderer/src/components/editor/use-rich-markdown-table-of-contents.ts @@ -0,0 +1,44 @@ +import { useCallback, useMemo, type MutableRefObject } from 'react' +import type { MarkdownTocItem } from './markdown-table-of-contents' +import { findRichMarkdownTocHeadingTarget } from './rich-markdown-toc-heading-target' +import { selectMarkdownTableOfContents } from './markdown-toc-visibility-gate' + +function flattenMarkdownTocItems(items: MarkdownTocItem[]): MarkdownTocItem[] { + return items.flatMap((item) => [item, ...flattenMarkdownTocItems(item.children)]) +} + +export function useRichMarkdownTableOfContents( + showTableOfContents: boolean, + content: string, + scrollContainerRef: MutableRefObject +): { + tableOfContentsItems: MarkdownTocItem[] + navigateToTableOfContentsItem: (id: string) => void +} { + // Why: building the table of contents runs a full-document remark parse on + // every content change. The result is only used while the panel is open + // (closed by default), so gate the parse on visibility; including + // showTableOfContents in deps rebuilds the outline the moment it opens. + const tableOfContentsItems = useMemo( + () => selectMarkdownTableOfContents(showTableOfContents, content), + [content, showTableOfContents] + ) + const flatTableOfContentsItems = useMemo( + () => flattenMarkdownTocItems(tableOfContentsItems), + [tableOfContentsItems] + ) + + const navigateToTableOfContentsItem = useCallback( + (id: string): void => { + const container = scrollContainerRef.current + if (!container) { + return + } + const heading = findRichMarkdownTocHeadingTarget(container, flatTableOfContentsItems, id) + heading?.scrollIntoView({ block: 'center' }) + }, + [flatTableOfContentsItems, scrollContainerRef] + ) + + return { tableOfContentsItems, navigateToTableOfContentsItem } +} diff --git a/src/renderer/src/components/editor/useRichMarkdownReviewController.ts b/src/renderer/src/components/editor/useRichMarkdownReviewController.ts index 43bade891..82b408006 100644 --- a/src/renderer/src/components/editor/useRichMarkdownReviewController.ts +++ b/src/renderer/src/components/editor/useRichMarkdownReviewController.ts @@ -201,41 +201,51 @@ export function useRichMarkdownReviewController({ ] ) - const openAnnotationPopover = useCallback((): void => { - if (!annotationTarget || !canAnnotateRichMarkdown) { - return - } - const editor = editorRef.current - const root = rootRef.current - const liveTarget = editor && root ? getRichMarkdownAnnotationTarget(editor, root) : null - const target = editor - ? clampRichMarkdownAnnotationTarget(editor, liveTarget ?? annotationTarget) - : annotationTarget - if ( - !target || - hasRichMarkdownCommentForRange(markdownComments, target, markdownSourceLineOffset) - ) { + // Why: reports whether a composer actually opened so keyboard callers can + // leave the chord unconsumed when this no-ops (mouse callers ignore it). + const openAnnotationPopover = useCallback( + (requireLiveSelection = false): boolean => { + if (!canAnnotateRichMarkdown) { + return false + } + const editor = editorRef.current + const root = rootRef.current + // Why: keyboard callers require the live selection to avoid stale-target + // races; the mouse button may use the target from the render that exposed it. + const liveTarget = editor && root ? getRichMarkdownAnnotationTarget(editor, root) : null + const baseTarget = liveTarget ?? (requireLiveSelection ? null : annotationTarget) + if (!baseTarget) { + return false + } + const target = editor ? clampRichMarkdownAnnotationTarget(editor, baseTarget) : baseTarget + if ( + !target || + hasRichMarkdownCommentForRange(markdownComments, target, markdownSourceLineOffset) + ) { + setAnnotationTarget(null) + return false + } + editor?.view.dispatch( + editor.state.tr.setMeta(richMarkdownAnnotationHighlightPluginKey, { + activeRange: { from: target.from, to: target.to } + }) + ) + // Why: opening a draft should reserve the notes rail immediately; saved notes stay visible. + setReviewRailOpen(true) + setAnnotationPopover(target) setAnnotationTarget(null) - return - } - editor?.view.dispatch( - editor.state.tr.setMeta(richMarkdownAnnotationHighlightPluginKey, { - activeRange: { from: target.from, to: target.to } - }) - ) - // Why: opening a draft should reserve the notes rail immediately; saved notes stay visible. - setReviewRailOpen(true) - setAnnotationPopover(target) - setAnnotationTarget(null) - }, [ - annotationTarget, - canAnnotateRichMarkdown, - editorRef, - markdownComments, - markdownSourceLineOffset, - rootRef, - setReviewRailOpen - ]) + return true + }, + [ + annotationTarget, + canAnnotateRichMarkdown, + editorRef, + markdownComments, + markdownSourceLineOffset, + rootRef, + setReviewRailOpen + ] + ) useEffect(() => { if (canAnnotateRichMarkdown) { diff --git a/src/shared/keybindings.test.ts b/src/shared/keybindings.test.ts index 4bf8344f4..db6ef52f1 100644 --- a/src/shared/keybindings.test.ts +++ b/src/shared/keybindings.test.ts @@ -197,6 +197,16 @@ describe('keybindings', () => { expect(formatKeybindingList(['Mod+Shift+O'], 'darwin')).toBe('⌘⇧O') }) + it('defines a default shortcut for adding an editor review note', () => { + expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'darwin')).toEqual([ + 'Mod+Alt+N' + ]) + expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'linux')).toEqual(['Mod+Alt+N']) + expect(getEffectiveKeybindingsForAction('editor.addReviewNote', 'win32')).toEqual(['Mod+Alt+N']) + expect(formatKeybindingList(['Mod+Alt+N'], 'darwin')).toBe('⌘⌥N') + expect(formatKeybindingList(['Mod+Alt+N'], 'linux')).toBe('Ctrl+Alt+N') + }) + it('defines platform-native replace-in-editor shortcuts', () => { expect(getEffectiveKeybindingsForAction('editor.replace', 'darwin')).toEqual(['Mod+Alt+F']) expect(getEffectiveKeybindingsForAction('editor.replace', 'linux')).toEqual(['Mod+H']) diff --git a/src/shared/keybindings.ts b/src/shared/keybindings.ts index f3c04d2ac..097cb661e 100644 --- a/src/shared/keybindings.ts +++ b/src/shared/keybindings.ts @@ -92,6 +92,7 @@ export type KeybindingActionId = | 'editor.copyContext' | 'editor.previousChange' | 'editor.nextChange' + | 'editor.addReviewNote' | 'fileExplorer.undo' | 'fileExplorer.redo' | 'fileExplorer.copyPath' @@ -852,6 +853,14 @@ export const KEYBINDING_DEFINITIONS: readonly KeybindingDefinition[] = [ defaultBindings: platformBindings(['F7']), allowBareKeybindings: true }, + { + id: 'editor.addReviewNote', + title: 'Add Review Note', + group: 'Editors', + scope: 'editor', + searchKeywords: ['shortcut', 'editor', 'markdown', 'note', 'comment', 'annotation', 'review'], + defaultBindings: platformBindings(['Mod+Alt+N']) + }, { id: 'fileExplorer.undo', title: 'Undo file operation', diff --git a/tests/e2e/markdown-add-review-note-shortcut.spec.ts b/tests/e2e/markdown-add-review-note-shortcut.spec.ts new file mode 100644 index 000000000..3fb16f5c0 --- /dev/null +++ b/tests/e2e/markdown-add-review-note-shortcut.spec.ts @@ -0,0 +1,184 @@ +import { test, expect } from './helpers/orca-app' +import { waitForActiveWorktree, waitForSessionReady } from './helpers/store' +import { + cleanupMarkdownFixture, + createMarkdownFixture, + getActiveWorktreeContext, + openMarkdownFixture, + waitForRichMarkdownEditor +} from './helpers/markdown-ordered-list-exit' + +test.describe('Markdown add-review-note shortcut', () => { + test.beforeEach(async ({ orcaPage }) => { + await waitForSessionReady(orcaPage) + await waitForActiveWorktree(orcaPage) + }) + + test('opens the review-note composer for the current selection in the rich editor', async ({ + orcaPage + }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + let filePath: string | null = null + + try { + filePath = await createMarkdownFixture( + context, + 'add-review-note', + testInfo.workerIndex, + 'A paragraph to annotate with a review note.\n' + ) + await openMarkdownFixture(orcaPage, context, filePath) + const editor = await waitForRichMarkdownEditor(orcaPage) + await editor.click() + await orcaPage.keyboard.press('ControlOrMeta+A') + + await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + + await expect(orcaPage.getByPlaceholder('Add note for the AI')).toBeVisible({ + timeout: 5_000 + }) + } finally { + await cleanupMarkdownFixture(filePath) + } + }) + + test('opens the composer for the current selection in the Monaco source editor', async ({ + orcaPage + }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + let filePath: string | null = null + + try { + filePath = await createMarkdownFixture( + context, + 'add-review-note-source', + testInfo.workerIndex, + 'A paragraph to annotate from the source editor.\n' + ) + await openMarkdownFixture(orcaPage, context, filePath) + await waitForRichMarkdownEditor(orcaPage) + await orcaPage.evaluate(() => { + // Why: switch to source mode through the store — the toolbar toggle is + // an icon menu that is brittle to locate; the store action is what it + // dispatches anyway, and the shortcut under test is mode-independent. + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + if (!state.activeFileId) { + throw new Error('No active editor file') + } + state.setMarkdownViewMode(state.activeFileId, 'source') + }) + const monaco = orcaPage.locator('.monaco-editor').first() + await expect(monaco).toBeVisible({ timeout: 25_000 }) + await monaco.click() + await orcaPage.keyboard.press('ControlOrMeta+A') + + await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + + await expect(orcaPage.getByPlaceholder('Add note for the AI')).toBeVisible({ + timeout: 5_000 + }) + } finally { + await cleanupMarkdownFixture(filePath) + } + }) + + test('opens the inline composer for the selected block in the markdown preview', async ({ + orcaPage + }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + let filePath: string | null = null + + try { + filePath = await createMarkdownFixture( + context, + 'add-review-note-preview', + testInfo.workerIndex, + 'A paragraph to annotate from the preview.\n' + ) + await openMarkdownFixture(orcaPage, context, filePath) + await waitForRichMarkdownEditor(orcaPage) + await orcaPage.evaluate(() => { + // Why: open the real markdown-preview tab through the store — preview + // is a separate file mode, not a view mode of the edit tab, and the + // toolbar entry point is an icon menu that is brittle to locate. + const store = window.__store + if (!store) { + throw new Error('window.__store is not available') + } + const state = store.getState() + const file = state.openFiles.find((f) => f.id === state.activeFileId) + if (!file) { + throw new Error('No active editor file') + } + state.openMarkdownPreview( + { + filePath: file.filePath, + relativePath: file.relativePath, + worktreeId: file.worktreeId, + runtimeEnvironmentId: file.runtimeEnvironmentId, + language: 'markdown' + }, + { sourceFileId: file.id } + ) + }) + await expect(orcaPage.locator('[data-annotation-block-key]').first()).toBeVisible({ + timeout: 25_000 + }) + + await orcaPage.evaluate(() => { + // Why: mirror a reader selecting rendered text — focus lands on the + // preview's tabIndex=0 root and the DOM selection covers the block. + const block = document.querySelector('[data-annotation-block-key]') + if (!block) { + throw new Error('No annotation block found in preview') + } + const focusable = block.closest('[tabindex]') + if (!focusable) { + throw new Error('No focusable preview root above the annotation block') + } + focusable.focus() + const paragraph = block.querySelector('p') ?? block + const selection = window.getSelection() + const range = document.createRange() + range.selectNodeContents(paragraph) + selection?.removeAllRanges() + selection?.addRange(range) + }) + + await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + + await expect(orcaPage.getByPlaceholder('Add note for the AI')).toBeVisible({ + timeout: 5_000 + }) + } finally { + await cleanupMarkdownFixture(filePath) + } + }) + + test('does not open the composer without a text selection', async ({ orcaPage }, testInfo) => { + const context = await getActiveWorktreeContext(orcaPage) + let filePath: string | null = null + + try { + filePath = await createMarkdownFixture( + context, + 'add-review-note-no-selection', + testInfo.workerIndex, + 'A paragraph without any selection.\n' + ) + await openMarkdownFixture(orcaPage, context, filePath) + const editor = await waitForRichMarkdownEditor(orcaPage) + await editor.click() + + await orcaPage.keyboard.press('ControlOrMeta+Alt+N') + + await expect(orcaPage.getByPlaceholder('Add note for the AI')).toHaveCount(0) + } finally { + await cleanupMarkdownFixture(filePath) + } + }) +})