Merge pull request #29971 from overleaf/mg-edit-search-bug

Make editor focusable in view-only mode

GitOrigin-RevId: ed9b079fa379d84f7f410669fa2d865f82e21cb1

services/web/frontend/js/features/source-editor/extensions/editable.ts

@@ -3,27 +3,43 @@ import { EditorView } from '@codemirror/view'
 
 const readOnlyConf = new Compartment()
 
+// Make the editor focusable even when contenteditable="false" (read-only mode)
+// This allows keyboard shortcuts like Cmd+F to work in read-only mode
+const focusableReadOnly = EditorView.contentAttributes.of({ tabindex: '0' })
+
+// Hide the blinking cursor in read-only mode
+const hideCursor = EditorView.theme({
+  '&.cm-editor .cm-cursorLayer': {
+    display: 'none',
+  },
+})
+
+const readOnlyAttributes = [
+  EditorState.readOnly.of(true),
+  EditorView.editable.of(false),
+  focusableReadOnly,
+  hideCursor,
+]
+
+const editableAttributes = [
+  EditorState.readOnly.of(false),
+  EditorView.editable.of(true),
+]
+
 /**
  * A custom extension which determines whether the content is editable, by setting the value of the EditorState.readOnly and EditorView.editable facets.
  * Commands and extensions read the EditorState.readOnly facet to decide whether they should be applied.
  * EditorView.editable determines whether the DOM can be focused, by changing the value of the contenteditable attribute.
+ * We add tabindex="0" in read-only mode to ensure the editor remains focusable for keyboard shortcuts.
  */
 export const editable = () => {
-  return [
-    readOnlyConf.of([
-      EditorState.readOnly.of(true),
-      EditorView.editable.of(false),
-    ]),
-  ]
+  return [readOnlyConf.of(readOnlyAttributes)]
 }
 
 export const setEditable = (value = true): TransactionSpec => {
   return {
     effects: [
-      readOnlyConf.reconfigure([
-        EditorState.readOnly.of(!value),
-        EditorView.editable.of(value),
-      ]),
+      readOnlyConf.reconfigure(value ? editableAttributes : readOnlyAttributes),
     ],
   }
 }

services/web/test/frontend/features/source-editor/components/codemirror-editor-visual-readonly.spec.tsx

@@ -8,6 +8,7 @@ import { FC } from 'react'
 import { FileTreePathContext } from '@/features/file-tree/contexts/file-tree-path'
 import { TestContainer } from '../helpers/test-container'
 import { PermissionsContext } from '@/features/ide-react/context/permissions-context'
+import { metaKey } from '../helpers/meta-key'
 
 const FileTreePathProvider: FC<React.PropsWithChildren> = ({ children }) => (
   <FileTreePathContext.Provider
@@ -154,4 +155,56 @@ describe('<CodeMirrorEditor/> in Visual mode with read-only permission', functio
     cy.findByLabelText('URL').should('be.disabled')
     cy.findByRole('button', { name: 'Remove link' }).should('not.exist')
   })
+
+  it('opens the CodeMirror search panel with Cmd/Ctrl+F', function () {
+    mountEditor('Hello world\n\nThis is a test document.')
+
+    // Click to focus the editor
+    cy.get('.cm-content').click()
+
+    // Search panel should not be open initially
+    cy.findByRole('search').should('not.exist')
+
+    // Press Cmd/Ctrl+F to open search
+    cy.get('.cm-content').type(`{${metaKey}+f}`)
+
+    // Search panel should now be open
+    cy.findByRole('search').should('exist')
+    cy.findByRole('textbox', { name: 'Find' }).should('be.visible')
+  })
+
+  it('allows searching for text in read-only mode', function () {
+    mountEditor('Hello world\n\nThis is a test document with hello again.')
+
+    // Click to focus the editor
+    cy.get('.cm-content').click()
+
+    // Open search panel
+    cy.get('.cm-content').type(`{${metaKey}+f}`)
+
+    // Type a search query
+    cy.findByRole('textbox', { name: 'Find' }).type('hello')
+
+    // Should find matches (case insensitive)
+    cy.get('.cm-searchMatch').should('have.length.at.least', 1)
+  })
+
+  it('closes the search panel with Escape', function () {
+    mountEditor('Hello world')
+
+    // Click to focus the editor
+    cy.get('.cm-content').click()
+
+    // Open search panel
+    cy.get('.cm-content').type(`{${metaKey}+f}`)
+
+    // Search panel should be open
+    cy.findByRole('search').should('exist')
+
+    // Press Escape to close
+    cy.findByRole('textbox', { name: 'Find' }).type('{esc}')
+
+    // Search panel should be closed
+    cy.findByRole('search').should('not.exist')
+  })
 })

