Merge pull request #35514 from overleaf/worktree-mg-comment-fix

Split add-comment to own extension and always register command GitOrigin-RevId: 2e25643b7fe4ff30693479975945d94cc586bd61
2026-11-12 13:13:45 +01:00 · 2026-09-17 11:27:36 +00:00
parent c65a630438
commit bbbbc49bc0
9 changed files with 116 additions and 90 deletions
@@ -2,7 +2,7 @@ import { useCallback } from 'react'
 import { EditorSelection } from '@codemirror/state'
 import { EditorView } from '@codemirror/view'
 import { useCodeMirrorViewContext } from '@/features/source-editor/components/codemirror-context'
-import { buildAddNewCommentRangeEffect } from '@/features/source-editor/extensions/review-tooltip'
+import { buildAddNewCommentRangeEffect } from '@/features/source-editor/extensions/add-comment'
 import { selectHighlightedOrNearestToken } from '@/features/source-editor/utils/select-highlighted-or-nearest-token'
 import { isCursorNearViewportEdge } from '@/features/source-editor/utils/is-cursor-near-edge'
 import useEventListener from '@/shared/hooks/use-event-listener'
@@ -15,7 +15,7 @@ import {
  useCodeMirrorStateContext,
  useCodeMirrorViewContext,
 } from '@/features/source-editor/components/codemirror-context'
-import { reviewTooltipStateField } from '@/features/source-editor/extensions/review-tooltip'
+import { reviewTooltipField } from '@/features/source-editor/extensions/review-tooltip'
 import usePreviousValue from '@/shared/hooks/use-previous-value'
 import { useLayoutContext } from '@/shared/context/layout-context'
 import { useEditorPropertiesContext } from '@/features/ide-react/context/editor-properties-context'
