fix(viewer): no overlapping saves, Ctrl+S saves wherever the focus is

src/__tests__/mindmapElement.spec.js

@@ -82,6 +82,43 @@ describe('MindMap element', () => {
 		await vi.waitFor(() => expect(loaded).toHaveBeenCalledOnce())
 		expect(loaded).toHaveReturned()
 	})
+
+	it('saves the mind map on Ctrl+S instead of the browser saving the page', async () => {
+		const element = await mount()
+		const save = vi.fn()
+		element.querySelector('iframe').contentWindow.MindMap = { save }
+
+		const event = new KeyboardEvent('keydown', { key: 's', ctrlKey: true, cancelable: true })
+		document.dispatchEvent(event)
+
+		expect(save).toHaveBeenCalledOnce()
+		expect(event.defaultPrevented).toBe(true)
+	})
+
+	it('leaves other keys alone', async () => {
+		const element = await mount()
+		const save = vi.fn()
+		element.querySelector('iframe').contentWindow.MindMap = { save }
+
+		const event = new KeyboardEvent('keydown', { key: 's', cancelable: true })
+		document.dispatchEvent(event)
+
+		expect(save).not.toHaveBeenCalled()
+		expect(event.defaultPrevented).toBe(false)
+	})
+
+	it('stops handling Ctrl+S once the viewer removes it', async () => {
+		const element = await mount()
+		const save = vi.fn()
+		element.querySelector('iframe').contentWindow.MindMap = { save }
+		element.remove()
+
+		const event = new KeyboardEvent('keydown', { key: 's', ctrlKey: true, cancelable: true })
+		document.dispatchEvent(event)
+
+		expect(save).not.toHaveBeenCalled()
+		expect(event.defaultPrevented).toBe(false)
+	})
 })
 
 describe('MIMES', () => {

src/viewer.js

@@ -110,6 +110,8 @@ normalizeMouseWheel();
 
 	const MindMap = {
 		_changed: false,
+		_saving: false,
+		_saveQueue: [],
 		_autoSaveTimer: null,
 		_clearStatusMessageTimer: null,
 		_loadStatus: false,
@@ -152,13 +154,14 @@ normalizeMouseWheel();
 		},
 		initHotkey() {
 			const self = this
-			window.$(document).keydown(function(e) {
-				if ((e.ctrlKey || e.metaKey) && e.which === 83) {
-					self.save()
+			// Capture phase, so the editor's own key handlers cannot swallow it
+			document.addEventListener('keydown', function(e) {
+				if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 's') {
 					e.preventDefault()
-					return false
+					e.stopPropagation()
+					self.save()
 				}
-			})
+			}, true)
 		},
 		bindEvent() {
 			const self = this
@@ -224,25 +227,55 @@ normalizeMouseWheel();
 		},
 		save(onComplete) {
 			const self = this
-			if (self._changed) {
-				self.updateSaveButtonInfo(t('files_mindmap', 'Saving…'))
-				const data = JSON.stringify(minder.exportJson())
-				window.parent.OCA.FilesMindMap.save(data, function(msg) {
-					self.updateSaveButtonInfo(msg)
-					self._changed = false
-					self.restoreSaveButtonInfo(3000)
-					if (undefined !== onComplete) {
-						onComplete(true, msg)
-					}
-				}, function(msg) {
-					self.updateSaveButtonInfo(msg)
-					self.restoreSaveButtonInfo(3000)
-					if (undefined !== onComplete) {
-						onComplete(false, msg)
-					}
-				})
-				self.restoreSaveButtonInfo(6000)
+			// A second request before the first one returns would fail the server's mtime check
+			if (self._saving) {
+				self._saveQueue.push(onComplete)
+				return
+			}
+			if (!self._changed) {
+				if (undefined !== onComplete) {
+					onComplete(true)
+				}
+				return
+			}
+			const finish = function(status, msg) {
+				self._saving = false
+				if (undefined !== onComplete) {
+					onComplete(status, msg)
+				}
+				const queued = self._saveQueue.splice(0)
+				const notify = function(queuedStatus, queuedMsg) {
+					queued.forEach(function(callback) {
+						if (undefined !== callback) {
+							callback(queuedStatus, queuedMsg)
+						}
+					})
+				}
+				if (queued.length === 0) {
+					return
+				}
+				if (status) {
+					self.save(notify)
+				} else {
+					notify(false, msg)
+				}
 			}
+			self._saving = true
+			// Edits made while the request is out mark the map as changed again
+			self._changed = false
+			self.updateSaveButtonInfo(t('files_mindmap', 'Saving…'))
+			const data = JSON.stringify(minder.exportJson())
+			window.parent.OCA.FilesMindMap.save(data, function(msg) {
+				self.updateSaveButtonInfo(msg)
+				self.restoreSaveButtonInfo(3000)
+				finish(true, msg)
+			}, function(msg) {
+				self._changed = true
+				self.updateSaveButtonInfo(msg)
+				self.restoreSaveButtonInfo(3000)
+				finish(false, msg)
+			})
+			self.restoreSaveButtonInfo(6000)
 		},
 		startAutoSaveTimer() {
 			const self = this

src/views/MindMap.js

@@ -38,7 +38,21 @@ export default class MindMap extends HTMLElement {
 		this._file = file
 	}
 
+	/**
+	 * Save the mind map on Ctrl+S while the focus is outside the editor iframe,
+	 * instead of the browser saving the page.
+	 *
+	 * @param {KeyboardEvent} event the key event
+	 */
+	onKeyDown = (event) => {
+		if ((event.ctrlKey || event.metaKey) && event.key.toLowerCase() === 's') {
+			event.preventDefault()
+			this.querySelector('iframe')?.contentWindow?.MindMap?.save()
+		}
+	}
+
 	async connectedCallback() {
+		document.addEventListener('keydown', this.onKeyDown)
 		try {
 			const FilesMindMap = await loadFilesMindMap()
 			FilesMindMap.setFile(this.file)
@@ -59,6 +73,10 @@ export default class MindMap extends HTMLElement {
 		this.dispatchEvent(new CustomEvent('loaded'))
 	}
 
+	disconnectedCallback() {
+		document.removeEventListener('keydown', this.onKeyDown)
+	}
+
 	attributeChangedCallback() {
 		this.resize()
 	}