diff --git a/services/web/frontend/js/features/source-editor/extensions/context-menu.ts b/services/web/frontend/js/features/source-editor/extensions/context-menu.ts index a91cc72037..19bff70c93 100644 --- a/services/web/frontend/js/features/source-editor/extensions/context-menu.ts +++ b/services/web/frontend/js/features/source-editor/extensions/context-menu.ts @@ -13,6 +13,7 @@ import { EditorSelection, Prec, } from '@codemirror/state' +import { closeAllContextMenusEffect } from '../utils/close-all-context-menus-effect' export const openContextMenuEffect = StateEffect.define<{ pos: number @@ -33,8 +34,16 @@ export const contextMenuStateField = StateField.define({ }, 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) { + if ( + effect.is(closeContextMenuEffect) || + effect.is(closeAllContextMenusEffect) + ) { + next = { tooltip: null, mousePosition: null } + } if (effect.is(openContextMenuEffect)) { const { pos, x, y } = effect.value return { @@ -42,9 +51,11 @@ export const contextMenuStateField = StateField.define({ 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 @@ -181,11 +192,14 @@ function openContextMenuAtPosition( ): void { view.dispatch({ selection, - effects: openContextMenuEffect.of({ - pos, - x: clientX, - y: clientY, - }), + effects: [ + closeAllContextMenusEffect.of(null), + openContextMenuEffect.of({ + pos, + x: clientX, + y: clientY, + }), + ], }) } diff --git a/services/web/frontend/js/features/source-editor/extensions/index.ts b/services/web/frontend/js/features/source-editor/extensions/index.ts index a86c3c9d28..472a460d1e 100644 --- a/services/web/frontend/js/features/source-editor/extensions/index.ts +++ b/services/web/frontend/js/features/source-editor/extensions/index.ts @@ -156,7 +156,7 @@ export const createExtensions = (options: Record): Extension[] => [ trackDetachedComments(options.currentDoc), visual(options.visual), mathPreview(options.settings.mathPreview), - reviewTooltip(), + reviewTooltip(options.editorContextMenuEnabled), contextMenu(options.editorContextMenuEnabled), toolbarPanel(), breadcrumbPanel(), diff --git a/services/web/frontend/js/features/source-editor/extensions/math-preview.ts b/services/web/frontend/js/features/source-editor/extensions/math-preview.ts index 1b9e051978..1c84adb756 100644 --- a/services/web/frontend/js/features/source-editor/extensions/math-preview.ts +++ b/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 { documentEnvironments } from '../languages/latex/document-environments' import { repositionAllTooltips } from './tooltips-reposition' +import { closeAllContextMenusEffect } from '../utils/close-all-context-menus-effect' const HIDE_TOOLTIP_EVENT = 'editor:hideMathTooltip' @@ -47,6 +48,9 @@ export const mathPreviewStateField = StateField.define<{ if (effect.is(hideTooltipEffect)) { return { tooltip: null, hide: true } } + if (effect.is(closeAllContextMenusEffect)) { + return { tooltip: null, hide: state.hide } + } } if (tr.docChanged || tr.selection) { diff --git a/services/web/frontend/js/features/source-editor/extensions/review-tooltip.ts b/services/web/frontend/js/features/source-editor/extensions/review-tooltip.ts index 1655a11e4a..0808e76517 100644 --- a/services/web/frontend/js/features/source-editor/extensions/review-tooltip.ts +++ b/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 const disableMouseUpListener = () => { if (mouseUpListener) { @@ -65,6 +65,11 @@ export const reviewTooltip = (): Extension => { mouseDownStateField, EditorView.domEventHandlers({ mousedown: (event, view) => { + // Hide tooltip when opening the context menu + if (editorContextMenuEnabled && (event.button === 2 || event.ctrlKey)) { + return false + } + disableMouseUpListener() mouseUpListener = () => { disableMouseUpListener() diff --git a/services/web/frontend/js/features/source-editor/extensions/spelling/context-menu.tsx b/services/web/frontend/js/features/source-editor/extensions/spelling/context-menu.tsx index deac7d2956..edb42dea40 100644 --- a/services/web/frontend/js/features/source-editor/extensions/spelling/context-menu.tsx +++ b/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 { addLearnedWord } from '@/features/source-editor/extensions/spelling/learned-words' 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. @@ -72,10 +73,13 @@ const handleContextMenuEvent = (event: MouseEvent, view: EditorView) => { openingUntil = Date.now() + 100 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({ - 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 @@ -126,6 +133,8 @@ export const spellingMenuField = StateField.define({ strictSide: false, create: createSpellingSuggestionList(word), } + } else if (effect.is(closeAllContextMenusEffect)) { + value = null } } return value diff --git a/services/web/frontend/js/features/source-editor/utils/close-all-context-menus-effect.ts b/services/web/frontend/js/features/source-editor/utils/close-all-context-menus-effect.ts new file mode 100644 index 0000000000..483447d66d --- /dev/null +++ b/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() 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 beec118334..3f1d98c425 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 @@ -2,6 +2,7 @@ import { mockScope } from '../helpers/mock-scope' import { EditorProviders, makeEditorPropertiesProvider, + makeProjectProvider, } from '../../../helpers/editor-providers' import CodeMirrorEditor from '../../../../../frontend/js/features/source-editor/components/codemirror-editor' 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 { DetachCompileContext } from '@/shared/context/detach-compile-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 = ( permissions: Partial @@ -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( + + + + + + ) + + 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( + + + + + + ) + + 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( + + + + + + ) + + // 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 () { const editorLine = 2 const gutterLineIndex = editorLine + 1 // extra hidden gutter line