@@ -42,7 +42,7 @@ const EditorFloatingMenu: FC = () => {
  const state = useCodeMirrorStateContext()
  const view = useCodeMirrorViewContext()
  const [show, setShow] = useState(true)
-  const tooltipState = state.field(reviewTooltipStateField, false)?.tooltip
+  const tooltipState = state.field(reviewTooltipField, false)
  const previousTooltipState = usePreviousValue(tooltipState)

  useEffect(() => {
@@ -6,7 +6,7 @@ import {
 import { EditorSelection } from '@codemirror/state'
 import { useTranslation } from 'react-i18next'
 import { useThreadsActionsContext } from '../context/threads-context'
-import { removeNewCommentRangeEffect } from '@/features/source-editor/extensions/review-tooltip'
+import { removeNewCommentRangeEffect } from '@/features/source-editor/extensions/add-comment'
 import AutoExpandingTextArea from '@/shared/components/auto-expanding-text-area'
 import { ReviewPanelEntry } from './review-panel-entry'
 import { ThreadId } from '../../../../../types/review-panel/review-panel'
@@ -28,7 +28,7 @@ import { canAggregate } from '../utils/can-aggregate'
 import ReviewPanelEmptyState from './review-panel-empty-state'
 import useEventListener from '@/shared/hooks/use-event-listener'
 import { hasActiveRange } from '@/features/review-panel/utils/has-active-range'
-import { reviewTooltipStateField } from '@/features/source-editor/extensions/review-tooltip'
+import { addCommentRangesField } from '@/features/source-editor/extensions/add-comment'
 import ReviewPanelMoreCommentsButton from './review-panel-more-comments-button'
 import useMoreCommments from '../hooks/use-more-comments'
 import { Decoration } from '@codemirror/view'
@@ -116,10 +116,7 @@ const ReviewPanelCurrentFile: FC = () => {

  const positionsRef = useRef<Map<string, number>>(new Map())

-  const addCommentRanges = state.field(
-    reviewTooltipStateField,
-    false
-  )?.addCommentRanges
+  const addCommentRanges = state.field(addCommentRangesField, false)

  const setUpdatedPositions = useMemo(
    () =>
@@ -14,10 +14,8 @@ import {
  useCodeMirrorStateContext,
  useCodeMirrorViewContext,
 } from '@/features/source-editor/components/codemirror-context'
-import {
-  buildAddNewCommentRangeEffect,
-  reviewTooltipStateField,
-} from '@/features/source-editor/extensions/review-tooltip'
+import { reviewTooltipField } from '@/features/source-editor/extensions/review-tooltip'
+import { buildAddNewCommentRangeEffect } from '@/features/source-editor/extensions/add-comment'
 import { selectHighlightedOrNearestToken } from '@/features/source-editor/utils/select-highlighted-or-nearest-token'
 import { EditorSelection } from '@codemirror/state'
 import { EditorView, getTooltip } from '@codemirror/view'
@@ -52,7 +50,7 @@ const ReviewTooltipMenu: FC = () => {
  const [show, setShow] = useState(true)
  const { setView } = useReviewPanelViewActionsContext()
  const { openReviewPanel } = useReviewPanelLayout()
-  const tooltipState = state.field(reviewTooltipStateField, false)?.tooltip
+  const tooltipState = state.field(reviewTooltipField, false)
  const previousTooltipState = usePreviousValue(tooltipState)

  useEffect(() => {
@@ -28,7 +28,6 @@ import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { useEditorOpenDocContext } from '@/features/ide-react/context/editor-open-doc-context'
 import { useEditorPropertiesContext } from '@/features/ide-react/context/editor-properties-context'
 import UpgradeTrackChangesModal from '@/features/review-panel/components/upgrade-track-changes-modal'
-import { useUserSettingsContext } from '@/shared/context/user-settings-context'

 // TODO: remove this when definitely no longer used
 export * from './codemirror-context'
@@ -96,9 +95,6 @@ function CodeMirrorEditorComponents({
 }: CodeMirrorEditorComponentsProps) {
  useToolbarMenuBarEditorCommands()
  const { features } = useProjectContext()
-  const {
-    userSettings: { floatingMenu },
-  } = useUserSettingsContext()
  const writefullToolbarMigrationEnabled = useFeatureFlag(
    'writefull-toolbar-migration'
  )
@@ -113,8 +109,7 @@ function CodeMirrorEditorComponents({

      <MathPreviewTooltip />
      <EditorContextMenu />
-      {floatingMenu &&
-        features.trackChangesVisible &&
+      {features.trackChangesVisible &&
        (writefullToolbarMigrationEnabled ? (
          <>
            <AddCommentCommand />
@@ -0,0 +1,55 @@
+import { Decoration, DecorationSet, EditorView } from '@codemirror/view'
+import {
+  StateField,
+  StateEffect,
+  Range,
+  SelectionRange,
+} from '@codemirror/state'
+import { v4 as uuid } from 'uuid'
+
+export const addNewCommentRangeEffect = StateEffect.define<Range<Decoration>>()
+
+export const removeNewCommentRangeEffect = StateEffect.define<string>()
+
+export const buildAddNewCommentRangeEffect = (range: SelectionRange) => {
+  return addNewCommentRangeEffect.of(
+    Decoration.mark({
+      tagName: 'span',
+      class: `ol-cm-change ol-cm-change-c`,
+      opType: 'c',
+      id: uuid(),
+    }).range(range.from, range.to)
+  )
+}
+
+export const addCommentRangesField = StateField.define<DecorationSet>({
+  create() {
+    return Decoration.none
+  },
+
+  update(addCommentRanges, tr) {
+    addCommentRanges = addCommentRanges.map(tr.changes)
+
+    for (const effect of tr.effects) {
+      if (effect.is(removeNewCommentRangeEffect)) {
+        const threadId = effect.value
+        addCommentRanges = addCommentRanges.update({
+          filter: (_from, _to, value) => {
+            return value.spec.id !== threadId
+          },
+        })
+      }
+
+      if (effect.is(addNewCommentRangeEffect)) {
+        const rangeToAdd = effect.value
+        addCommentRanges = addCommentRanges.update({
+          add: [rangeToAdd],
+        })
+      }
+    }
+
+    return addCommentRanges
+  },
+
+  provide: field => EditorView.decorations.from(field),
+})
@@ -1,28 +1,15 @@
-import {
-  Decoration,
-  DecorationSet,
-  EditorView,
-  showTooltip,
-  Tooltip,
-  TooltipView,
-} from '@codemirror/view'
+import { EditorView, showTooltip, Tooltip, TooltipView } from '@codemirror/view'
 import {
  Extension,
  StateField,
  StateEffect,
-  Range,
-  SelectionRange,
  EditorState,
  Transaction,
  Compartment,
  TransactionSpec,
 } from '@codemirror/state'
-import { v4 as uuid } from 'uuid'
 import { isContextMenuMouseEvent } from '../utils/context-menu-mouse-event'
-
-export const addNewCommentRangeEffect = StateEffect.define<Range<Decoration>>()
-
-export const removeNewCommentRangeEffect = StateEffect.define<string>()
+import { addCommentRangesField } from './add-comment'

 const mouseDownEffect = StateEffect.define()
 const mouseUpEffect = StateEffect.define()
@@ -43,26 +30,18 @@ const mouseDownStateField = StateField.define<boolean>({
  },
 })

-export const buildAddNewCommentRangeEffect = (range: SelectionRange) => {
-  return addNewCommentRangeEffect.of(
-    Decoration.mark({
-      tagName: 'span',
-      class: `ol-cm-change ol-cm-change-c`,
-      opType: 'c',
-      id: uuid(),
-    }).range(range.from, range.to)
-  )
-}
-
 const reviewTooltipCompartment = new Compartment()

 export const reviewTooltip = (
  enabled: boolean,
  editorContextMenuEnabled: boolean = false
 ): Extension => {
-  return reviewTooltipCompartment.of(
-    enabled ? reviewTooltipEnabled(editorContextMenuEnabled) : []
-  )
+  return [
+    addCommentRangesField,
+    reviewTooltipCompartment.of(
+      enabled ? reviewTooltipMenu(editorContextMenuEnabled) : []
+    ),
+  ]
 }

 export const setReviewTooltip = (
@@ -70,13 +49,11 @@ export const setReviewTooltip = (
  editorContextMenuEnabled: boolean
 ): TransactionSpec => ({
  effects: reviewTooltipCompartment.reconfigure(
-    enabled ? reviewTooltipEnabled(editorContextMenuEnabled) : []
+    enabled ? reviewTooltipMenu(editorContextMenuEnabled) : []
  ),
 })

-export const reviewTooltipEnabled = (
-  editorContextMenuEnabled = false
-): Extension => {
+const reviewTooltipMenu = (editorContextMenuEnabled: boolean): Extension => {
  let mouseUpListener: null | (() => void) = null
  const disableMouseUpListener = () => {
    if (mouseUpListener) {
@@ -86,7 +63,7 @@ export const reviewTooltipEnabled = (

  return [
    reviewTooltipTheme,
-    reviewTooltipStateField,
+    reviewTooltipField,
    mouseDownStateField,
    EditorView.domEventHandlers({
      mousedown: (event, view) => {
@@ -110,39 +87,14 @@ export const reviewTooltipEnabled = (
  ]
 }

-export const reviewTooltipStateField = StateField.define<{
-  tooltip: Tooltip | null
-  addCommentRanges: DecorationSet
-}>({
+export const reviewTooltipField = StateField.define<Tooltip | null>({
  create() {
-    return { tooltip: null, addCommentRanges: Decoration.none }
+    return null
  },

-  update(field, tr) {
-    let { tooltip, addCommentRanges } = field
-
-    addCommentRanges = addCommentRanges.map(tr.changes)
-
-    for (const effect of tr.effects) {
-      if (effect.is(removeNewCommentRangeEffect)) {
-        const threadId = effect.value
-        addCommentRanges = addCommentRanges.update({
-          filter: (_from, _to, value) => {
-            return value.spec.id !== threadId
-          },
-        })
-      }
-
-      if (effect.is(addNewCommentRangeEffect)) {
-        const rangeToAdd = effect.value
-        addCommentRanges = addCommentRanges.update({
-          add: [rangeToAdd],
-        })
-      }
-    }
-
+  update(tooltip, tr) {
    if (tr.state.selection.main.empty) {
-      return { tooltip: null, addCommentRanges }
+      return null
    }

    if (
@@ -152,24 +104,21 @@ export const reviewTooltipStateField = StateField.define<{
    ) {
      if (tr.selection) {
        // selection was changed, remove the tooltip
-        return { tooltip: null, addCommentRanges }
+        return null
      }
      // for any other update, we keep the tooltip because it could be created in previous transaction
      // and we are still waiting for "mouse up" event to show it
-      return { tooltip, addCommentRanges }
+      return tooltip
    }

    const isMouseDown = tr.state.field(mouseDownStateField)
    // if "isMouseDown" is true, tooltip will be created but still hidden
    // the reason why we cant just create the tooltip on mouse up is because transaction.userEvent is empty at that point

-    return { tooltip: buildTooltip(tr.state, isMouseDown), addCommentRanges }
+    return buildTooltip(tr.state, isMouseDown)
  },

-  provide: field => [
-    EditorView.decorations.from(field, field => field.addCommentRanges),
-    showTooltip.compute([field], state => state.field(field).tooltip),
-  ],
+  provide: field => showTooltip.from(field),
 })

 function buildTooltip(state: EditorState, hidden: boolean): Tooltip | null {
@@ -217,6 +217,38 @@ describe('<EditorFloatingMenu />', function () {
    })
  })

+  describe('adding a comment with the floating menu disabled', function () {
+    describe('when migrationEnabled is true', function () {
+      it('still creates a pending comment via the command', function () {
+        mountEditor({ migrationEnabled: true, floatingMenu: false })
+
+        // The quick-actions popup stays hidden...
+        cy.get('.editor-floating-menu').should('not.exist')
+
+        // The toolbar / menu / shortcut path (all dispatch this event) works.
+        cy.window().then(win => {
+          win.dispatchEvent(new Event('add-new-review-comment'))
+        })
+
+        cy.get('.ol-cm-change-c').should('exist')
+      })
+    })
+
+    describe('when migrationEnabled is false', function () {
+      it('still creates a pending comment via the command (legacy tooltip)', function () {
+        mountEditor({ migrationEnabled: false, floatingMenu: false })
+
+        cy.get('.review-tooltip-menu').should('not.exist')
+
+        cy.window().then(win => {
+          win.dispatchEvent(new Event('add-new-review-comment'))
+        })
+
+        cy.get('.ol-cm-change-c').should('exist')
+      })
+    })
+  })
+
  describe('bulk tracked-change actions', function () {
    beforeEach(function () {
      mountEditorWithChanges()