From 4da838b6963d96923b2ad6f01850b8d3a107733e Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 7 Apr 2025 10:38:03 +0200 Subject: [PATCH 1/2] enh(sync): add useDelayedFlag Allow changing a flag with different delays: * Switch it on slow. * Switch it off fast. Useful for errors - so they only show after a while but are resolved quickly. Example: const input = ref(false) const { delayed } = useDelayedFlag(input) ... input.value = true // 5 seconds later delayed.value will be true ... input.value = false // 200 ms later delayed.value will be true Signed-off-by: Max --- src/components/Editor/useDelayedFlag.spec.ts | 58 ++++++++++++++++++++ src/components/Editor/useDelayedFlag.ts | 29 ++++++++++ 2 files changed, 87 insertions(+) create mode 100644 src/components/Editor/useDelayedFlag.spec.ts create mode 100644 src/components/Editor/useDelayedFlag.ts diff --git a/src/components/Editor/useDelayedFlag.spec.ts b/src/components/Editor/useDelayedFlag.spec.ts new file mode 100644 index 00000000000..e76a2194717 --- /dev/null +++ b/src/components/Editor/useDelayedFlag.spec.ts @@ -0,0 +1,58 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { afterEach, expect, test, vi } from 'vitest' +import { useDelayedFlag } from './useDelayedFlag' +import { nextTick, ref, watch } from 'vue' + +afterEach(() => { + vi.useRealTimers() +}) + +test('useDelayedFlag defaults to provided ref value', () => { + [true, false].forEach(val => { + const { delayed } = useDelayedFlag(ref(val)) + expect(delayed.value).toBe(val) + }) +}) + +test('switches slowly to true', async () => { + vi.useFakeTimers() + const input = ref(false) + const { delayed } = useDelayedFlag(input) + input.value = true + await nextTick() + vi.advanceTimersByTime(3000) + expect(delayed.value).toBe(false) + vi.advanceTimersByTime(5000) + expect(delayed.value).toBe(true) +}) + +test('switches fast to false', async () => { + vi.useFakeTimers() + const input = ref(true) + const { delayed } = useDelayedFlag(input) + input.value = false + await nextTick() + expect(delayed.value).toBe(true) + vi.advanceTimersByTime(300) + expect(delayed.value).toBe(false) +}) + +test('does not flip flop', async () => { + vi.useFakeTimers() + const input = ref(false) + const { delayed } = useDelayedFlag(input) + const probe = vi.fn() + watch(delayed, probe) + input.value = true + await nextTick() + vi.advanceTimersByTime(1000) + input.value = false + await nextTick() + vi.advanceTimersByTime(5000) + expect(delayed.value).toBe(false) + expect(probe).not.toBeCalled() +}) diff --git a/src/components/Editor/useDelayedFlag.ts b/src/components/Editor/useDelayedFlag.ts new file mode 100644 index 00000000000..eec96ccd1f2 --- /dev/null +++ b/src/components/Editor/useDelayedFlag.ts @@ -0,0 +1,29 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { ref, watch, type Ref } from 'vue' + +/** + * Delay the changing of the boolean + * @param input - ref to react to + */ +export function useDelayedFlag(input: Ref): { delayed: Ref } { + + let timeout: ReturnType | undefined + const delayed = ref(input.value) + + watch(input, (val) => { + if (timeout) { + clearTimeout(timeout) + } + const delay = val ? 5000 : 200 + timeout = setTimeout(() => { + delayed.value = val + }, delay) + + }) + + return { delayed } +} From a13358442b666a4333ff0a9284c1c61920ca641d Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 10 Apr 2025 09:55:37 +0200 Subject: [PATCH 2/2] fix(sync): allow writing during short connection failures Delay the error message and allow editing for 5 seconds. Also speed up the recovery by triggering push after a successful sync. Signed-off-by: Max --- cypress/e2e/sync.spec.js | 12 +++++++++-- src/components/Editor.vue | 44 +++++++++++++++++++++++---------------- 2 files changed, 36 insertions(+), 20 deletions(-) diff --git a/cypress/e2e/sync.spec.js b/cypress/e2e/sync.spec.js index c4a6a6138ff..fce1e3edbb2 100644 --- a/cypress/e2e/sync.spec.js +++ b/cypress/e2e/sync.spec.js @@ -69,6 +69,16 @@ describe('Sync', () => { .should('include', 'after the lost connection') }) + it('handles brief network outages', () => { + cy.intercept('**/apps/text/session/*/*', req => req.destroy()).as('dead') + cy.wait('@dead', { timeout: 30000 }) + // bring back the network connection + cy.intercept('**/apps/text/session/*/*', req => { req.continue() }).as('alive') + cy.wait('@alive', { timeout: 30000 }) + cy.getContent().type('staying alive') + cy.getContent().should('contain', 'staying alive') + }) + it('reconnects via button after a short lost connection', () => { cy.intercept('**/apps/text/session/*/*', req => req.destroy()).as('dead') cy.wait('@dead', { timeout: 30000 }) @@ -76,8 +86,6 @@ describe('Sync', () => { .should('contain', 'The document could not be loaded.') cy.get('#editor-container .document-status') .find('.button.primary').click() - cy.get('.toastify').should('contain', 'Connection failed.') - cy.get('.toastify', { timeout: 30000 }).should('not.exist') cy.get('#editor-container .document-status', { timeout: 30000 }) .should('contain', 'The document could not be loaded.') // bring back the network connection diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 2cb49d5aa1d..79a67bd838f 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -14,7 +14,7 @@ + :has-connection-issue="requireReconnect" /> @@ -43,7 +43,7 @@ :dirty="dirty" :sessions="filteredSessions" :sync-error="syncError" - :has-connection-issue="hasConnectionIssue" + :has-connection-issue="requireReconnect" @editor-width-change="handleEditorWidthChange" /> @@ -58,7 +58,7 @@ @@ -125,6 +125,7 @@ import Translate from './Modal/Translate.vue' import CollisionResolveDialog from './CollisionResolveDialog.vue' import { generateRemoteUrl } from '@nextcloud/router' import { fetchNode } from '../services/WebdavClient.ts' +import { useDelayedFlag } from './Editor/useDelayedFlag.ts' export default { name: 'Editor', @@ -241,7 +242,9 @@ export default { const maxWidth = Math.floor(value) - 36 el.value.style.setProperty('--widget-full-width', `${maxWidth}px`) }) - return { el, width } + const hasConnectionIssue = ref(false) + const { delayed: requireReconnect } = useDelayedFlag(hasConnectionIssue) + return { el, width, hasConnectionIssue, requireReconnect } }, data() { @@ -259,7 +262,6 @@ export default { dirty: false, contentLoaded: false, syncError: null, - hasConnectionIssue: false, hasEditor: false, readOnly: true, openReadOnlyEnabled: OCA.Text.OpenReadOnlyEnabled, @@ -351,6 +353,14 @@ export default { window.removeEventListener('beforeunload', this.saveBeforeUnload) } }, + requireReconnect(val) { + if (val) { + this.emit('sync-service:error') + } + if (this.$editor?.isEditable === val) { + this.$editor.setEditable(!val) + } + }, }, mounted() { if (this.active && (this.hasDocumentParameters)) { @@ -596,7 +606,7 @@ export default { this.document = document this.syncError = null - const editable = this.editMode && !this.hasConnectionIssue + const editable = this.editMode && !this.requireReconnect if (this.$editor.isEditable !== editable) { this.$editor.setEditable(editable) } @@ -619,7 +629,13 @@ export default { }, onSync({ steps, document }) { - this.hasConnectionIssue = this.$syncService.backend.fetcher === 0 || !this.$providers[0].wsconnected || this.$syncService.pushError > 0 + this.hasConnectionIssue = this.$syncService.backend.fetcher === 0 + || !this.$providers[0].wsconnected + || this.$syncService.pushError > 0 + if (this.$syncService.pushError > 0) { + // successfully received steps - so let's try and also push + this.$syncService.sendStepsNow() + } this.$nextTick(() => { this.emit('sync-service:sync') }) @@ -629,11 +645,6 @@ export default { }, onError({ type, data }) { - this.$nextTick(() => { - this.$editor?.setEditable(false) - this.emit('sync-service:error') - }) - if (type === ERROR_TYPE.LOAD_ERROR) { this.syncError = { type, @@ -648,11 +659,8 @@ export default { data, } } - if (type === ERROR_TYPE.CONNECTION_FAILED && !this.hasConnectionIssue) { - this.hasConnectionIssue = true - OC.Notification.showTemporary(t('text', 'Connection failed.')) - } - if (type === ERROR_TYPE.SOURCE_NOT_FOUND) { + if (type === ERROR_TYPE.CONNECTION_FAILED + || type === ERROR_TYPE.SOURCE_NOT_FOUND) { this.hasConnectionIssue = true }