From 432bbe0db88df92d4b6087f5cd6ffc3d1ed71832 Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Wed, 19 Feb 2025 16:46:40 +0000 Subject: [PATCH] Merge pull request #23714 from overleaf/ae-revert-unsaved-docs Revert "Increase the length of unsaved doc time before the editor is locked" GitOrigin-RevId: 0f2d257a55ebc3353ded2b10d505bfdb2e85f402 --- .../web/frontend/extracted-translations.json | 5 ++-- .../unsaved-docs/unsaved-docs-alert.tsx | 4 +-- .../unsaved-docs-locked-alert.tsx | 19 ------------ .../unsaved-docs-locked-modal.tsx | 28 +++++++++++++++++ .../components/unsaved-docs/unsaved-docs.tsx | 30 ++++++++----------- .../ide-react/editor/document-container.ts | 6 +--- .../ide-react/editor/open-documents.ts | 10 +++---- .../features/ide-react/editor/share-js-doc.ts | 27 ++++------------- .../unsaved-docs-locked-modal.stories.tsx | 20 ------------- services/web/locales/en.json | 5 ++-- 10 files changed, 56 insertions(+), 98 deletions(-) delete mode 100644 services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert.tsx create mode 100644 services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal.tsx delete mode 100644 services/web/frontend/stories/modals/unsaved-docs-locked-modal.stories.tsx diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json index 34d8431612..cb2373dcec 100644 --- a/services/web/frontend/extracted-translations.json +++ b/services/web/frontend/extracted-translations.json @@ -303,7 +303,7 @@ "congratulations_youve_successfully_join_group": "", "connect_overleaf_with_github": "", "connected_users": "", - "connection_lost_with_unsaved_changes": "", + "connection_lost": "", "contact_group_admin": "", "contact_sales": "", "contact_support_to_change_group_subscription": "", @@ -416,7 +416,6 @@ "doing_this_will_verify_affiliation_and_allow_log_in_2": "", "done": "", "dont_forget_you_currently_have": "", - "dont_reload_or_close_this_tab": "", "download": "", "download_all": "", "download_metadata": "", @@ -1524,6 +1523,7 @@ "something_went_wrong_server": "", "somthing_went_wrong_compiling": "", "sorry_it_looks_like_that_didnt_work_this_time": "", + "sorry_the_connection_to_the_server_is_down": "", "sorry_there_are_no_experiments": "", "sorry_your_table_cant_be_displayed_at_the_moment": "", "sort_by": "", @@ -2044,7 +2044,6 @@ "your_add_on_has_been_cancelled_and_will_remain_active_until_your_billing_cycle_ends_on": "", "your_affiliation_is_confirmed": "", "your_browser_does_not_support_this_feature": "", - "your_changes_will_save": "", "your_compile_timed_out": "", "your_current_plan": "", "your_current_plan_gives_you": "", diff --git a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-alert.tsx b/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-alert.tsx index 782cc44635..18398710ab 100644 --- a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-alert.tsx +++ b/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-alert.tsx @@ -3,15 +3,13 @@ import { useFileTreePathContext } from '@/features/file-tree/contexts/file-tree- import { useTranslation } from 'react-i18next' import OLNotification from '@/features/ui/components/ol/ol-notification' -const MAX_UNSAVED_ALERT_SECONDS = 15 - export const UnsavedDocsAlert: FC<{ unsavedDocs: Map }> = ({ unsavedDocs, }) => ( <> {[...unsavedDocs.entries()].map( ([docId, seconds]) => - seconds >= MAX_UNSAVED_ALERT_SECONDS && ( + seconds > 8 && ( ) )} diff --git a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert.tsx b/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert.tsx deleted file mode 100644 index 3630cc5dab..0000000000 --- a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert.tsx +++ /dev/null @@ -1,19 +0,0 @@ -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 ( - - {t('connection_lost_with_unsaved_changes')}{' '} - {t('dont_reload_or_close_this_tab')} {t('your_changes_will_save')} - - } - /> - ) -} diff --git a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal.tsx b/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal.tsx new file mode 100644 index 0000000000..138852e6d9 --- /dev/null +++ b/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal.tsx @@ -0,0 +1,28 @@ +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 ( + {}} // It's not possible to hide this modal, but it's a required prop + className="lock-editor-modal" + backdrop={false} + keyboard={false} + > + + {t('connection_lost')} + + + {t('sorry_the_connection_to_the_server_is_down')} + + + ) +} diff --git a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs.tsx b/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs.tsx index 6942ee8c8c..4aa63cd3b5 100644 --- a/services/web/frontend/js/features/ide-react/components/unsaved-docs/unsaved-docs.tsx +++ b/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 { FC, useCallback, useEffect, useRef, useState } from 'react' import { PermissionsLevel } from '@/features/ide-react/types/permissions' -import { UnsavedDocsLockedAlert } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-locked-alert' +import { UnsavedDocsLockedModal } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-locked-modal' import { UnsavedDocsAlert } from '@/features/ide-react/components/unsaved-docs/unsaved-docs-alert' import useEventListener from '@/shared/hooks/use-event-listener' import { createPortal } from 'react-dom' import { useGlobalAlertsContainer } from '@/features/ide-react/context/global-alerts-context' -const MAX_UNSAVED_SECONDS = 30 // lock the editor after this time if unsaved +const MAX_UNSAVED_SECONDS = 15 // lock the editor after this time if unsaved export const UnsavedDocs: FC = () => { const { openDocs, debugTimers } = useEditorManagerContext() @@ -19,6 +19,9 @@ export const UnsavedDocs: FC = () => { // always contains the latest value const previousUnsavedDocsRef = useRef(unsavedDocs) + useEffect(() => { + previousUnsavedDocsRef.current = unsavedDocs + }, [unsavedDocs]) // always contains the latest value const permissionsLevelRef = useRef(permissionsLevel) @@ -47,16 +50,12 @@ export const UnsavedDocs: FC = () => { debugTimers.current.CheckUnsavedDocs = Date.now() const unsavedDocs = new Map() - const docs = openDocs.unsavedDocs() + const unsavedDocIds = openDocs.unsavedDocIds() - for (const doc of docs) { - const inflightOpCreatedAt = doc.getInflightOpCreatedAt() - if (inflightOpCreatedAt) { - const unsavedSeconds = Math.floor( - (performance.now() - inflightOpCreatedAt) / 1000 - ) - unsavedDocs.set(doc.doc_id, unsavedSeconds) - } + for (const docId of unsavedDocIds) { + const unsavedSeconds = + (previousUnsavedDocsRef.current.get(docId) ?? 0) + 1 + unsavedDocs.set(docId, unsavedSeconds) } // avoid setting the unsavedDocs state to a new empty Map every second @@ -101,16 +100,11 @@ export const UnsavedDocs: FC = () => { } }, [unsavedDocs]) - if (!globalAlertsContainer) { - return null - } - return ( <> - {isLocked && - createPortal(, globalAlertsContainer)} - + {isLocked && } {unsavedDocs.size > 0 && + globalAlertsContainer && createPortal( , globalAlertsContainer diff --git a/services/web/frontend/js/features/ide-react/editor/document-container.ts b/services/web/frontend/js/features/ide-react/editor/document-container.ts index 118cbecf2b..af2b7af73e 100644 --- a/services/web/frontend/js/features/ide-react/editor/document-container.ts +++ b/services/web/frontend/js/features/ide-react/editor/document-container.ts @@ -182,10 +182,6 @@ export class DocumentContainer extends EventEmitter { return this.doc?.getRecentAck() } - getInflightOpCreatedAt() { - return this.doc?.getInflightOpCreatedAt() - } - hasBufferedOps() { return this.doc?.hasBufferedOps() } @@ -358,7 +354,7 @@ export class DocumentContainer extends EventEmitter { pendingOpSize < MAX_PENDING_OP_SIZE ) { // There is an op waiting to go to server but it is small and - // within the recent ack limit, this is OK for now. + // within the flushDelay, this is OK for now. saved = true debugConsole.log( '[pollSavedStatus] pending op (small with recent ack) assume ok', diff --git a/services/web/frontend/js/features/ide-react/editor/open-documents.ts b/services/web/frontend/js/features/ide-react/editor/open-documents.ts index 66250eaebe..39d42b1c60 100644 --- a/services/web/frontend/js/features/ide-react/editor/open-documents.ts +++ b/services/web/frontend/js/features/ide-react/editor/open-documents.ts @@ -83,14 +83,14 @@ export class OpenDocuments { } } - unsavedDocs() { - const docs = [] - for (const doc of this.openDocs.values()) { + unsavedDocIds() { + const ids = [] + for (const [docId, doc] of this.openDocs) { if (!doc.pollSavedStatus()) { - docs.push(doc) + ids.push(docId) } } - return docs + return ids } async awaitBufferedOps(signal: AbortSignal) { diff --git a/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts b/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts index 14276a6176..f9a4109b71 100644 --- a/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts +++ b/services/web/frontend/js/features/ide-react/editor/share-js-doc.ts @@ -23,8 +23,7 @@ const SINGLE_USER_FLUSH_DELAY = 2000 const MULTI_USER_FLUSH_DELAY = 500 const INFLIGHT_OP_TIMEOUT = 5000 // Retry sending ops after 5 seconds without an ack const WAIT_FOR_CONNECTION_TIMEOUT = 500 -const FATAL_OP_TIMEOUT = 45000 -const RECENT_ACK_LIMIT = 2 * SINGLE_USER_FLUSH_DELAY +const FATAL_OP_TIMEOUT = 30000 type Update = Record @@ -43,9 +42,7 @@ export class ShareJsDoc extends EventEmitter { // @ts-ignore _doc: Doc private editorWatchdogManager: EditorWatchdogManager - private lastAcked: number | null = null - private pendingOpCreatedAt: number | null = null - private inflightOpCreatedAt: number | null = null + private lastAcked: Date | null = null private queuedMessageTimer: number | null = null private queuedMessages: Message[] = [] private detachEditorWatchdogManager: (() => void) | null = null @@ -93,19 +90,13 @@ export class ShareJsDoc extends EventEmitter { }) this._doc.setFlushDelay(SINGLE_USER_FLUSH_DELAY) this._doc.on('change', (...args: any[]) => { - if (!this.pendingOpCreatedAt) { - debugConsole.log('set pendingOpCreatedAt', new Date()) - this.pendingOpCreatedAt = performance.now() - } return this.trigger('change', ...args) }) this.editorWatchdogManager = new EditorWatchdogManager({ parent: globalEditorWatchdogManager, }) this._doc.on('acknowledge', () => { - this.lastAcked = performance.now() // note time of last ack from server for an op we sent - this.inflightOpCreatedAt = null - debugConsole.log('unset inflightOpCreatedAt') + this.lastAcked = new Date() // note time of last ack from server for an op we sent this.editorWatchdogManager.onAck() // keep track of last ack globally return this.trigger('acknowledge') }) @@ -116,10 +107,6 @@ export class ShareJsDoc extends EventEmitter { return this.trigger('remoteop', ...args) }) 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') }) this._doc.on('saved', () => { @@ -293,7 +280,7 @@ export class ShareJsDoc extends EventEmitter { this.connection.id = this.socket.publicId this._doc.autoOpen = false this._doc._connectionStateChanged(state) - this.lastAcked = null // reset the last ack time when connection changes + return (this.lastAcked = null) // reset the last ack time when connection changes } hasBufferedOps() { @@ -312,14 +299,10 @@ export class ShareJsDoc extends EventEmitter { // check if we have received an ack recently (within a factor of two of the single user flush delay) return ( this.lastAcked !== null && - performance.now() - this.lastAcked < RECENT_ACK_LIMIT + Date.now() - this.lastAcked.getTime() < 2 * SINGLE_USER_FLUSH_DELAY ) } - getInflightOpCreatedAt() { - return this.inflightOpCreatedAt - } - private attachEditorWatchdogManager(editor: EditorFacade) { // end-to-end check for edits -> acks, for this very ShareJsdoc // This will catch a broken connection and missing UX-blocker for the diff --git a/services/web/frontend/stories/modals/unsaved-docs-locked-modal.stories.tsx b/services/web/frontend/stories/modals/unsaved-docs-locked-modal.stories.tsx deleted file mode 100644 index 44092f2218..0000000000 --- a/services/web/frontend/stories/modals/unsaved-docs-locked-modal.stories.tsx +++ /dev/null @@ -1,20 +0,0 @@ -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 - -export const Locked: Story = {} diff --git a/services/web/locales/en.json b/services/web/locales/en.json index 5b730599d9..3608bb805d 100644 --- a/services/web/locales/en.json +++ b/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.", "connected_users": "Connected Users", "connecting": "Connecting", - "connection_lost_with_unsaved_changes": "Connection lost with unsaved changes.", + "connection_lost": "Connection lost", "contact": "Contact", "contact_group_admin": "Please contact your group administrator.", "contact_message_label": "Message", @@ -541,7 +541,6 @@ "done": "Done", "dont_forget_you_currently_have": "Don’t forget, you currently have:", "dont_have_account": "Don’t have an account?", - "dont_reload_or_close_this_tab": "Don’t reload or close this tab.", "download": "Download", "download_all": "Download all", "download_metadata": "Download Overleaf metadata", @@ -1998,6 +1997,7 @@ "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 contact us 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_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_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.", @@ -2599,7 +2599,6 @@ "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__ 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_changes_will_save": "Your changes will save when we get the connection back.", "your_compile_timed_out": "Your compile timed out", "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.",