From 6901a2a8de73cc38309bf9ba46f614be246b526c Mon Sep 17 00:00:00 2001 From: Malik Glossop Date: Fri, 20 Mar 2026 09:06:09 +0100 Subject: [PATCH] Merge pull request #32135 from overleaf/mg-comment-empty-select Allow adding comments on empty selection GitOrigin-RevId: 14e742bb563fab99624be860691f1a9d2dabc00e --- .../components/review-tooltip-menu.tsx | 20 ++++- .../components/toolbar/toolbar-items.tsx | 3 +- .../hooks/use-context-menu-items.tsx | 3 +- .../use-toolbar-menu-editor-commands.tsx | 6 +- .../utils/is-cursor-on-empty-line.ts | 18 +++++ .../select-highlighted-or-nearest-token.ts | 68 ++++++++++++++++ .../codemirror-editor-context-menu.spec.tsx | 8 +- .../utils/is-cursor-on-empty-line.test.ts | 44 ++++++++++ ...elect-highlighted-or-nearest-token.test.ts | 81 +++++++++++++++++++ 9 files changed, 238 insertions(+), 13 deletions(-) create mode 100644 services/web/frontend/js/features/source-editor/utils/is-cursor-on-empty-line.ts create mode 100644 services/web/frontend/js/features/source-editor/utils/select-highlighted-or-nearest-token.ts create mode 100644 services/web/test/frontend/features/source-editor/utils/is-cursor-on-empty-line.test.ts create mode 100644 services/web/test/frontend/features/source-editor/utils/select-highlighted-or-nearest-token.test.ts diff --git a/services/web/frontend/js/features/review-panel/components/review-tooltip-menu.tsx b/services/web/frontend/js/features/review-panel/components/review-tooltip-menu.tsx index aa9ab376a5..0d792014cc 100644 --- a/services/web/frontend/js/features/review-panel/components/review-tooltip-menu.tsx +++ b/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]) diff --git a/services/web/frontend/js/features/source-editor/components/toolbar/toolbar-items.tsx b/services/web/frontend/js/features/source-editor/components/toolbar/toolbar-items.tsx index b1e11c2542..fa65e9bded 100644 --- a/services/web/frontend/js/features/source-editor/components/toolbar/toolbar-items.tsx +++ b/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<{ diff --git a/services/web/frontend/js/features/source-editor/hooks/use-context-menu-items.tsx b/services/web/frontend/js/features/source-editor/hooks/use-context-menu-items.tsx index 8841ca710b..13a7c8416c 100644 --- a/services/web/frontend/js/features/source-editor/hooks/use-context-menu-items.tsx +++ b/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'), }, diff --git a/services/web/frontend/js/features/source-editor/hooks/use-toolbar-menu-editor-commands.tsx b/services/web/frontend/js/features/source-editor/hooks/use-toolbar-menu-editor-commands.tsx index 65d8f09190..5cf450890a 100644 --- a/services/web/frontend/js/features/source-editor/hooks/use-toolbar-menu-editor-commands.tsx +++ b/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, ]) diff --git a/services/web/frontend/js/features/source-editor/utils/is-cursor-on-empty-line.ts b/services/web/frontend/js/features/source-editor/utils/is-cursor-on-empty-line.ts new file mode 100644 index 0000000000..08b46b84cc --- /dev/null +++ b/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 +} diff --git a/services/web/frontend/js/features/source-editor/utils/select-highlighted-or-nearest-token.ts b/services/web/frontend/js/features/source-editor/utils/select-highlighted-or-nearest-token.ts new file mode 100644 index 0000000000..56b77ba232 --- /dev/null +++ b/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 +} diff --git a/services/web/test/frontend/features/source-editor/components/codemirror-editor-context-menu.spec.tsx b/services/web/test/frontend/features/source-editor/components/codemirror-editor-context-menu.spec.tsx index 18b1d43d91..5ee615b4b3 100644 --- a/services/web/test/frontend/features/source-editor/components/codemirror-editor-context-menu.spec.tsx +++ b/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' ) diff --git a/services/web/test/frontend/features/source-editor/utils/is-cursor-on-empty-line.test.ts b/services/web/test/frontend/features/source-editor/utils/is-cursor-on-empty-line.test.ts new file mode 100644 index 0000000000..30153a2669 --- /dev/null +++ b/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 + }) +}) diff --git a/services/web/test/frontend/features/source-editor/utils/select-highlighted-or-nearest-token.test.ts b/services/web/test/frontend/features/source-editor/utils/select-highlighted-or-nearest-token.test.ts new file mode 100644 index 0000000000..70081512f1 --- /dev/null +++ b/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 + }) +})