Address first round of feedback

lib/prompt-editor/src/v2/MentionsMenu.module.css

@@ -1,11 +1,5 @@
 .popover-dimensions {
     width: clamp(300px, 65vw, 440px);
-
-    /*
-    --max-items: 12;
-    --mention-item-height: 30px;
-    height: min(calc(var(--max-items)*var(--mention-item-height) + 2px), 90vh);
-    */
 }
 
 .menu {

lib/prompt-editor/src/v2/MentionsMenu.tsx

@@ -85,14 +85,12 @@ export const MentionsMenu = <T,>({
         })
     }, [menuPosition, refs])
 
+    // biome-ignore lint/correctness/useExhaustiveDependencies(selectedIndex): we want to scroll the selected item into view but only when selectedIndex changes
     useEffect(() => {
-        // WORKAROUND: We want to scroll the selected item into view, but only when selectedIndex changes.
-        // This statement was added to prevent biome from flagging that selectedIndex is not used.
-        void selectedIndex
         container.current?.querySelector('[aria-selected="true"]')?.scrollIntoView({ block: 'nearest' })
     }, [selectedIndex])
 
-    // This prevents the input from lossing focus when clicking on the menu.
+    // This prevents the input from loosing focus when clicking on the menu.
     const handleMouseDown: MouseEventHandler = useCallback(event => {
         event.preventDefault()
     }, [])
