From 991c12025b67ac6a8a7e85b7c22bf9764237367e Mon Sep 17 00:00:00 2001
From: Laurent Cozic <laurent22@users.noreply.github.com>
Date: Wed, 3 May 2023 11:45:19 +0100
Subject: [PATCH] Mobile: Fixes issue where the note body is not updated after
 attaching a file

---
 .../NoteBodyViewer/hooks/useSource.ts         | 48 +++++++++++++++----
 1 file changed, 39 insertions(+), 9 deletions(-)

diff --git a/packages/app-mobile/components/NoteBodyViewer/hooks/useSource.ts b/packages/app-mobile/components/NoteBodyViewer/hooks/useSource.ts
index b1c4975951..0003fda63a 100644
--- a/packages/app-mobile/components/NoteBodyViewer/hooks/useSource.ts
+++ b/packages/app-mobile/components/NoteBodyViewer/hooks/useSource.ts
@@ -3,8 +3,11 @@ import shim from '@joplin/lib/shim';
 import Setting from '@joplin/lib/models/Setting';
 const { themeStyle } = require('../../global-style.js');
 import markupLanguageUtils from '@joplin/lib/markupLanguageUtils';
+import Logger from '@joplin/lib/Logger';
 const { assetsToHeaders } = require('@joplin/renderer');
 
+const logger = Logger.create('NoteBodyViewer/useSource');
+
 interface UseSourceResult {
 	// [html] can be null if the note is still being rendered.
 	html: string|null;
@@ -19,6 +22,23 @@ function usePrevious(value: any, initialValue: any = null): any {
 	return ref.current;
 }
 
+const onlyCheckboxHasChangedHack = (previousBody: string, newBody: string) => {
+	if (previousBody.length !== newBody.length) return false;
+
+	for (let i = 0; i < previousBody.length; i++) {
+		const c1 = previousBody.charAt(i);
+		const c2 = newBody.charAt(i);
+
+		if (c1 !== c2) {
+			if (c1 === ' ' && (c2 === 'x' || c2 === 'X')) continue;
+			if (c2 === ' ' && (c1 === 'x' || c1 === 'X')) continue;
+			return false;
+		}
+	}
+
+	return true;
+};
+
 export default function useSource(noteBody: string, noteMarkupLanguage: number, themeId: number, highlightedKeywords: string[], noteResources: any, paddingBottom: number, noteHash: string): UseSourceResult {
 	const [html, setHtml] = useState<string>('');
 	const [injectedJs, setInjectedJs] = useState<string[]>([]);
@@ -42,14 +62,20 @@ export default function useSource(noteBody: string, noteMarkupLanguage: number,
 
 	// To address https://github.com/laurent22/joplin/issues/433
 	//
-	// If a checkbox in a note is ticked, the body changes, which normally
-	// would trigger a re-render of this component, which has the
-	// unfortunate side effect of making the view scroll back to the top.
-	// This re-rendering however is uncessary since the component is
-	// already visually updated via JS. So here, if the note has not
-	// changed, we prevent the component from updating. This fixes the
-	// above issue. A drawback of this is if the note is updated via sync,
-	// this change will not be displayed immediately.
+	// If a checkbox in a note is ticked, the body changes, which normally would
+	// trigger a re-render of this component, which has the unfortunate side
+	// effect of making the view scroll back to the top. This re-rendering
+	// however is uncessary since the component is already visually updated via
+	// JS. So here, if the note has not changed, we prevent the component from
+	// updating. This fixes the above issue. A drawback of this is if the note
+	// is updated via sync, this change will not be displayed immediately.
+	//
+	// 2022-05-03: However we sometimes need the HTML to be updated, even when
+	// only the body has changed - for example when attaching a resource, or
+	// when adding text via speech recognition. So the logic has been narrowed
+	// down so that updates are skipped only when checkbox has been changed.
+	// Checkboxes still work as expected, without making the note scroll, and
+	// other text added to the note is displayed correctly.
 	//
 	// IMPORTANT: KEEP noteBody AS THE FIRST dependency in the array as the
 	// below logic rely on this.
@@ -62,9 +88,13 @@ export default function useSource(noteBody: string, noteMarkupLanguage: number,
 		return accum;
 	}, {});
 	const onlyNoteBodyHasChanged = Object.keys(changedDeps).length === 1 && changedDeps[0];
+	const onlyCheckboxesHaveChanged = previousDeps[0] && changedDeps[0] && onlyCheckboxHasChangedHack(previousDeps[0], noteBody);
 
 	useEffect(() => {
-		if (onlyNoteBodyHasChanged) return () => {};
+		if (onlyNoteBodyHasChanged && onlyCheckboxesHaveChanged) {
+			logger.info('Only a checkbox has changed - not updating HTML');
+			return () => {};
+		}
 
 		let cancelled = false;