Merge pull request #32135 from overleaf/mg-comment-empty-select

Allow adding comments on empty selection

GitOrigin-RevId: 14e742bb563fab99624be860691f1a9d2dabc00e

services/web/frontend/js/features/review-panel/components/review-tooltip-menu.tsx

@@ -18,6 +18,8 @@ import {
   buildAddNewCommentRangeEffect,
   reviewTooltipStateField,
 } from '@/features/source-editor/extensions/review-tooltip'
+import { selectHighlightedOrNearestToken } from '@/features/source-editor/utils/select-highlighted-or-nearest-token'
+import { EditorSelection } from '@codemirror/state'
 import { EditorView, getTooltip } from '@codemirror/view'
 import usePreviousValue from '@/shared/hooks/use-previous-value'
 import { useLayoutContext } from '@/shared/context/layout-context'
@@ -59,11 +61,20 @@ const ReviewTooltipMenu: FC = () => {
   }, [tooltipState, previousTooltipState])
 
   const addComment = useCallback(() => {
-    const { main } = view.state.selection
-    if (main.empty || !permissions.comment) {
+    if (!permissions.comment) {
       return
     }
 
+    let { main } = view.state.selection
+
+    if (main.empty) {
+      const tokenRange = selectHighlightedOrNearestToken(view.state)
+      if (!tokenRange) {
+        return
+      }
+      main = EditorSelection.range(tokenRange.from, tokenRange.to)
+    }
+
     openReviewPanel()
     setView('cur_file')
 
@@ -74,7 +85,10 @@ const ReviewTooltipMenu: FC = () => {
         ]
       : [buildAddNewCommentRangeEffect(main)]
 
-    view.dispatch({ effects })
+    view.dispatch({
+      selection: { anchor: main.anchor, head: main.head },
+      effects,
+    })
     setShow(false)
   }, [view, permissions.comment, openReviewPanel, setView])
 

services/web/frontend/js/features/source-editor/components/toolbar/toolbar-items.tsx

@@ -16,6 +16,7 @@ import { isMac } from '@/shared/utils/os'
 import { useProjectContext } from '@/shared/context/project-context'
 import { useEditorPropertiesContext } from '@/features/ide-react/context/editor-properties-context'
 import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
+import { isCursorOnEmptyLine } from '@/features/source-editor/utils/is-cursor-on-empty-line'
 
 const addCommentFromToolbar = () => commands.addComment('toolbar')
 
@@ -136,7 +137,7 @@ export const ToolbarItems: FC<{
                 <ToolbarButton
                   id="toolbar-add-comment"
                   label={t('add_comment')}
-                  disabled={state.selection.main.empty}
+                  disabled={isCursorOnEmptyLine(state)}
                   command={addCommentFromToolbar}
                   icon="add_comment"
                 />

services/web/frontend/js/features/source-editor/hooks/use-context-menu-items.tsx

@@ -31,6 +31,7 @@ import {
   sendContextMenuEvent,
   ContextMenuItemSegmentation,
 } from '../utils/context-menu-analytics'
+import { isCursorOnEmptyLine } from '../utils/is-cursor-on-empty-line'
 
 export const useContextMenuItems = () => {
   const view = useCodeMirrorViewContext()
@@ -248,7 +249,7 @@ export const useContextMenuItems = () => {
       {
         label: t('comment'),
         handler: handleComment,
-        disabled: !hasSelection,
+        disabled: isCursorOnEmptyLine(state),
         show: permissions.comment,
         shortcut: getShortcut('insert-comment'),
       },

services/web/frontend/js/features/source-editor/hooks/use-toolbar-menu-editor-commands.tsx

@@ -15,6 +15,7 @@ import { useCallback } from 'react'
 import { useTranslation } from 'react-i18next'
 import { usePermissionsContext } from '@/features/ide-react/context/permissions-context'
 import { language } from '@codemirror/language'
+import { isCursorOnEmptyLine } from '@/features/source-editor/utils/is-cursor-on-empty-line'
 
 export const useToolbarMenuBarEditorCommands = () => {
   const view = useCodeMirrorViewContext()
@@ -25,6 +26,7 @@ export const useToolbarMenuBarEditorCommands = () => {
   const { trackedWrite, comment } = usePermissionsContext()
   const languageName = state.facet(language)?.name
   const isTeXFile = languageName === 'latex'
+  const canComment = !isCursorOnEmptyLine(state)
 
   const openFigureModal = useCallback((source: FigureModalSource) => {
     window.dispatchEvent(
@@ -174,7 +176,7 @@ export const useToolbarMenuBarEditorCommands = () => {
         handler: () => {
           commands.addComment('toolbar')
         },
-        disabled: !comment || state.selection.main.empty,
+        disabled: !comment || !canComment,
       },
       /************************************
        *         Format menu
@@ -283,7 +285,7 @@ export const useToolbarMenuBarEditorCommands = () => {
     openFigureModal,
     trackedWrite,
     isTeXFile,
-    state.selection.main.empty,
+    canComment,
     comment,
   ])
 

services/web/frontend/js/features/source-editor/utils/is-cursor-on-empty-line.ts

@@ -0,0 +1,18 @@
+import { EditorState } from '@codemirror/state'
+
+/**
+ * Returns true when the cursor is on an empty line with no active selection.
+ *
+ * Used to disable the "Add comment" action — there is nothing to anchor a
+ * comment to on a blank line.
+ */
+export function isCursorOnEmptyLine(state: EditorState): boolean {
+  const { main } = state.selection
+
+  if (!main.empty) {
+    return false
+  }
+
+  const line = state.doc.lineAt(main.head)
+  return line.text.trim().length === 0
+}

services/web/frontend/js/features/source-editor/utils/select-highlighted-or-nearest-token.ts

@@ -0,0 +1,68 @@
+import { EditorState } from '@codemirror/state'
+import { isCursorOnEmptyLine } from './is-cursor-on-empty-line'
+
+/**
+ * Returns the current highlighted range, or the nearest non-whitespace token
+ * to the cursor when there is no active selection.
+ *
+ * "Token" here means any contiguous run of non-whitespace characters (`/\S+/`),
+ * not necessarily an alphanumeric word. When two tokens are equidistant the
+ * earlier one (closer to document start) wins.
+ *
+ * Only searches the current line.
+ *
+ * Only called on user-initiated actions (e.g. "Add comment"), not on every render.
+ * Returns null when the cursor is on an empty/whitespace-only line.
+ */
+export function selectHighlightedOrNearestToken(
+  state: EditorState
+): { from: number; to: number } | null {
+  if (isCursorOnEmptyLine(state)) {
+    return null
+  }
+
+  const { main } = state.selection
+
+  if (!main.empty) {
+    return { from: main.from, to: main.to }
+  }
+
+  const pos = main.head
+  const line = state.doc.lineAt(pos)
+
+  let bestFrom = 0
+  let bestTo = 0
+  let bestDist = Infinity
+
+  for (const match of line.text.matchAll(/\S+/g)) {
+    const from = line.from + match.index!
+    const to = from + match[0].length
+
+    let dist = 0
+    if (pos < from) {
+      dist = from - pos
+    } else if (pos > to) {
+      dist = pos - to
+    }
+
+    if (dist === 0) {
+      return { from, to }
+    }
+
+    if (dist < bestDist) {
+      bestFrom = from
+      bestTo = to
+      bestDist = dist
+    }
+
+    // Past the cursor — no closer match possible on this line
+    if (from > pos) {
+      break
+    }
+  }
+
+  if (bestDist < Infinity) {
+    return { from: bestFrom, to: bestTo }
+  }
+  return null
+}

services/web/test/frontend/features/source-editor/components/codemirror-editor-context-menu.spec.tsx

@@ -239,7 +239,7 @@ describe('editor context menu', { scrollBehavior: false }, function () {
   })
 
   describe('when nothing is selected', function () {
-    it('should enable Cut, Copy, Paste, Suggest edits and disable Delete, Comment', function () {
+    it('should enable Cut, Copy, Paste, Suggest edits, Comment and disable Delete', function () {
       const scope = mockScope()
 
       cy.mount(
@@ -269,11 +269,7 @@ describe('editor context menu', { scrollBehavior: false }, function () {
           'aria-disabled',
           'true'
         )
-        cy.findByRole('menuitem', { name: /comment/i }).should(
-          'have.attr',
-          'aria-disabled',
-          'true'
-        )
+        cy.findByRole('menuitem', { name: /comment/i }).should('be.enabled')
         cy.findByRole('menuitem', { name: /suggest edits/i }).should(
           'be.enabled'
         )

services/web/test/frontend/features/source-editor/utils/is-cursor-on-empty-line.test.ts

@@ -0,0 +1,44 @@
+import { expect } from 'chai'
+import { EditorState, EditorSelection } from '@codemirror/state'
+import { isCursorOnEmptyLine } from '../../../../../frontend/js/features/source-editor/utils/is-cursor-on-empty-line'
+
+function cursorState(doc: string, cursorPos: number) {
+  return EditorState.create({
+    doc,
+    selection: EditorSelection.cursor(cursorPos),
+  })
+}
+
+function selectionState(doc: string, from: number, to: number) {
+  return EditorState.create({
+    doc,
+    selection: EditorSelection.range(from, to),
+  })
+}
+
+describe('isCursorOnEmptyLine', function () {
+  it('returns true when cursor is on an empty line', function () {
+    const state = cursorState('hello\n\nworld', 6)
+    expect(isCursorOnEmptyLine(state)).to.be.true
+  })
+
+  it('returns true when cursor is on a whitespace-only line', function () {
+    const state = cursorState('hello\n   \nworld', 8)
+    expect(isCursorOnEmptyLine(state)).to.be.true
+  })
+
+  it('returns false when cursor is on a line with content', function () {
+    const state = cursorState('hello world', 3)
+    expect(isCursorOnEmptyLine(state)).to.be.false
+  })
+
+  it('returns false when there is an active selection', function () {
+    const state = selectionState('hello\n\nworld', 0, 5)
+    expect(isCursorOnEmptyLine(state)).to.be.false
+  })
+
+  it('returns false when selection spans an empty line', function () {
+    const state = selectionState('hello\n\nworld', 4, 8)
+    expect(isCursorOnEmptyLine(state)).to.be.false
+  })
+})

services/web/test/frontend/features/source-editor/utils/select-highlighted-or-nearest-token.test.ts

@@ -0,0 +1,81 @@
+import { expect } from 'chai'
+import { EditorState, EditorSelection } from '@codemirror/state'
+import { selectHighlightedOrNearestToken } from '../../../../../frontend/js/features/source-editor/utils/select-highlighted-or-nearest-token'
+
+function cursorState(doc: string, cursorPos: number) {
+  return EditorState.create({
+    doc,
+    selection: EditorSelection.cursor(cursorPos),
+  })
+}
+
+function selectionState(doc: string, from: number, to: number) {
+  return EditorState.create({
+    doc,
+    selection: EditorSelection.range(from, to),
+  })
+}
+
+function sliceRange(state: EditorState, range: { from: number; to: number }) {
+  return state.doc.sliceString(range.from, range.to)
+}
+
+describe('selectHighlightedOrNearestToken', function () {
+  it('returns current highlighted selection when it exists', function () {
+    const state = selectionState('hello world', 0, 5)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.not.be.null
+    expect(sliceRange(state, result!)).to.equal('hello')
+  })
+
+  it('selects the token containing the cursor', function () {
+    const state = cursorState('hello world', 8)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.not.be.null
+    expect(sliceRange(state, result!)).to.equal('world')
+  })
+
+  it('selects nearest token when cursor is in leading whitespace', function () {
+    const state = cursorState('   hello world', 0)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.not.be.null
+    expect(sliceRange(state, result!)).to.equal('hello')
+  })
+
+  it('selects token when cursor is at its boundary', function () {
+    // pos 5 is immediately after "hello" (from=0, to=5) — pos equals to, so dist is 0
+    const state = cursorState('hello world', 5)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.not.be.null
+    expect(sliceRange(state, result!)).to.equal('hello')
+  })
+
+  it('selects nearer token when cursor is between two tokens', function () {
+    // "hello   world" — pos 6, "hello" ends at 5 (dist 1), "world" starts at 8 (dist 2)
+    const state = cursorState('hello   world', 6)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.not.be.null
+    expect(sliceRange(state, result!)).to.equal('hello')
+  })
+
+  it('selects earlier token when equidistant between two tokens', function () {
+    // "hello  world" — pos 6 is equidistant from "hello" (ends at 5, dist 1) and "world" (starts at 7, dist 1)
+    const state = cursorState('hello  world', 6)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.not.be.null
+    expect(sliceRange(state, result!)).to.equal('hello')
+  })
+
+  it('returns null when line has no tokens', function () {
+    const state = cursorState('   \n\t  ', 2)
+    const result = selectHighlightedOrNearestToken(state)
+
+    expect(result).to.be.null
+  })
+})