Răsfoiți Sursa

Reapply "Increase the length of unsaved doc time before the editor is locked" (#23723)

* Reapply "Increase the length of unsaved doc time before the editor is locked (#23431)"

This reverts commit d0efca398e59ca488c530ee9ec5ca76f29843679.

* Use pendingOpCreatedAt if inflightOpCreatedAt isn't set

GitOrigin-RevId: b3301d7cc1554215a4ba569537bd47beeefde20f
Alf Eaton 1 an în urmă
părinte
comite
8ca5360454

+ 3 - 2
services/web/frontend/extracted-translations.json

@@ -303,7 +303,7 @@
   "congratulations_youve_successfully_join_group": "",
   "congratulations_youve_successfully_join_group": "",
   "connect_overleaf_with_github": "",
   "connect_overleaf_with_github": "",
   "connected_users": "",
   "connected_users": "",
-  "connection_lost": "",
+  "connection_lost_with_unsaved_changes": "",
   "contact_group_admin": "",
   "contact_group_admin": "",
   "contact_sales": "",
   "contact_sales": "",
   "contact_support_to_change_group_subscription": "",
   "contact_support_to_change_group_subscription": "",
@@ -416,6 +416,7 @@
   "doing_this_will_verify_affiliation_and_allow_log_in_2": "",
   "doing_this_will_verify_affiliation_and_allow_log_in_2": "",
   "done": "",
   "done": "",
   "dont_forget_you_currently_have": "",
   "dont_forget_you_currently_have": "",
+  "dont_reload_or_close_this_tab": "",
   "download": "",
   "download": "",
   "download_all": "",
   "download_all": "",
   "download_metadata": "",
   "download_metadata": "",
@@ -1523,7 +1524,6 @@
   "something_went_wrong_server": "",
   "something_went_wrong_server": "",
   "somthing_went_wrong_compiling": "",
   "somthing_went_wrong_compiling": "",
   "sorry_it_looks_like_that_didnt_work_this_time": "",
   "sorry_it_looks_like_that_didnt_work_this_time": "",
-  "sorry_the_connection_to_the_server_is_down": "",
   "sorry_there_are_no_experiments": "",
   "sorry_there_are_no_experiments": "",
   "sorry_your_table_cant_be_displayed_at_the_moment": "",
   "sorry_your_table_cant_be_displayed_at_the_moment": "",
   "sort_by": "",
   "sort_by": "",
@@ -2044,6 +2044,7 @@
   "your_add_on_has_been_cancelled_and_will_remain_active_until_your_billing_cycle_ends_on": "",
   "your_add_on_has_been_cancelled_and_will_remain_active_until_your_billing_cycle_ends_on": "",
   "your_affiliation_is_confirmed": "",
   "your_affiliation_is_confirmed": "",
   "your_browser_does_not_support_this_feature": "",
   "your_browser_does_not_support_this_feature": "",
+  "your_changes_will_save": "",
   "your_compile_timed_out": "",
   "your_compile_timed_out": "",
   "your_current_plan": "",
   "your_current_plan": "",
   "your_current_plan_gives_you": "",
   "your_current_plan_gives_you": "",

+ 3 - 1
services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-alert.tsx

@@ -3,13 +3,15 @@ import { useFileTreePathContext } from '@/features/file-tree/contexts/file-tree-
 import { useTranslation } from 'react-i18next'
 import { useTranslation } from 'react-i18next'
 import OLNotification from '@/features/ui/components/ol/ol-notification'
 import OLNotification from '@/features/ui/components/ol/ol-notification'
 
 
+const MAX_UNSAVED_ALERT_SECONDS = 15
+
 export const UnsavedDocsAlert: FC<{ unsavedDocs: Map<string, number> }> = ({
 export const UnsavedDocsAlert: FC<{ unsavedDocs: Map<string, number> }> = ({
   unsavedDocs,
   unsavedDocs,
 }) => (
 }) => (
   <>
   <>
     {[...unsavedDocs.entries()].map(
     {[...unsavedDocs.entries()].map(
       ([docId, seconds]) =>
       ([docId, seconds]) =>
-        seconds > 8 && (
+        seconds >= MAX_UNSAVED_ALERT_SECONDS && (
           <UnsavedDocAlert key={docId} docId={docId} seconds={seconds} />
           <UnsavedDocAlert key={docId} docId={docId} seconds={seconds} />
         )
         )
     )}
     )}

+ 19 - 0
services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert.tsx

@@ -0,0 +1,19 @@
+import { FC } from 'react'
+import { useTranslation } from 'react-i18next'
+import OLNotification from '@/features/ui/components/ol/ol-notification'
+
+export const UnsavedDocsLockedAlert: FC = () => {
+  const { t } = useTranslation()
+
+  return (
+    <OLNotification
+      type="warning"
+      content={
+        <>
+          <strong>{t('connection_lost_with_unsaved_changes')}</strong>{' '}
+          {t('dont_reload_or_close_this_tab')} {t('your_changes_will_save')}
+        </>
+      }
+    />
+  )
+}

+ 0 - 28
services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal.tsx

@@ -1,28 +0,0 @@
-import { FC } from 'react'
-import { useTranslation } from 'react-i18next'
-import OLModal, {
-  OLModalBody,
-  OLModalHeader,
-  OLModalTitle,
-} from '@/features/ui/components/ol/ol-modal'
-
-export const UnsavedDocsLockedModal: FC = () => {
-  const { t } = useTranslation()
-
-  return (
-    <OLModal
-      show
-      onHide={() => {}} // It's not possible to hide this modal, but it's a required prop
-      className="lock-editor-modal"
-      backdrop={false}
-      keyboard={false}
-    >
-      <OLModalHeader>
-        <OLModalTitle>{t('connection_lost')}</OLModalTitle>
-      </OLModalHeader>
-      <OLModalBody>
-        {t('sorry_the_connection_to_the_server_is_down')}
-      </OLModalBody>
-    </OLModal>
-  )
-}

+ 19 - 12
services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs.tsx

@@ -2,13 +2,13 @@ import { useEditorManagerContext } from '@/features/ide-react/context/editor-man
 import { useEditorContext } from '@/shared/context/editor-context'
 import { useEditorContext } from '@/shared/context/editor-context'
 import { FC, useCallback, useEffect, useRef, useState } from 'react'
 import { FC, useCallback, useEffect, useRef, useState } from 'react'
 import { PermissionsLevel } from '@/features/ide-react/types/permissions'
 import { PermissionsLevel } from '@/features/ide-react/types/permissions'
-import { UnsavedDocsLockedModal } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal'
+import { UnsavedDocsLockedAlert } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert'
 import { UnsavedDocsAlert } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-alert'
 import { UnsavedDocsAlert } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-alert'
 import useEventListener from '@/shared/hooks/use-event-listener'
 import useEventListener from '@/shared/hooks/use-event-listener'
 import { createPortal } from 'react-dom'
 import { createPortal } from 'react-dom'
 import { useGlobalAlertsContainer } from '@/features/ide-react/context/global-alerts-context'
 import { useGlobalAlertsContainer } from '@/features/ide-react/context/global-alerts-context'
 
 
-const MAX_UNSAVED_SECONDS = 15 // lock the editor after this time if unsaved
+const MAX_UNSAVED_SECONDS = 30 // lock the editor after this time if unsaved
 
 
 export const UnsavedDocs: FC = () => {
 export const UnsavedDocs: FC = () => {
   const { openDocs, debugTimers } = useEditorManagerContext()
   const { openDocs, debugTimers } = useEditorManagerContext()
@@ -19,9 +19,6 @@ export const UnsavedDocs: FC = () => {
 
 
   // always contains the latest value
   // always contains the latest value
   const previousUnsavedDocsRef = useRef(unsavedDocs)
   const previousUnsavedDocsRef = useRef(unsavedDocs)
-  useEffect(() => {
-    previousUnsavedDocsRef.current = unsavedDocs
-  }, [unsavedDocs])
 
 
   // always contains the latest value
   // always contains the latest value
   const permissionsLevelRef = useRef(permissionsLevel)
   const permissionsLevelRef = useRef(permissionsLevel)
@@ -50,12 +47,17 @@ export const UnsavedDocs: FC = () => {
       debugTimers.current.CheckUnsavedDocs = Date.now()
       debugTimers.current.CheckUnsavedDocs = Date.now()
       const unsavedDocs = new Map()
       const unsavedDocs = new Map()
 
 
-      const unsavedDocIds = openDocs.unsavedDocIds()
+      const docs = openDocs.unsavedDocs()
 
 
-      for (const docId of unsavedDocIds) {
-        const unsavedSeconds =
-          (previousUnsavedDocsRef.current.get(docId) ?? 0) + 1
-        unsavedDocs.set(docId, unsavedSeconds)
+      for (const doc of docs) {
+        const oldestOpCreatedAt =
+          doc.getInflightOpCreatedAt() ?? doc.getPendingOpCreatedAt()
+        if (oldestOpCreatedAt) {
+          const unsavedSeconds = Math.floor(
+            (performance.now() - oldestOpCreatedAt) / 1000
+          )
+          unsavedDocs.set(doc.doc_id, unsavedSeconds)
+        }
       }
       }
 
 
       // avoid setting the unsavedDocs state to a new empty Map every second
       // avoid setting the unsavedDocs state to a new empty Map every second
@@ -100,11 +102,16 @@ export const UnsavedDocs: FC = () => {
     }
     }
   }, [unsavedDocs])
   }, [unsavedDocs])
 
 
+  if (!globalAlertsContainer) {
+    return null
+  }
+
   return (
   return (
     <>
     <>
-      {isLocked && <UnsavedDocsLockedModal />}
+      {isLocked &&
+        createPortal(<UnsavedDocsLockedAlert />, globalAlertsContainer)}
+
       {unsavedDocs.size > 0 &&
       {unsavedDocs.size > 0 &&
-        globalAlertsContainer &&
         createPortal(
         createPortal(
           <UnsavedDocsAlert unsavedDocs={unsavedDocs} />,
           <UnsavedDocsAlert unsavedDocs={unsavedDocs} />,
           globalAlertsContainer
           globalAlertsContainer

+ 9 - 1
services/web/frontend/js/features/ide-react/editor/document-container.ts

@@ -182,6 +182,14 @@ export class DocumentContainer extends EventEmitter {
     return this.doc?.getRecentAck()
     return this.doc?.getRecentAck()
   }
   }
 
 
+  getInflightOpCreatedAt() {
+    return this.doc?.getInflightOpCreatedAt()
+  }
+
+  getPendingOpCreatedAt() {
+    return this.doc?.getPendingOpCreatedAt()
+  }
+
   hasBufferedOps() {
   hasBufferedOps() {
     return this.doc?.hasBufferedOps()
     return this.doc?.hasBufferedOps()
   }
   }
@@ -354,7 +362,7 @@ export class DocumentContainer extends EventEmitter {
       pendingOpSize < MAX_PENDING_OP_SIZE
       pendingOpSize < MAX_PENDING_OP_SIZE
     ) {
     ) {
       // There is an op waiting to go to server but it is small and
       // There is an op waiting to go to server but it is small and
-      // within the flushDelay, this is OK for now.
+      // within the recent ack limit, this is OK for now.
       saved = true
       saved = true
       debugConsole.log(
       debugConsole.log(
         '[pollSavedStatus] pending op (small with recent ack) assume ok',
         '[pollSavedStatus] pending op (small with recent ack) assume ok',

+ 5 - 5
services/web/frontend/js/features/ide-react/editor/open-documents.ts

@@ -83,14 +83,14 @@ export class OpenDocuments {
     }
     }
   }
   }
 
 
-  unsavedDocIds() {
-    const ids = []
-    for (const [docId, doc] of this.openDocs) {
+  unsavedDocs() {
+    const docs = []
+    for (const doc of this.openDocs.values()) {
       if (!doc.pollSavedStatus()) {
       if (!doc.pollSavedStatus()) {
-        ids.push(docId)
+        docs.push(doc)
       }
       }
     }
     }
-    return ids
+    return docs
   }
   }
 
 
   async awaitBufferedOps(signal: AbortSignal) {
   async awaitBufferedOps(signal: AbortSignal) {

+ 26 - 5
services/web/frontend/js/features/ide-react/editor/share-js-doc.ts

@@ -23,7 +23,8 @@ const SINGLE_USER_FLUSH_DELAY = 2000
 const MULTI_USER_FLUSH_DELAY = 500
 const MULTI_USER_FLUSH_DELAY = 500
 const INFLIGHT_OP_TIMEOUT = 5000 // Retry sending ops after 5 seconds without an ack
 const INFLIGHT_OP_TIMEOUT = 5000 // Retry sending ops after 5 seconds without an ack
 const WAIT_FOR_CONNECTION_TIMEOUT = 500
 const WAIT_FOR_CONNECTION_TIMEOUT = 500
-const FATAL_OP_TIMEOUT = 30000
+const FATAL_OP_TIMEOUT = 45000
+const RECENT_ACK_LIMIT = 2 * SINGLE_USER_FLUSH_DELAY
 
 
 type Update = Record<string, any>
 type Update = Record<string, any>
 
 
@@ -42,7 +43,9 @@ export class ShareJsDoc extends EventEmitter {
   // @ts-ignore
   // @ts-ignore
   _doc: Doc
   _doc: Doc
   private editorWatchdogManager: EditorWatchdogManager
   private editorWatchdogManager: EditorWatchdogManager
-  private lastAcked: Date | null = null
+  private lastAcked: number | null = null
+  private pendingOpCreatedAt: number | null = null
+  private inflightOpCreatedAt: number | null = null
   private queuedMessageTimer: number | null = null
   private queuedMessageTimer: number | null = null
   private queuedMessages: Message[] = []
   private queuedMessages: Message[] = []
   private detachEditorWatchdogManager: (() => void) | null = null
   private detachEditorWatchdogManager: (() => void) | null = null
@@ -90,13 +93,19 @@ export class ShareJsDoc extends EventEmitter {
     })
     })
     this._doc.setFlushDelay(SINGLE_USER_FLUSH_DELAY)
     this._doc.setFlushDelay(SINGLE_USER_FLUSH_DELAY)
     this._doc.on('change', (...args: any[]) => {
     this._doc.on('change', (...args: any[]) => {
+      if (!this.pendingOpCreatedAt) {
+        debugConsole.log('set pendingOpCreatedAt', new Date())
+        this.pendingOpCreatedAt = performance.now()
+      }
       return this.trigger('change', ...args)
       return this.trigger('change', ...args)
     })
     })
     this.editorWatchdogManager = new EditorWatchdogManager({
     this.editorWatchdogManager = new EditorWatchdogManager({
       parent: globalEditorWatchdogManager,
       parent: globalEditorWatchdogManager,
     })
     })
     this._doc.on('acknowledge', () => {
     this._doc.on('acknowledge', () => {
-      this.lastAcked = new Date() // note time of last ack from server for an op we sent
+      this.lastAcked = performance.now() // note time of last ack from server for an op we sent
+      this.inflightOpCreatedAt = null
+      debugConsole.log('unset inflightOpCreatedAt')
       this.editorWatchdogManager.onAck() // keep track of last ack globally
       this.editorWatchdogManager.onAck() // keep track of last ack globally
       return this.trigger('acknowledge')
       return this.trigger('acknowledge')
     })
     })
@@ -107,6 +116,10 @@ export class ShareJsDoc extends EventEmitter {
       return this.trigger('remoteop', ...args)
       return this.trigger('remoteop', ...args)
     })
     })
     this._doc.on('flipped_pending_to_inflight', () => {
     this._doc.on('flipped_pending_to_inflight', () => {
+      this.inflightOpCreatedAt = this.pendingOpCreatedAt
+      debugConsole.log('set inflightOpCreatedAt from pendingOpCreatedAt')
+      this.pendingOpCreatedAt = null
+      debugConsole.log('unset pendingOpCreatedAt')
       return this.trigger('flipped_pending_to_inflight')
       return this.trigger('flipped_pending_to_inflight')
     })
     })
     this._doc.on('saved', () => {
     this._doc.on('saved', () => {
@@ -280,7 +293,7 @@ export class ShareJsDoc extends EventEmitter {
     this.connection.id = this.socket.publicId
     this.connection.id = this.socket.publicId
     this._doc.autoOpen = false
     this._doc.autoOpen = false
     this._doc._connectionStateChanged(state)
     this._doc._connectionStateChanged(state)
-    return (this.lastAcked = null) // reset the last ack time when connection changes
+    this.lastAcked = null // reset the last ack time when connection changes
   }
   }
 
 
   hasBufferedOps() {
   hasBufferedOps() {
@@ -299,10 +312,18 @@ export class ShareJsDoc extends EventEmitter {
     // check if we have received an ack recently (within a factor of two of the single user flush delay)
     // check if we have received an ack recently (within a factor of two of the single user flush delay)
     return (
     return (
       this.lastAcked !== null &&
       this.lastAcked !== null &&
-      Date.now() - this.lastAcked.getTime() < 2 * SINGLE_USER_FLUSH_DELAY
+      performance.now() - this.lastAcked < RECENT_ACK_LIMIT
     )
     )
   }
   }
 
 
+  getInflightOpCreatedAt() {
+    return this.inflightOpCreatedAt
+  }
+
+  getPendingOpCreatedAt() {
+    return this.pendingOpCreatedAt
+  }
+
   private attachEditorWatchdogManager(editor: EditorFacade) {
   private attachEditorWatchdogManager(editor: EditorFacade) {
     // end-to-end check for edits -> acks, for this very ShareJsdoc
     // end-to-end check for edits -> acks, for this very ShareJsdoc
     // This will catch a broken connection and missing UX-blocker for the
     // This will catch a broken connection and missing UX-blocker for the

+ 20 - 0
services/web/frontend/stories/modals/unsaved-docs-locked-modal.stories.tsx

@@ -0,0 +1,20 @@
+import { Meta, StoryObj } from '@storybook/react'
+import { UnsavedDocsLockedAlert } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert'
+import { ScopeDecorator } from '../decorators/scope'
+import { bsVersionDecorator } from '../../../.storybook/utils/with-bootstrap-switcher'
+
+export default {
+  title: 'Editor / Modals / Unsaved Docs Locked',
+  component: UnsavedDocsLockedAlert,
+  decorators: [Story => ScopeDecorator(Story)],
+  argTypes: {
+    ...bsVersionDecorator.argTypes,
+  },
+  parameters: {
+    bootstrap5: true,
+  },
+} satisfies Meta
+
+type Story = StoryObj<typeof UnsavedDocsLockedAlert>
+
+export const Locked: Story = {}

+ 3 - 2
services/web/locales/en.json

@@ -395,7 +395,7 @@
   "connect_overleaf_with_github": "Connect __appName__ with Github for easy project syncing and real-time version control.",
   "connect_overleaf_with_github": "Connect __appName__ with Github for easy project syncing and real-time version control.",
   "connected_users": "Connected Users",
   "connected_users": "Connected Users",
   "connecting": "Connecting",
   "connecting": "Connecting",
-  "connection_lost": "Connection lost",
+  "connection_lost_with_unsaved_changes": "Connection lost with unsaved changes.",
   "contact": "Contact",
   "contact": "Contact",
   "contact_group_admin": "Please contact your group administrator.",
   "contact_group_admin": "Please contact your group administrator.",
   "contact_message_label": "Message",
   "contact_message_label": "Message",
@@ -541,6 +541,7 @@
   "done": "Done",
   "done": "Done",
   "dont_forget_you_currently_have": "Don’t forget, you currently have:",
   "dont_forget_you_currently_have": "Don’t forget, you currently have:",
   "dont_have_account": "Don’t have an account?",
   "dont_have_account": "Don’t have an account?",
+  "dont_reload_or_close_this_tab": "Don’t reload or close this tab.",
   "download": "Download",
   "download": "Download",
   "download_all": "Download all",
   "download_all": "Download all",
   "download_metadata": "Download Overleaf metadata",
   "download_metadata": "Download Overleaf metadata",
@@ -1997,7 +1998,6 @@
   "sorry_detected_sales_restricted_region": "Sorry, we’ve detected that you are in a region from which we cannot presently accept payments. If you think you’ve received this message in error, please <a href=\"__link__\">contact us</a> with details of your location, and we will look into this for you. We apologize for the inconvenience.",
   "sorry_detected_sales_restricted_region": "Sorry, we’ve detected that you are in a region from which we cannot presently accept payments. If you think you’ve received this message in error, please <a href=\"__link__\">contact us</a> with details of your location, and we will look into this for you. We apologize for the inconvenience.",
   "sorry_it_looks_like_that_didnt_work_this_time": "Sorry! It looks like that didn’t work this time. Please try again.",
   "sorry_it_looks_like_that_didnt_work_this_time": "Sorry! It looks like that didn’t work this time. Please try again.",
   "sorry_something_went_wrong_opening_the_document_please_try_again": "Sorry, an unexpected error occurred when trying to open this content on Overleaf. Please try again.",
   "sorry_something_went_wrong_opening_the_document_please_try_again": "Sorry, an unexpected error occurred when trying to open this content on Overleaf. Please try again.",
-  "sorry_the_connection_to_the_server_is_down": "Sorry, the connection to the server is down.",
   "sorry_there_are_no_experiments": "Sorry, there are no experiments currently running in Overleaf Labs.",
   "sorry_there_are_no_experiments": "Sorry, there are no experiments currently running in Overleaf Labs.",
   "sorry_this_account_has_been_suspended": "Sorry, this account has been suspended.",
   "sorry_this_account_has_been_suspended": "Sorry, this account has been suspended.",
   "sorry_your_table_cant_be_displayed_at_the_moment": "Sorry, your table can’t be displayed at the moment.",
   "sorry_your_table_cant_be_displayed_at_the_moment": "Sorry, your table can’t be displayed at the moment.",
@@ -2599,6 +2599,7 @@
   "your_add_on_has_been_cancelled_and_will_remain_active_until_your_billing_cycle_ends_on": "Your add-on has been cancelled and will remain active until your billing cycle ends on __nextBillingDate__",
   "your_add_on_has_been_cancelled_and_will_remain_active_until_your_billing_cycle_ends_on": "Your add-on has been cancelled and will remain active until your billing cycle ends on __nextBillingDate__",
   "your_affiliation_is_confirmed": "Your <0>__institutionName__</0> affiliation is confirmed.",
   "your_affiliation_is_confirmed": "Your <0>__institutionName__</0> affiliation is confirmed.",
   "your_browser_does_not_support_this_feature": "Sorry, your browser doesn’t support this feature. Please update your browser to its latest version.",
   "your_browser_does_not_support_this_feature": "Sorry, your browser doesn’t support this feature. Please update your browser to its latest version.",
+  "your_changes_will_save": "Your changes will save when we get the connection back.",
   "your_compile_timed_out": "Your compile timed out",
   "your_compile_timed_out": "Your compile timed out",
   "your_current_plan": "Your current plan",
   "your_current_plan": "Your current plan",
   "your_current_plan_gives_you": "By pausing your subscription, you’ll be able to access your premium features faster when you need them again.",
   "your_current_plan_gives_you": "By pausing your subscription, you’ll be able to access your premium features faster when you need them again.",