feat(editor): bindable keyboard shortcut to add a markdown review note (Mod+Alt+N) (#8250)

* feat(editor): bindable shortcut to add a markdown review note

Adds editor.addReviewNote (default Mod+Alt+N) to the shared keybinding
registry and wires it into all three markdown surfaces: the rich editor
key handler invokes the annotation popover opener, the Monaco editor
installs a keydown listener that opens the composer for the tracked
selection target, and the preview maps the DOM selection to its
annotation block. openAnnotationPopover now prefers the live selection
target over synced state so the shortcut works even before the sync
render lands.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYtipbTz8N4woN1sxTK1ia

* fix(editor): cover list items and Monaco path for add-review-note shortcut

Tag the preview's list-item annotation blocks with data-annotation-block-key
so the shortcut resolves selections inside li blocks (review feedback), and
extend the e2e spec to drive the Monaco source-editor wiring.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYtipbTz8N4woN1sxTK1ia

* docs(e2e): explain store-driven view-mode switch in add-review-note spec

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XYtipbTz8N4woN1sxTK1ia

* refactor(editor): extract add-review-note + selection-flush modules to satisfy max-lines after rebase

* refactor(editor): spread key-handler params and extract TOC hook to satisfy max-lines

* test(editor): move add-review-note installer test into its own describe

* fix(editor): pass add-review-note chord through when Monaco cannot act; cover preview surface e2e

* fix(editor): unify add-review-note chord consumption — consume only when a composer opens

* fix(editor): gate list-item annotation block key on composer availability

* fix(editor): require live selection for keyboard add-review-note

* chore: retrigger CI against current main (merge ref built during transient main breakage at 6e91ca6c0)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Brennan Benson <79079362+brennanb2025@users.noreply.github.com>
This commit is contained in:
Guillermo Avelar 2026-07-16 23:10:15 -05:00 committed by GitHub
parent cc1ad064d7
commit 1b331f282c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
19 changed files with 688 additions and 155 deletions

View File

@ -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<void> => {
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 (
<li {...props}>
<div
@ -1676,14 +1700,13 @@ export default function MarkdownPreview({
}`.trim()}
data-source-line={range.startLine}
data-source-end-line={range.endLine}
// Why: only advertise the block to the add-review-note shortcut
// when the composer can actually render (mirrors wrapAnnotatedBlock).
data-annotation-block-key={controls ? blockKey : undefined}
onClick={(event) => handleAnnotatedMarkdownBlockClick(range, event)}
>
<span className="markdown-annotation-list-content">{children}</span>
{renderAnnotationControls(
range,
blockKey,
getMarkdownPreviewAnnotationQuote(children)
)}
{controls}
</div>
</li>
)

View File

@ -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<MarkdownCommentPopoverState | null>(null)
const [selectionAnnotationTarget, setSelectionAnnotationTarget] =
useState<MonacoMarkdownSelectionAnnotationTarget | null>(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<MonacoMarkdownSelectionAnnotationTarget | null>(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()

View File

@ -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<Editor | null>(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 (
<RichMarkdownEditorSurface
editor={editor}

View File

@ -18,6 +18,7 @@ vi.mock('@/store', () => ({
}))
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

View File

@ -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<void> } | null
getContainerDomNode: () => HTMLElement

View File

@ -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)
})
})

View File

@ -0,0 +1,38 @@
import { keybindingMatchesAction, type KeybindingOverrides } from '../../../../shared/keybindings'
export function isMarkdownPreviewAddReviewNoteShortcut(
event: Pick<KeyboardEvent, 'key' | 'code' | 'metaKey' | 'ctrlKey' | 'altKey' | 'shiftKey'>,
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)
)
}

View File

@ -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
}

View File

@ -73,6 +73,7 @@ function createConfigParams(overrides: Partial<EditorConfigParams> = {}): Editor
markdownSourceLineOffsetRef: ref(0),
flushPendingSerialization: vi.fn(),
openSearchRef: ref(vi.fn()),
openAnnotationPopoverRef: ref(vi.fn()),
syncAnnotationTarget: vi.fn(),
clearAnnotationTarget: vi.fn(),
scrollRichMarkdownReviewNoteCardIntoView: vi.fn(),

View File

@ -77,6 +77,7 @@ export type EditorConfigParams = {
markdownSourceLineOffsetRef: MutableRefObject<number>
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,

View File

@ -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())

View File

@ -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<boolean>
flushPendingSerialization: () => void
openSearchRef: MutableRefObject<() => void>
openAnnotationPopoverRef: MutableRefObject<(requireLiveSelection?: boolean) => boolean>
setIsEditingLink: (editing: boolean) => void
setLinkBubble: (bubble: LinkBubbleState | null) => void
setSelectedCommandIndex: Dispatch<SetStateAction<number>>
@ -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).

View File

@ -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()
}

View File

@ -89,6 +89,7 @@ function createContext(editor: Editor): KeyHandlerContext {
subscribe: () => () => {},
update: () => {}
},
openAnnotationPopoverRef: { current: vi.fn() },
setIsEditingLink: vi.fn(),
setLinkBubble: vi.fn(),
setSelectedCommandIndex: vi.fn(),

View File

@ -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<HTMLElement | null>
): {
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 }
}

View File

@ -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) {

View File

@ -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'])

View File

@ -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',

View File

@ -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<HTMLElement>('[data-annotation-block-key]')
if (!block) {
throw new Error('No annotation block found in preview')
}
const focusable = block.closest<HTMLElement>('[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)
}
})
})