Merge pull request #33687 from overleaf/mj-temporary-tabs-fix

[web] Only consider real key presses to make tab permanent

GitOrigin-RevId: 50ab453445e111de2b317f50470f9f4eec39a66f

services/web/frontend/js/features/source-editor/components/tabs/tab.tsx

@@ -17,6 +17,10 @@ import {
   useState,
 } from 'react'
 import { useTranslation } from 'react-i18next'
+import {
+  TAB_USER_EDIT_EVENT,
+  tabsEvents,
+} from '@/features/source-editor/extensions/tabs-listener'
 
 type TabProps = {
   tab: EditorFileTab
@@ -186,9 +190,9 @@ export const Tab = memo(function Tab({
       const handler = () => {
         makeTabPermanent(tab.id)
       }
-      document.body.addEventListener('keydown', handler)
+      tabsEvents.addEventListener(TAB_USER_EDIT_EVENT, handler)
       return () => {
-        document.body.removeEventListener('keydown', handler)
+        tabsEvents.removeEventListener(TAB_USER_EDIT_EVENT, handler)
       }
     }
   }, [isSelected, makeTabPermanent, tab])

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

@@ -57,6 +57,8 @@ import { reviewTooltip } from './review-tooltip'
 import { tooltipsReposition } from './tooltips-reposition'
 import { selectionListener } from '@/features/source-editor/extensions/selection-listener'
 import { contextMenu } from './context-menu'
+import { tabsListener } from './tabs-listener'
+import { isSplitTestEnabled } from '@/utils/splitTestUtils'
 
 const moduleExtensions: Array<(options: Record<string, any>) => Extension> =
   importOverleafModules('sourceEditorExtensions').map(
@@ -178,4 +180,5 @@ export const createExtensions = (options: Record<string, any>): Extension[] => [
   fileTreeItemDrop(),
   tooltipsReposition(),
   selectionListener(options.setEditorSelection),
+  isSplitTestEnabled('editor-tabs') ? tabsListener() : [],
 ]

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

@@ -0,0 +1,17 @@
+import { Transaction } from '@codemirror/state'
+import { EditorView } from '@codemirror/view'
+
+export const TAB_USER_EDIT_EVENT = 'tab-user-edit'
+
+export const tabsEvents = new EventTarget()
+
+export const tabsListener = () =>
+  EditorView.updateListener.of(update => {
+    if (!update.docChanged) return
+    for (const transaction of update.transactions) {
+      if (!transaction.annotation(Transaction.remote)) {
+        tabsEvents.dispatchEvent(new Event(TAB_USER_EDIT_EVENT))
+        return
+      }
+    }
+  })

services/web/test/frontend/features/source-editor/components/tabs.spec.tsx

@@ -1,4 +1,4 @@
-import React, { FC, useEffect, useRef } from 'react'
+import React, { FC, useEffect, useRef, useState } from 'react'
 import { EditorProviders } from '../../../helpers/editor-providers'
 import { TabsContainer } from '../../../../../frontend/js/features/source-editor/components/tabs/tabs-container'
 import {
@@ -11,6 +11,13 @@ import {
 } from '@/features/ide-react/context/editor-manager-context'
 import { TAB_TRANSFER_TYPE } from '@/features/ide-react/context/tabs-context'
 import { useFileTreeOpenContext } from '@/features/ide-react/context/file-tree-open-context'
+import {
+  EditorViewContext,
+  useEditorViewContext,
+} from '@/features/ide-react/context/editor-view-context'
+import { EditorView } from '@codemirror/view'
+import { EditorState, Transaction } from '@codemirror/state'
+import { tabsListener } from '@/features/source-editor/extensions/tabs-listener'
 
 const DOC_IDS = {
   main: 'doc-main-id',
@@ -120,6 +127,53 @@ function makeEditorManagerProvider() {
   return EditorManagerProvider
 }
 
+function makeEditorViewProvider() {
+  const EditorViewProvider: FC<React.PropsWithChildren> = ({ children }) => {
+    const parentRef = useRef<HTMLDivElement>(null)
+    const [view, setView] = useState<EditorView | null>(null)
+    useEffect(() => {
+      if (!parentRef.current) return
+      const editorView = new EditorView({
+        state: EditorState.create({
+          extensions: [
+            tabsListener(),
+            EditorView.contentAttributes.of({
+              'data-testid': 'mock-editor-view',
+            }),
+          ],
+        }),
+        parent: parentRef.current,
+      })
+      setView(editorView)
+      return () => editorView.destroy()
+    }, [])
+    return (
+      <EditorViewContext.Provider value={{ view, setView: () => {} }}>
+        {children}
+        <div ref={parentRef} />
+      </EditorViewContext.Provider>
+    )
+  }
+  return EditorViewProvider
+}
+
+function RemoteChangeButton() {
+  const { view } = useEditorViewContext()
+  return (
+    <button
+      type="button"
+      onClick={() =>
+        view?.dispatch({
+          changes: { from: 0, insert: 'remote text' },
+          annotations: Transaction.remote.of(true),
+        })
+      }
+    >
+      Add a remote change
+    </button>
+  )
+}
+
 // Rendered inside the provider tree to call handleFileTreeSelect() when a
 // custom DOM event fires. Also triggers handleFileTreeInit() on mount.
 function FileSelectionDriver({
@@ -188,10 +242,12 @@ describe('File Tabs', function () {
         userSettings={options?.userSettings}
         providers={{
           EditorManagerProvider: makeEditorManagerProvider(),
+          EditorViewProvider: makeEditorViewProvider(),
         }}
       >
         <FileSelectionDriver />
         <TabsContainer />
+        <RemoteChangeButton />
       </EditorProviders>
     )
   }
@@ -216,6 +272,8 @@ describe('File Tabs', function () {
     })
 
     mountTabs()
+
+    cy.findByTestId('mock-editor-view').as('editorView')
   })
 
   describe('Initial file selection', function () {
@@ -228,6 +286,7 @@ describe('File Tabs', function () {
           rootDocId={DOC_IDS.main}
           providers={{
             EditorManagerProvider: makeEditorManagerProvider(),
+            EditorViewProvider: makeEditorViewProvider(),
           }}
         >
           <FileSelectionDriver
@@ -266,7 +325,7 @@ describe('File Tabs', function () {
       cy.findByRole('tab', { name: /main\.tex/ }).should('exist')
 
       // Make main permanent (keypress) so selecting another file doesn't replace it
-      cy.get('body').type('a')
+      cy.get('@editorView').type('a')
 
       // Select another file
       cy.then(() => selectDoc(DOC_IDS.intro))
@@ -307,7 +366,7 @@ describe('File Tabs', function () {
         'tab-temporary'
       )
 
-      cy.get('body').type('a')
+      cy.get('@editorView').type('a')
 
       cy.findByRole('tab', { name: /main\.tex/ }).should(
         'not.have.class',
@@ -321,7 +380,7 @@ describe('File Tabs', function () {
       cy.findByRole('tab', { name: /main\.tex/ }).should('exist')
 
       // Make main permanent
-      cy.get('body').type('a')
+      cy.get('@editorView').type('a')
 
       // Open intro (temporary)
       cy.then(() => selectDoc(DOC_IDS.intro))
@@ -338,6 +397,21 @@ describe('File Tabs', function () {
       cy.findByRole('tab', { name: /main\.tex/ }).should('exist')
     })
 
+    it('does not make a temporary tab permanent on remote changes', function () {
+      cy.then(() => selectDoc(DOC_IDS.main))
+      cy.findByRole('tab', { name: /main\.tex/ }).should(
+        'have.class',
+        'tab-temporary'
+      )
+
+      cy.findByRole('button', { name: 'Add a remote change' }).click()
+
+      cy.findByRole('tab', { name: /main\.tex/ }).should(
+        'have.class',
+        'tab-temporary'
+      )
+    })
+
     it('makes a temporary tab permanent on double-click', function () {
       cy.then(() => selectDoc(DOC_IDS.main))
       cy.findByRole('tab', { name: /main\.tex/ }).should(
@@ -902,7 +976,7 @@ describe('File Tabs', function () {
       for (let i = 1; i <= 10; i++) {
         const id = `ch${i}`
         cy.then(() => selectEntity(makeDocEntity(id, `chapter-${i}.tex`)))
-        cy.get('body').type('a')
+        cy.get('@editorView').type('a')
         cy.findByRole('tab', { name: new RegExp(`chapter-${i}.tex`) }).should(
           'be.visible'
         )