services/web/test/frontend/features/source-editor/extensions/editable.test.ts

@@ -0,0 +1,130 @@
+import { expect } from 'chai'
+import { EditorState } from '@codemirror/state'
+import { EditorView } from '@codemirror/view'
+import {
+  editable,
+  setEditable,
+} from '../../../../../frontend/js/features/source-editor/extensions/editable'
+
+const doc = `\\documentclass{article}
+\\begin{document}
+Hello world
+\\end{document}`
+
+describe('editable extension', function () {
+  let view: EditorView
+  let container: HTMLElement
+
+  beforeEach(function () {
+    container = document.createElement('div')
+    document.body.appendChild(container)
+  })
+
+  afterEach(function () {
+    view?.destroy()
+    container?.remove()
+  })
+
+  function createView(extensions = [editable()]) {
+    view = new EditorView({
+      parent: container,
+      state: EditorState.create({
+        doc,
+        extensions,
+      }),
+    })
+    return view
+  }
+
+  describe('initial read-only state', function () {
+    beforeEach(function () {
+      createView()
+    })
+
+    it('should set EditorState.readOnly to true', function () {
+      expect(view.state.readOnly).to.be.true
+    })
+
+    it('should set EditorView.editable to false', function () {
+      expect(view.state.facet(EditorView.editable)).to.be.false
+    })
+
+    it('should set contenteditable="false" on the content element', function () {
+      expect(view.contentDOM.getAttribute('contenteditable')).to.equal('false')
+    })
+
+    it('should set tabindex="0" to allow focus in read-only mode', function () {
+      expect(view.contentDOM.getAttribute('tabindex')).to.equal('0')
+    })
+
+    it('should allow the editor to receive focus via tabindex', function () {
+      view.contentDOM.focus()
+      expect(document.activeElement).to.equal(view.contentDOM)
+    })
+  })
+
+  describe('setEditable(true) - switching to editable mode', function () {
+    beforeEach(function () {
+      createView()
+      view.dispatch(setEditable(true))
+    })
+
+    it('should set EditorState.readOnly to false', function () {
+      expect(view.state.readOnly).to.be.false
+    })
+
+    it('should set EditorView.editable to true', function () {
+      expect(view.state.facet(EditorView.editable)).to.be.true
+    })
+
+    it('should set contenteditable="true" on the content element', function () {
+      expect(view.contentDOM.getAttribute('contenteditable')).to.equal('true')
+    })
+
+    it('should not have tabindex attribute (not needed when contenteditable)', function () {
+      expect(view.contentDOM.getAttribute('tabindex')).to.be.null
+    })
+
+    it('should allow document modifications', function () {
+      view.dispatch({
+        changes: { from: 0, insert: 'New text ' },
+      })
+
+      expect(view.state.doc.toString().startsWith('New text ')).to.be.true
+    })
+
+    it('should allow the editor to receive focus', function () {
+      view.contentDOM.focus()
+      expect(document.activeElement).to.equal(view.contentDOM)
+    })
+  })
+
+  describe('setEditable(false) - switching to read-only mode', function () {
+    beforeEach(function () {
+      createView()
+      view.dispatch(setEditable(true))
+      view.dispatch(setEditable(false))
+    })
+
+    it('should set EditorState.readOnly to true', function () {
+      expect(view.state.readOnly).to.be.true
+    })
+
+    it('should set EditorView.editable to false', function () {
+      expect(view.state.facet(EditorView.editable)).to.be.false
+    })
+
+    it('should set contenteditable="false" on the content element', function () {
+      expect(view.contentDOM.getAttribute('contenteditable')).to.equal('false')
+    })
+
+    it('should restore tabindex="0" for focusability', function () {
+      expect(view.contentDOM.getAttribute('tabindex')).to.equal('0')
+    })
+
+    it('should still allow the editor to receive focus after switching modes', function () {
+      view.contentDOM.focus()
+      expect(document.activeElement).to.equal(view.contentDOM)
+    })
+  })
+})