From a3b1597cd537433b8fd30dc71ca953c9f89dca31 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Fri, 15 Sep 2023 12:11:45 +0800 Subject: [PATCH 01/16] ignore scroll callback if triggered by text update --- .../ReportActionCompose/ComposerWithSuggestions.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 3b5b181d2fcb..696627a13947 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -125,6 +125,7 @@ function ComposerWithSuggestions({ const textInputRef = useRef(null); const insertedEmojisRef = useRef([]); + const shouldIgnoreScrollCallback = useRef(false); /** * Update frequently used emojis list. We debounce this method in the constructor so that UpdateFrequentlyUsedEmojis @@ -175,6 +176,7 @@ function ComposerWithSuggestions({ */ const updateComment = useCallback( (commentValue, shouldDebounceSaveComment) => { + shouldIgnoreScrollCallback.current = true; const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis(commentValue, preferredSkinTone, preferredLocale); if (!_.isEmpty(emojis)) { @@ -314,13 +316,16 @@ function ComposerWithSuggestions({ [suggestionsRef], ); - const updateShouldShowSuggestionMenuToFalse = useCallback(() => { - if (!suggestionsRef.current) { + const hideSuggestionMenu = useCallback(() => { + if (!suggestionsRef.current || shouldIgnoreScrollCallback.current) { + shouldIgnoreScrollCallback.current = false; return; } suggestionsRef.current.updateShouldShowSuggestionMenuToFalse(false); }, [suggestionsRef]); + const debouncedHideSuggestionMenu = useMemo(() => _.debounce(hideSuggestionMenu, 200, true), [hideSuggestionMenu]); + const setShouldBlockSuggestionCalcToFalse = useCallback(() => { if (!suggestionsRef.current) { return false; @@ -494,7 +499,7 @@ function ComposerWithSuggestions({ } setComposerHeight(composerLayoutHeight); }} - onScroll={updateShouldShowSuggestionMenuToFalse} + onScroll={debouncedHideSuggestionMenu} /> From d9ab01755e8a016cecc814fe18aa4dbdc9429508 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 14:04:24 +0800 Subject: [PATCH 02/16] fix suggestion menu hides on typing --- src/hooks/useDebounce.js | 16 +++++++++++++ .../ComposerWithSuggestions.js | 24 ++++++++++++------- 2 files changed, 32 insertions(+), 8 deletions(-) create mode 100644 src/hooks/useDebounce.js diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js new file mode 100644 index 000000000000..0ca06cdbb197 --- /dev/null +++ b/src/hooks/useDebounce.js @@ -0,0 +1,16 @@ +import {useEffect, useRef} from 'react'; +import _ from 'underscore'; + +export default function useDebounce(func, wait, immediate) { + const debouncedFnRef = useRef(); + + useEffect(() => { + const debouncedFn = _.debounce(func, wait, immediate); + + debouncedFnRef.current = debouncedFn; + + return debouncedFn.cancel; + }, [func, wait, immediate]); + + return debouncedFnRef.current; +} diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 38ee1ab1d513..c4bea207cb75 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -33,6 +33,7 @@ import compose from '../../../../libs/compose'; import withKeyboardState from '../../../../components/withKeyboardState'; import {propTypes, defaultProps} from './composerWithSuggestionsProps'; import focusWithDelay from '../../../../libs/focusWithDelay'; +import useDebounce from '../../../../hooks/useDebounce'; const {RNTextInputReset} = NativeModules; @@ -125,7 +126,7 @@ function ComposerWithSuggestions({ const textInputRef = useRef(null); const insertedEmojisRef = useRef([]); - const shouldIgnoreScrollCallback = useRef(false); + const isScrollLikelyLayoutTriggered = useRef(false); /** * Update frequently used emojis list. We debounce this method in the constructor so that UpdateFrequentlyUsedEmojis @@ -136,6 +137,16 @@ function ComposerWithSuggestions({ insertedEmojisRef.current = []; }, []); + const debouncedLowerIsScrollLikelyLayoutTriggered = useDebounce( + useCallback(() => (isScrollLikelyLayoutTriggered.current = false), []), + 500, + ); + + const raiseIsScrollLikelyLayoutTriggered = useCallback(() => { + isScrollLikelyLayoutTriggered.current = true; + debouncedLowerIsScrollLikelyLayoutTriggered(); + }, [debouncedLowerIsScrollLikelyLayoutTriggered]); + const onInsertedEmoji = useCallback( (emojiObject) => { insertedEmojisRef.current = [...insertedEmojisRef.current, emojiObject]; @@ -176,7 +187,7 @@ function ComposerWithSuggestions({ */ const updateComment = useCallback( (commentValue, shouldDebounceSaveComment) => { - shouldIgnoreScrollCallback.current = true; + raiseIsScrollLikelyLayoutTriggered(); const {text: newComment, emojis} = EmojiUtils.replaceAndExtractEmojis(commentValue, preferredSkinTone, preferredLocale); if (!_.isEmpty(emojis)) { @@ -214,7 +225,7 @@ function ComposerWithSuggestions({ debouncedBroadcastUserIsTyping(reportID); } }, - [debouncedUpdateFrequentlyUsedEmojis, preferredLocale, preferredSkinTone, reportID, setIsCommentEmpty], + [debouncedUpdateFrequentlyUsedEmojis, preferredLocale, preferredSkinTone, reportID, setIsCommentEmpty, raiseIsScrollLikelyLayoutTriggered], ); /** @@ -317,15 +328,12 @@ function ComposerWithSuggestions({ ); const hideSuggestionMenu = useCallback(() => { - if (!suggestionsRef.current || shouldIgnoreScrollCallback.current) { - shouldIgnoreScrollCallback.current = false; + if (!suggestionsRef.current || isScrollLikelyLayoutTriggered.current) { return; } suggestionsRef.current.updateShouldShowSuggestionMenuToFalse(false); }, [suggestionsRef]); - const debouncedHideSuggestionMenu = useMemo(() => _.debounce(hideSuggestionMenu, 200, true), [hideSuggestionMenu]); - const setShouldBlockSuggestionCalcToFalse = useCallback(() => { if (!suggestionsRef.current) { return false; @@ -499,7 +507,7 @@ function ComposerWithSuggestions({ } setComposerHeight(composerLayoutHeight); }} - onScroll={debouncedHideSuggestionMenu} + onScroll={hideSuggestionMenu} /> From a187423e51e24f12ce9b0c0623796ef484236bea Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 16:37:36 +0800 Subject: [PATCH 03/16] add jsdoc --- src/hooks/useDebounce.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 0ca06cdbb197..40b21d38451e 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -1,6 +1,12 @@ import {useEffect, useRef} from 'react'; import _ from 'underscore'; +/** + * @param {*} func Function to debounce `waitMS` ms. + * @param {*} wait The number of milliseconds to wait before `func` can be invoked again. + * @param {*} immediate True if `func` should be invoked on the leading edge of `waitMS` instead of the trailing edge. + * @returns + */ export default function useDebounce(func, wait, immediate) { const debouncedFnRef = useRef(); From 3a577e623ab57f10a8ee719656b5ab1b9609b8d1 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 16:37:42 +0800 Subject: [PATCH 04/16] add comment --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index c4bea207cb75..ed98356da970 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -137,6 +137,13 @@ function ComposerWithSuggestions({ insertedEmojisRef.current = []; }, []); + /** + * Reset isScrollLikelyLayoutTriggered to false. + * + * The function is debounced with a handpicked wait time to address 2 issues: + * 1. There is a slight delay between onChangeText and onScroll + * 2. Layout change will trigger onScroll multiple times + */ const debouncedLowerIsScrollLikelyLayoutTriggered = useDebounce( useCallback(() => (isScrollLikelyLayoutTriggered.current = false), []), 500, From 762e549db02675d4e28124d34cd3d55e24ea1499 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 16:39:50 +0800 Subject: [PATCH 05/16] use lodash debounce --- src/hooks/useDebounce.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 40b21d38451e..f78eb2ae84b6 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -1,5 +1,5 @@ import {useEffect, useRef} from 'react'; -import _ from 'underscore'; +import lodashDebounce from 'lodash/debounce'; /** * @param {*} func Function to debounce `waitMS` ms. @@ -11,7 +11,7 @@ export default function useDebounce(func, wait, immediate) { const debouncedFnRef = useRef(); useEffect(() => { - const debouncedFn = _.debounce(func, wait, immediate); + const debouncedFn = lodashDebounce(func, wait, immediate); debouncedFnRef.current = debouncedFn; From c212e9e2237377378c2c4172113ff3892ad0811a Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 16:42:18 +0800 Subject: [PATCH 06/16] update params and jsdoc --- src/hooks/useDebounce.js | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index f78eb2ae84b6..09cb79dfc440 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -2,21 +2,24 @@ import {useEffect, useRef} from 'react'; import lodashDebounce from 'lodash/debounce'; /** - * @param {*} func Function to debounce `waitMS` ms. - * @param {*} wait The number of milliseconds to wait before `func` can be invoked again. - * @param {*} immediate True if `func` should be invoked on the leading edge of `waitMS` instead of the trailing edge. - * @returns + * @param {Function} func The function to debounce. + * @param {Number} wait The number of milliseconds to delay. + * @param {Object} options The options object. + * @param {Boolean} options.leading Specify invoking on the leading edge of the timeout. + * @param {Number} options.maxWait The maximum time func is allowed to be delayed before it’s invoked. + * @param {Boolean} options.trailing Specify invoking on the trailing edge of the timeout. + * @returns Returns the new debounced function. */ -export default function useDebounce(func, wait, immediate) { +export default function useDebounce(func, wait, options) { const debouncedFnRef = useRef(); useEffect(() => { - const debouncedFn = lodashDebounce(func, wait, immediate); + const debouncedFn = lodashDebounce(func, wait, options); debouncedFnRef.current = debouncedFn; return debouncedFn.cancel; - }, [func, wait, immediate]); + }, [func, wait, options]); return debouncedFnRef.current; } From 2f93e7670757f4eba44ce2ff0b7598be5c163d6d Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 16:44:31 +0800 Subject: [PATCH 07/16] add comment --- .../home/report/ReportActionCompose/ComposerWithSuggestions.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index ed98356da970..9bdc6e3251da 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -126,6 +126,8 @@ function ComposerWithSuggestions({ const textInputRef = useRef(null); const insertedEmojisRef = useRef([]); + + // A flag to indicate whether onScroll callback is likely triggered by layout change or not const isScrollLikelyLayoutTriggered = useRef(false); /** From 81d66d4bfc2592d9c93d1e30a60b82a60a12a3f5 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 16:45:46 +0800 Subject: [PATCH 08/16] update comment --- .../home/report/ReportActionCompose/ComposerWithSuggestions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 9bdc6e3251da..0e40d1d1cb14 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -127,7 +127,7 @@ function ComposerWithSuggestions({ const textInputRef = useRef(null); const insertedEmojisRef = useRef([]); - // A flag to indicate whether onScroll callback is likely triggered by layout change or not + // A flag to indicate whether the onScroll callback is likely triggered by a layout change (caused by text change) or not const isScrollLikelyLayoutTriggered = useRef(false); /** From a6a03e1538e4de0c9219aa8ea2fc5149b6cf0df4 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 17:10:24 +0800 Subject: [PATCH 09/16] make sure debounced fn is not undefined before calling it --- src/hooks/useDebounce.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 09cb79dfc440..d6ff20a1e91b 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -8,7 +8,7 @@ import lodashDebounce from 'lodash/debounce'; * @param {Boolean} options.leading Specify invoking on the leading edge of the timeout. * @param {Number} options.maxWait The maximum time func is allowed to be delayed before it’s invoked. * @param {Boolean} options.trailing Specify invoking on the trailing edge of the timeout. - * @returns Returns the new debounced function. + * @returns Returns a function to call the debounced function. */ export default function useDebounce(func, wait, options) { const debouncedFnRef = useRef(); @@ -21,5 +21,11 @@ export default function useDebounce(func, wait, options) { return debouncedFn.cancel; }, [func, wait, options]); - return debouncedFnRef.current; + return (...args) => { + const debouncedFn = debouncedFnRef.current; + + if (debouncedFn) { + debouncedFn(...args); + } + }; } From 34b682590100951da4ba06e0a098016719f9b98c Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 17:17:17 +0800 Subject: [PATCH 10/16] add return type --- src/hooks/useDebounce.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index d6ff20a1e91b..266ea5d3058a 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -8,7 +8,7 @@ import lodashDebounce from 'lodash/debounce'; * @param {Boolean} options.leading Specify invoking on the leading edge of the timeout. * @param {Number} options.maxWait The maximum time func is allowed to be delayed before it’s invoked. * @param {Boolean} options.trailing Specify invoking on the trailing edge of the timeout. - * @returns Returns a function to call the debounced function. + * @returns {Function} Returns a function to call the debounced function. */ export default function useDebounce(func, wait, options) { const debouncedFnRef = useRef(); From dfa7435cc1211b81a5e359a72c683db980e07e8b Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 17:32:36 +0800 Subject: [PATCH 11/16] prettier --- .../home/report/ReportActionCompose/ComposerWithSuggestions.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 847f3146a7b8..53c71aa82f94 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -141,7 +141,7 @@ function ComposerWithSuggestions({ /** * Reset isScrollLikelyLayoutTriggered to false. - * + * * The function is debounced with a handpicked wait time to address 2 issues: * 1. There is a slight delay between onChangeText and onScroll * 2. Layout change will trigger onScroll multiple times From 4fe1076356aa5a00e64fbf771df2e82c0cee3c11 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 18:27:11 +0800 Subject: [PATCH 12/16] update comment --- src/hooks/useDebounce.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 266ea5d3058a..7a44ad4deb1b 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -2,6 +2,10 @@ import {useEffect, useRef} from 'react'; import lodashDebounce from 'lodash/debounce'; /** + * Create and return a debounced function. + * + * Make sure to pass a stable function reference to prevent recreating the debounced function on each render. + * * @param {Function} func The function to debounce. * @param {Number} wait The number of milliseconds to delay. * @param {Object} options The options object. From 76b9cc7cf578e263a82d27e860281e0a31c2c9b3 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 18:37:07 +0800 Subject: [PATCH 13/16] prettier --- src/hooks/useDebounce.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 7a44ad4deb1b..ccafb832e7ab 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -3,9 +3,9 @@ import lodashDebounce from 'lodash/debounce'; /** * Create and return a debounced function. - * + * * Make sure to pass a stable function reference to prevent recreating the debounced function on each render. - * + * * @param {Function} func The function to debounce. * @param {Number} wait The number of milliseconds to delay. * @param {Object} options The options object. From 4b71ed2c879a645c75c20f93ca1ae43d318b5e05 Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Wed, 20 Sep 2023 21:26:43 +0800 Subject: [PATCH 14/16] update comment --- src/hooks/useDebounce.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index ccafb832e7ab..137c41e8bfb1 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -4,7 +4,8 @@ import lodashDebounce from 'lodash/debounce'; /** * Create and return a debounced function. * - * Make sure to pass a stable function reference to prevent recreating the debounced function on each render. + * Every time the identity of any of the arguments changes, the debounce operation will restart (canceling any ongoing debounce). + * This is especially important in the case of func. To prevent that, pass stable references. * * @param {Function} func The function to debounce. * @param {Number} wait The number of milliseconds to delay. From e416c34821adda0a9b2f2cc871574fae4284b24f Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 21 Sep 2023 02:05:36 +0800 Subject: [PATCH 15/16] unpack options args --- src/hooks/useDebounce.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 137c41e8bfb1..2d6ad83bedab 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -17,14 +17,15 @@ import lodashDebounce from 'lodash/debounce'; */ export default function useDebounce(func, wait, options) { const debouncedFnRef = useRef(); + const {leading, maxWait, trailing} = options || {}; useEffect(() => { - const debouncedFn = lodashDebounce(func, wait, options); + const debouncedFn = lodashDebounce(func, wait, {leading, maxWait, trailing}); debouncedFnRef.current = debouncedFn; return debouncedFn.cancel; - }, [func, wait, options]); + }, [func, wait, leading, maxWait, trailing]); return (...args) => { const debouncedFn = debouncedFnRef.current; From 489b600412f09679243789a5260642a53360beac Mon Sep 17 00:00:00 2001 From: Bernhard Owen Josephus Date: Thu, 21 Sep 2023 19:46:13 +0800 Subject: [PATCH 16/16] make trailing default to true --- src/hooks/useDebounce.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hooks/useDebounce.js b/src/hooks/useDebounce.js index 2d6ad83bedab..8995a0443b85 100644 --- a/src/hooks/useDebounce.js +++ b/src/hooks/useDebounce.js @@ -17,7 +17,7 @@ import lodashDebounce from 'lodash/debounce'; */ export default function useDebounce(func, wait, options) { const debouncedFnRef = useRef(); - const {leading, maxWait, trailing} = options || {}; + const {leading, maxWait, trailing = true} = options || {}; useEffect(() => { const debouncedFn = lodashDebounce(func, wait, {leading, maxWait, trailing});