Browse Source

Merge pull request #30596 from overleaf/mg-context-menu-hide-others

Hide other menus when right-click context menu opens

GitOrigin-RevId: c7a2126ec58ed69520f31ba20fa450a6f1524a3f
Malik Glossop 6 tháng trước cách đây
mục cha
commit
ae5e3543c5

+ 23 - 9
services/web/frontend/js/features/source-editor/extensions/context-menu.ts

@@ -13,6 +13,7 @@ import {
   EditorSelection,
   EditorSelection,
   Prec,
   Prec,
 } from '@codemirror/state'
 } from '@codemirror/state'
+import { closeAllContextMenusEffect } from '../utils/close-all-context-menus-effect'
 
 
 export const openContextMenuEffect = StateEffect.define<{
 export const openContextMenuEffect = StateEffect.define<{
   pos: number
   pos: number
@@ -33,8 +34,16 @@ export const contextMenuStateField = StateField.define<ContextMenuState>({
   },
   },
 
 
   update(field, tr) {
   update(field, tr) {
-    // Process state effects to open/close menu
+    let next = field
+
+    // Process effects in order but let open win if present in the same transaction
     for (const effect of tr.effects) {
     for (const effect of tr.effects) {
+      if (
+        effect.is(closeContextMenuEffect) ||
+        effect.is(closeAllContextMenusEffect)
+      ) {
+        next = { tooltip: null, mousePosition: null }
+      }
       if (effect.is(openContextMenuEffect)) {
       if (effect.is(openContextMenuEffect)) {
         const { pos, x, y } = effect.value
         const { pos, x, y } = effect.value
         return {
         return {
@@ -42,9 +51,11 @@ export const contextMenuStateField = StateField.define<ContextMenuState>({
           mousePosition: { x, y },
           mousePosition: { x, y },
         }
         }
       }
       }
-      if (effect.is(closeContextMenuEffect)) {
-        return { tooltip: null, mousePosition: null }
-      }
+    }
+
+    // If effects changed the state, return early so doc-change fallback doesn’t override it
+    if (next !== field) {
+      return next
     }
     }
 
 
     // Close menu on document changes
     // Close menu on document changes
@@ -181,11 +192,14 @@ function openContextMenuAtPosition(
 ): void {
 ): void {
   view.dispatch({
   view.dispatch({
     selection,
     selection,
-    effects: openContextMenuEffect.of({
-      pos,
-      x: clientX,
-      y: clientY,
-    }),
+    effects: [
+      closeAllContextMenusEffect.of(null),
+      openContextMenuEffect.of({
+        pos,
+        x: clientX,
+        y: clientY,
+      }),
+    ],
   })
   })
 }
 }
 
 

+ 1 - 1
services/web/frontend/js/features/source-editor/extensions/index.ts

@@ -156,7 +156,7 @@ export const createExtensions = (options: Record<string, any>): Extension[] => [
   trackDetachedComments(options.currentDoc),
   trackDetachedComments(options.currentDoc),
   visual(options.visual),
   visual(options.visual),
   mathPreview(options.settings.mathPreview),
   mathPreview(options.settings.mathPreview),
-  reviewTooltip(),
+  reviewTooltip(options.editorContextMenuEnabled),
   contextMenu(options.editorContextMenuEnabled),
   contextMenu(options.editorContextMenuEnabled),
   toolbarPanel(),
   toolbarPanel(),
   breadcrumbPanel(),
   breadcrumbPanel(),

+ 4 - 0
services/web/frontend/js/features/source-editor/extensions/math-preview.ts

@@ -19,6 +19,7 @@ import { debugConsole } from '@/utils/debugging'
 import { nodeHasError } from '../utils/tree-operations/common'
 import { nodeHasError } from '../utils/tree-operations/common'
 import { documentEnvironments } from '../languages/latex/document-environments'
 import { documentEnvironments } from '../languages/latex/document-environments'
 import { repositionAllTooltips } from './tooltips-reposition'
 import { repositionAllTooltips } from './tooltips-reposition'
+import { closeAllContextMenusEffect } from '../utils/close-all-context-menus-effect'
 
 
 const HIDE_TOOLTIP_EVENT = 'editor:hideMathTooltip'
 const HIDE_TOOLTIP_EVENT = 'editor:hideMathTooltip'
 
 
@@ -47,6 +48,9 @@ export const mathPreviewStateField = StateField.define<{
       if (effect.is(hideTooltipEffect)) {
       if (effect.is(hideTooltipEffect)) {
         return { tooltip: null, hide: true }
         return { tooltip: null, hide: true }
       }
       }
+      if (effect.is(closeAllContextMenusEffect)) {
+        return { tooltip: null, hide: state.hide }
+      }
     }
     }
 
 
     if (tr.docChanged || tr.selection) {
     if (tr.docChanged || tr.selection) {

+ 6 - 1
services/web/frontend/js/features/source-editor/extensions/review-tooltip.ts

@@ -51,7 +51,7 @@ export const buildAddNewCommentRangeEffect = (range: SelectionRange) => {
   )
   )
 }
 }
 
 
-export const reviewTooltip = (): Extension => {
+export const reviewTooltip = (editorContextMenuEnabled = false): Extension => {
   let mouseUpListener: null | (() => void) = null
   let mouseUpListener: null | (() => void) = null
   const disableMouseUpListener = () => {
   const disableMouseUpListener = () => {
     if (mouseUpListener) {
     if (mouseUpListener) {
@@ -65,6 +65,11 @@ export const reviewTooltip = (): Extension => {
     mouseDownStateField,
     mouseDownStateField,
     EditorView.domEventHandlers({
     EditorView.domEventHandlers({
       mousedown: (event, view) => {
       mousedown: (event, view) => {
+        // Hide tooltip when opening the context menu
+        if (editorContextMenuEnabled && (event.button === 2 || event.ctrlKey)) {
+          return false
+        }
+
         disableMouseUpListener()
         disableMouseUpListener()
         mouseUpListener = () => {
         mouseUpListener = () => {
           disableMouseUpListener()
           disableMouseUpListener()

+ 17 - 8
services/web/frontend/js/features/source-editor/extensions/spelling/context-menu.tsx

@@ -17,6 +17,7 @@ import { SpellingSuggestions } from '@/features/source-editor/extensions/spellin
 import { SplitTestProvider } from '@/shared/context/split-test-context'
 import { SplitTestProvider } from '@/shared/context/split-test-context'
 import { addLearnedWord } from '@/features/source-editor/extensions/spelling/learned-words'
 import { addLearnedWord } from '@/features/source-editor/extensions/spelling/learned-words'
 import { postJSON } from '@/infrastructure/fetch-json'
 import { postJSON } from '@/infrastructure/fetch-json'
+import { closeAllContextMenusEffect } from '../../utils/close-all-context-menus-effect'
 
 
 /*
 /*
  * The time until which a click event will be ignored, so it doesn't immediately close the spelling menu.
  * The time until which a click event will be ignored, so it doesn't immediately close the spelling menu.
@@ -72,10 +73,13 @@ const handleContextMenuEvent = (event: MouseEvent, view: EditorView) => {
   openingUntil = Date.now() + 100
   openingUntil = Date.now() + 100
 
 
   view.dispatch({
   view.dispatch({
-    effects: showSpellingMenu.of({
-      mark: targetMark,
-      word: targetWord,
-    }),
+    effects: [
+      closeAllContextMenusEffect.of(null),
+      showSpellingMenu.of({
+        mark: targetMark,
+        word: targetWord,
+      }),
+    ],
   })
   })
 }
 }
 
 
@@ -87,10 +91,13 @@ const handleShortcutEvent = (view: EditorView) => {
   }
   }
 
 
   view.dispatch({
   view.dispatch({
-    effects: showSpellingMenu.of({
-      mark: targetMark,
-      word: targetMark.value.spec.word,
-    }),
+    effects: [
+      closeAllContextMenusEffect.of(null),
+      showSpellingMenu.of({
+        mark: targetMark,
+        word: targetMark.value.spec.word,
+      }),
+    ],
   })
   })
 
 
   return true
   return true
@@ -126,6 +133,8 @@ export const spellingMenuField = StateField.define<Tooltip | null>({
           strictSide: false,
           strictSide: false,
           create: createSpellingSuggestionList(word),
           create: createSpellingSuggestionList(word),
         }
         }
+      } else if (effect.is(closeAllContextMenusEffect)) {
+        value = null
       }
       }
     }
     }
     return value
     return value

+ 5 - 0
services/web/frontend/js/features/source-editor/utils/close-all-context-menus-effect.ts

@@ -0,0 +1,5 @@
+import { StateEffect } from '@codemirror/state'
+
+// Effect used to ask any loaded menu/tooltip extension to close itself.
+// Used to ensure only one context menu is open at a time.
+export const closeAllContextMenusEffect = StateEffect.define<null>()

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

@@ -2,6 +2,7 @@ import { mockScope } from '../helpers/mock-scope'
 import {
 import {
   EditorProviders,
   EditorProviders,
   makeEditorPropertiesProvider,
   makeEditorPropertiesProvider,
+  makeProjectProvider,
 } from '../../../helpers/editor-providers'
 } from '../../../helpers/editor-providers'
 import CodeMirrorEditor from '../../../../../frontend/js/features/source-editor/components/codemirror-editor'
 import CodeMirrorEditor from '../../../../../frontend/js/features/source-editor/components/codemirror-editor'
 import { TestContainer } from '../helpers/test-container'
 import { TestContainer } from '../helpers/test-container'
@@ -10,6 +11,8 @@ import { PermissionsContext } from '@/features/ide-react/context/permissions-con
 import { Permissions } from '@/features/ide-react/types/permissions'
 import { Permissions } from '@/features/ide-react/types/permissions'
 import { DetachCompileContext } from '@/shared/context/detach-compile-context'
 import { DetachCompileContext } from '@/shared/context/detach-compile-context'
 import { FileTreeDataContext } from '@/shared/context/file-tree-data-context'
 import { FileTreeDataContext } from '@/shared/context/file-tree-data-context'
+import PackageVersions from '../../../../../app/src/infrastructure/PackageVersions'
+import { mockProject } from '../helpers/mock-project'
 
 
 const createPermissionsProvider = (
 const createPermissionsProvider = (
   permissions: Partial<Permissions>
   permissions: Partial<Permissions>
@@ -808,6 +811,122 @@ describe('editor context menu', { scrollBehavior: false }, function () {
     })
     })
   })
   })
 
 
+  describe('when interacting with other tooltips/menus', function () {
+    it('should hide the add-comment tooltip when the context menu opens', function () {
+      const scope = mockScope(undefined, {
+        docOptions: { wantTrackChanges: true },
+      })
+
+      cy.mount(
+        <TestContainer>
+          <EditorProviders
+            scope={scope}
+            projectFeatures={{ trackChangesVisible: true }}
+            features={{ trackChangesVisible: true }}
+          >
+            <CodeMirrorEditor />
+          </EditorProviders>
+        </TestContainer>
+      )
+
+      cy.get('.cm-line').eq(12).type('{shift}{leftArrow}', {
+        scrollBehavior: false,
+      })
+
+      cy.get('.review-tooltip-menu').should('exist')
+
+      cy.get('.cm-line').eq(5).rightclick()
+      cy.get('.editor-context-menu').should('be.visible')
+      cy.get('.review-tooltip-menu').should('not.exist')
+    })
+
+    it('should close the spelling suggestions menu when another context menu opens', function () {
+      cy.window().then(win => {
+        win.metaAttributesCache.set('ol-learnedWords', ['baz'])
+        win.metaAttributesCache.set(
+          'ol-dictionariesRoot',
+          `js/dictionaries/${PackageVersions.version.dictionaries}/`
+        )
+        win.metaAttributesCache.set('ol-baseAssetPath', '/__cypress/src/')
+        win.metaAttributesCache.set('ol-languages', [
+          { code: 'en_GB', dic: 'en_GB', name: 'English (British)' },
+        ])
+      })
+
+      const spellcheckerContent = `
+      \\documentclass{}
+
+      \\title{}
+      \\author{}
+
+      \\begin{document}
+      \\maketitle
+
+      \\begin{abstract}
+      \\end{abstract}
+
+      \\section{}
+
+      \\end{document}`
+
+      const scope = mockScope(spellcheckerContent)
+      const project = mockProject({ spellCheckLanguage: 'en_GB' })
+
+      cy.mount(
+        <TestContainer>
+          <EditorProviders
+            scope={scope}
+            providers={{ ProjectProvider: makeProjectProvider(project) }}
+          >
+            <CodeMirrorEditor />
+          </EditorProviders>
+        </TestContainer>
+      )
+
+      cy.get('.cm-line').eq(13).as('line')
+      cy.get('@line').click()
+      cy.get('@line').type('medecin foo', { delay: 0 })
+
+      cy.get('@line')
+        .find('.ol-cm-spelling-error', { timeout: 15000 })
+        .should('have.length', 1)
+
+      cy.get('@line').find('.ol-cm-spelling-error').rightclick()
+      cy.get('.ol-cm-spelling-context-menu-tooltip').should('be.visible')
+
+      cy.get('.cm-line').eq(5).rightclick()
+      cy.get('.editor-context-menu').should('be.visible')
+      cy.get('.ol-cm-spelling-context-menu-tooltip').should('not.exist')
+    })
+
+    it('should close math preview tooltip when context menu opens', function () {
+      const scope = mockScope()
+
+      cy.mount(
+        <TestContainer>
+          <EditorProviders scope={scope}>
+            <CodeMirrorEditor />
+          </EditorProviders>
+        </TestContainer>
+      )
+
+      // Click on a math expression to show the tooltip
+      cy.get('.cm-line').eq(5).click()
+      cy.get('.cm-line')
+        .eq(5)
+        .type('$x + y$', { parseSpecialCharSequences: false })
+      // Move cursor into the math expression
+      cy.get('.cm-line').eq(5).type('{leftArrow}{leftArrow}')
+
+      cy.get('.ol-cm-math-tooltip').should('be.visible')
+
+      cy.get('.cm-line').eq(5).rightclick()
+
+      cy.get('.ol-cm-math-tooltip').should('not.exist')
+      cy.get('.editor-context-menu').should('be.visible')
+    })
+  })
+
   describe('when right-clicking on the gutter', function () {
   describe('when right-clicking on the gutter', function () {
     const editorLine = 2
     const editorLine = 2
     const gutterLineIndex = editorLine + 1 // extra hidden gutter line
     const gutterLineIndex = editorLine + 1 // extra hidden gutter line