@@ -103,8 +101,7 @@ export const MentionsMenu = <T,>({
             const listNode = target?.closest('[role=option]') as HTMLElement | null
             if (listNode?.parentNode) {
                 const options = listNode.parentNode.querySelectorAll('[role="option"]')
-                // @ts-expect-error - TS doesn't like this but it's OK
-                const index = [].indexOf.call(options, listNode)
+                const index = Array.prototype.indexOf.call(options, listNode)
                 if (index !== -1) {
                     onSelect?.(index)
                 }
@@ -116,7 +113,7 @@ export const MentionsMenu = <T,>({
     const header = getHeader()
 
     return (
-        // biome-ignore lint/a11y/useKeyWithClickEvents: the menu works like a combobox and isn't directly contrallable via keyboard. The keyboard events are handled by the editor.
+        // biome-ignore lint/a11y/useKeyWithClickEvents: the menu works like a combobox and isn't directly contrallable via keyboard. The keyboard events are handled by the editor and the component is updated accordingly.
         <div
             ref={node => {
                 container.current = node

lib/prompt-editor/src/v2/lexical-interop.ts

@@ -63,18 +63,28 @@ function fromSerializedLexicalNode(node: SerializedLexicalNode): unknown {
 
 /**
  * Convert a {@link SerializedPromptEditorState} to a ProseMirror document.
+ *
+ * This makes a best-effort attempt to convert a Lexical document to a ProseMirror document.
+ * A Lexical document contains more information than a ProseMirror document, some data is ignored/lost.
  */
 export function fromSerializedPromptEditorState(state: SerializedPromptEditorState): unknown {
     return fromSerializedLexicalNode(state.lexicalEditorState.root)
 }
 
 /**
  * Convert a ProseMirror document to a {@link SerializedPromptEditorValue}.
+ *
+ * This makes a best-effort attempt to serialize a ProseMirror document to a Lexical document.
+ * A Lexical document contains more information than a ProseMirror document, some data is set to seemingly reasonable defaults.
  */
 export function toSerializedPromptEditorValue(doc: Node): SerializedPromptEditorValue {
     const contextItems: SerializedContextItem[] = []
     const direction =
-        typeof window !== 'undefined' ? window.getComputedStyle(window.document.body).direction : null
+        (typeof window !== 'undefined'
+            ? window.getComputedStyle(window.document.body).direction
+            : null) === 'rtl'
+            ? 'rtl'
+            : 'ltr'
 
     doc.descendants(node => {
         if (node.type.name === 'mention') {
@@ -98,7 +108,7 @@ export function toSerializedPromptEditorValue(doc: Node): SerializedPromptEditor
                 return {
                     type: 'paragraph',
                     children,
-                    direction: direction === 'rtl' ? 'rtl' : 'ltr',
+                    direction,
                     format: '',
                     indent: 0,
                     version: 1,
@@ -146,7 +156,7 @@ export function toSerializedPromptEditorValue(doc: Node): SerializedPromptEditor
             format: '',
             indent: 0,
             version: 1,
-            direction: direction === 'rtl' ? 'rtl' : 'ltr',
+            direction,
         }
     }
 

lib/prompt-editor/src/v2/plugins/atMention.module.css

@@ -12,7 +12,7 @@
     border: 1px solid rgba(0, 0, 0, 0.02);
 }
 
-/* TODO dark */
+/* TODO: Properly handle dark and light mode after learning how theming works in Cody */
 .atMention::after {
     background-color: rgba(255, 255, 255, 0.035);
     border: 1px solid rgba(255, 255, 255, 0.035);

lib/prompt-editor/src/v2/plugins/atMention.test.ts

@@ -1,7 +1,13 @@
 import { EditorState, TextSelection } from 'prosemirror-state'
 import { beforeEach, expect, test } from 'vitest'
 import { schema } from '../promptInput'
-import { createAtMentionPlugin, enableAtMention, getAtMentionValue, hasAtMention } from './atMention'
+import {
+    createAtMentionPlugin,
+    enableAtMention,
+    getAtMentionPosition,
+    getAtMentionValue,
+    hasAtMention,
+} from './atMention'
 
 let state: EditorState
 
@@ -20,6 +26,7 @@ beforeEach(() => {
 test('create at mention', () => {
     const newState = state.apply(enableAtMention(state.tr.insertText('abc @')))
     expect(hasAtMention(newState)).toBe(true)
+    expect(getAtMentionPosition(newState)).toBe(5)
 })
 
 test('update at mention value', () => {
@@ -32,27 +39,59 @@ test('update at mention value', () => {
     expect(getAtMentionValue(newState)).toBe('@foobar')
 })
 
+test('create at mention in the middle of a text', () => {
+    let newState = state.apply(state.tr.insertText('foo bar'))
+    newState = newState.apply(
+        enableAtMention(newState.tr.setSelection(TextSelection.create(newState.doc, 5)).insertText('@'))
+    )
+
+    expect(newState.doc.textContent).toBe('foo @bar')
+    expect(hasAtMention(newState)).toBe(true)
+    expect(getAtMentionPosition(newState)).toBe(5)
+    expect(getAtMentionValue(newState)).toBe('@')
+
+    newState = newState.apply(newState.tr.insertText('baz'))
+    expect(newState.doc.textContent).toBe('foo @bazbar')
+    expect(getAtMentionValue(newState)).toBe('@baz')
+})
+
 test('disable at mention when selection moves outside', () => {
     let newState = state.apply(enableAtMention(state.tr.insertText('abc @')))
 
     newState = newState.apply(newState.tr.insertText('foo'))
     expect(hasAtMention(newState)).toBe(true)
 
     // NOTE: 5 is the position before the '@' character
-    newState = newState.apply(newState.tr.setSelection(TextSelection.create(newState.doc, 5)))
+    const atMentionPosition = 5
+
+    expect(getAtMentionPosition(newState)).toBe(atMentionPosition)
+
+    newState = newState.apply(
+        newState.tr.setSelection(TextSelection.create(newState.doc, atMentionPosition))
+    )
     expect(hasAtMention(newState), 'keeps at mention when selection moves to its start').toBe(true)
 
     newState = newState.apply(newState.tr.setSelection(TextSelection.atStart(newState.doc)))
     expect(hasAtMention(newState), 'removes at mention when selection moves outside').toBe(false)
 })
 
-test('disable at mention when two space are entered', () => {
+test('disable at mention when selection is not empty', () => {
     let newState = state.apply(enableAtMention(state.tr.insertText('abc @')))
 
     newState = newState.apply(newState.tr.insertText('foo'))
     expect(hasAtMention(newState)).toBe(true)
 
-    newState = newState.apply(newState.tr.insertText(' '))
+    newState = newState.apply(newState.tr.setSelection(TextSelection.create(newState.doc, 2, 8)))
+    expect(hasAtMention(newState), 'removes at mention when selection is not empty').toBe(false)
+})
+
+test('disable at mention when three space are entered', () => {
+    let newState = state.apply(enableAtMention(state.tr.insertText('abc @')))
+
+    newState = newState.apply(newState.tr.insertText('foo'))
+    expect(hasAtMention(newState)).toBe(true)
+
+    newState = newState.apply(newState.tr.insertText('  '))
     expect(hasAtMention(newState)).toBe(true)
 
     newState = newState.apply(newState.tr.insertText(' '))