From adc3aeed2e03d572807c31b3f9b0367a42fc2b94 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Wed, 4 Oct 2023 12:22:56 +0530 Subject: [PATCH 01/12] fix: auto focus and scroll to bottom on large-text in composer --- .../ComposerWithSuggestions.js | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 6e103f77c068..7f8480768a8d 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -1,5 +1,5 @@ import React, {useEffect, useCallback, useState, useRef, useMemo, useImperativeHandle} from 'react'; -import {View, NativeModules, findNodeHandle} from 'react-native'; +import {View, NativeModules, findNodeHandle, InteractionManager} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import lodashGet from 'lodash/get'; @@ -33,6 +33,7 @@ import withKeyboardState from '../../../../components/withKeyboardState'; import {propTypes, defaultProps} from './composerWithSuggestionsProps'; import focusWithDelay from '../../../../libs/focusWithDelay'; import useDebounce from '../../../../hooks/useDebounce'; +import updateMultilineInputRange from '../../../../libs/UpdateMultilineInputRange'; const {RNTextInputReset} = NativeModules; @@ -111,6 +112,7 @@ function ComposerWithSuggestions({ const isEmptyChat = useMemo(() => _.size(reportActions) === 1, [reportActions]); const shouldAutoFocus = !modal.isVisible && (shouldFocusInputOnScreenFocus || isEmptyChat) && shouldShowComposeInput; + const autoFocusCheck = shouldAutoFocus && !willBlurTextInputOnTapOutside; const valueRef = useRef(value); valueRef.current = value; @@ -124,6 +126,7 @@ function ComposerWithSuggestions({ const textInputRef = useRef(null); const insertedEmojisRef = useRef([]); + const initialFocusedRef = useRef(false); // 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); @@ -445,6 +448,34 @@ function ComposerWithSuggestions({ textInputRef.current.blur(); }, []); + const focusWithScrolledToBottom = useCallback(() => { + if (!textInputRef.current || !willBlurTextInputOnTapOutside) { + return; + } + + InteractionManager.runAfterInteractions(() => { + // Using `shouldAutoFocus` check to determine whether the component should be focused or not. + // Also handled Mobile Safari case, where the input should scroll to bottom and focus. + if (!shouldAutoFocus && !isMobileSafari) { + return; + } + + updateMultilineInputRange(textInputRef.current); + + focus(); // This will focus the composer after its fully rendered. + }); + }, [focus, value.length, autoFocusCheck]); + + useEffect(() => { + // Added initial focus ref to remove unneccessary focus after first render. + if (initialFocusedRef.current) { + return; + } + + initialFocusedRef.current = true; + focusWithScrolledToBottom(); + }, [focusWithScrolledToBottom]); + useEffect(() => { const unsubscribeNavigationBlur = navigation.addListener('blur', () => KeyDownListener.removeKeyDownPressListner(focusComposerOnKeyPress)); const unsubscribeNavigationFocus = navigation.addListener('focus', () => { From 72bb8a23134f22cb6fd49df698a482807bee58e0 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Wed, 4 Oct 2023 21:41:10 +0530 Subject: [PATCH 02/12] changed shouldAutoFocus to autoFocusCheck in Composer --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 7f8480768a8d..da2aab3a436e 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -456,7 +456,7 @@ function ComposerWithSuggestions({ InteractionManager.runAfterInteractions(() => { // Using `shouldAutoFocus` check to determine whether the component should be focused or not. // Also handled Mobile Safari case, where the input should scroll to bottom and focus. - if (!shouldAutoFocus && !isMobileSafari) { + if (!shouldAutoFocus) { return; } @@ -538,7 +538,7 @@ function ComposerWithSuggestions({ Date: Wed, 4 Oct 2023 21:52:55 +0530 Subject: [PATCH 03/12] updated name for flag and fixed callback dependencies --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index da2aab3a436e..ee28b54f5c88 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -112,7 +112,7 @@ function ComposerWithSuggestions({ const isEmptyChat = useMemo(() => _.size(reportActions) === 1, [reportActions]); const shouldAutoFocus = !modal.isVisible && (shouldFocusInputOnScreenFocus || isEmptyChat) && shouldShowComposeInput; - const autoFocusCheck = shouldAutoFocus && !willBlurTextInputOnTapOutside; + const shouldAutoFocusWithLongText = shouldAutoFocus && !willBlurTextInputOnTapOutside; const valueRef = useRef(value); valueRef.current = value; @@ -464,7 +464,7 @@ function ComposerWithSuggestions({ focus(); // This will focus the composer after its fully rendered. }); - }, [focus, value.length, autoFocusCheck]); + }, [focus, shouldAutoFocus]); useEffect(() => { // Added initial focus ref to remove unneccessary focus after first render. @@ -538,7 +538,7 @@ function ComposerWithSuggestions({ Date: Wed, 4 Oct 2023 21:58:00 +0530 Subject: [PATCH 04/12] removed extra code --- .../ReportActionCompose/ComposerWithSuggestions.js | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index ee28b54f5c88..d77c66a292ed 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -449,19 +449,13 @@ function ComposerWithSuggestions({ }, []); const focusWithScrolledToBottom = useCallback(() => { - if (!textInputRef.current || !willBlurTextInputOnTapOutside) { + // Using `shouldAutoFocus` check to determine whether the component should be focused or not. + if (!textInputRef.current || !willBlurTextInputOnTapOutside || !shouldAutoFocus) { return; } InteractionManager.runAfterInteractions(() => { - // Using `shouldAutoFocus` check to determine whether the component should be focused or not. - // Also handled Mobile Safari case, where the input should scroll to bottom and focus. - if (!shouldAutoFocus) { - return; - } - updateMultilineInputRange(textInputRef.current); - focus(); // This will focus the composer after its fully rendered. }); }, [focus, shouldAutoFocus]); From f1899814298f0046085c1f4ead762d470ed91ac3 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Thu, 5 Oct 2023 22:25:17 +0530 Subject: [PATCH 05/12] removed InteractionManager as it was creating a janky animation --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index d77c66a292ed..c7ba7e5588bc 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -1,5 +1,5 @@ import React, {useEffect, useCallback, useState, useRef, useMemo, useImperativeHandle} from 'react'; -import {View, NativeModules, findNodeHandle, InteractionManager} from 'react-native'; +import {View, NativeModules, findNodeHandle} from 'react-native'; import {withOnyx} from 'react-native-onyx'; import _ from 'underscore'; import lodashGet from 'lodash/get'; @@ -454,10 +454,8 @@ function ComposerWithSuggestions({ return; } - InteractionManager.runAfterInteractions(() => { - updateMultilineInputRange(textInputRef.current); - focus(); // This will focus the composer after its fully rendered. - }); + updateMultilineInputRange(textInputRef.current); + focus(); // Manually focus the input - as we have disabled `autoFocus` flag in `Composer`. }, [focus, shouldAutoFocus]); useEffect(() => { From 23ff678a804a4d771acbbe033adbbba925b60598 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Fri, 6 Oct 2023 20:38:54 +0530 Subject: [PATCH 06/12] Updated comments and variable name for better understanding --- .../ComposerWithSuggestions.js | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index c7ba7e5588bc..916b223f36eb 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -51,6 +51,7 @@ const debouncedBroadcastUserIsTyping = _.debounce((reportID) => { }, 100); const willBlurTextInputOnTapOutside = willBlurTextInputOnTapOutsideFunc(); +const isNativeApp = !willBlurTextInputOnTapOutside; // We want consistent auto focus behavior on input between native and mWeb so we have some auto focus management code that will // prevent auto focus on existing chat for mobile device @@ -112,7 +113,10 @@ function ComposerWithSuggestions({ const isEmptyChat = useMemo(() => _.size(reportActions) === 1, [reportActions]); const shouldAutoFocus = !modal.isVisible && (shouldFocusInputOnScreenFocus || isEmptyChat) && shouldShowComposeInput; - const shouldAutoFocusWithLongText = shouldAutoFocus && !willBlurTextInputOnTapOutside; + + // Disabled auto-focus for `web platforms`. + // Focus for `web platforms` with `focusWithScrolledToBottom` method. + const autoFocusOnMount = shouldAutoFocus && isNativeApp; const valueRef = useRef(value); valueRef.current = value; @@ -449,17 +453,21 @@ function ComposerWithSuggestions({ }, []); const focusWithScrolledToBottom = useCallback(() => { - // Using `shouldAutoFocus` check to determine whether the component should be focused or not. - if (!textInputRef.current || !willBlurTextInputOnTapOutside || !shouldAutoFocus) { + // `shouldAutoFocus` determines if component should focus or not. + if (isNativeApp || !shouldAutoFocus) { return; } + // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. updateMultilineInputRange(textInputRef.current); - focus(); // Manually focus the input - as we have disabled `autoFocus` flag in `Composer`. + + // `autoFocusOnMount` has disabled auto-focus for `Native Platforms`. + // So we Focus input programmatically here. + focus(); }, [focus, shouldAutoFocus]); useEffect(() => { - // Added initial focus ref to remove unneccessary focus after first render. + // Initial focus ref to prevent unneccessary focus after first render. if (initialFocusedRef.current) { return; } @@ -530,7 +538,7 @@ function ComposerWithSuggestions({ Date: Mon, 9 Oct 2023 22:04:12 +0530 Subject: [PATCH 07/12] remove unnecessary code. --- .../ReportActionCompose/ComposerWithSuggestions.js | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 916b223f36eb..d8068c6ff55e 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -114,10 +114,6 @@ function ComposerWithSuggestions({ const isEmptyChat = useMemo(() => _.size(reportActions) === 1, [reportActions]); const shouldAutoFocus = !modal.isVisible && (shouldFocusInputOnScreenFocus || isEmptyChat) && shouldShowComposeInput; - // Disabled auto-focus for `web platforms`. - // Focus for `web platforms` with `focusWithScrolledToBottom` method. - const autoFocusOnMount = shouldAutoFocus && isNativeApp; - const valueRef = useRef(value); valueRef.current = value; @@ -460,11 +456,7 @@ function ComposerWithSuggestions({ // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. updateMultilineInputRange(textInputRef.current); - - // `autoFocusOnMount` has disabled auto-focus for `Native Platforms`. - // So we Focus input programmatically here. - focus(); - }, [focus, shouldAutoFocus]); + }, [shouldAutoFocus]); useEffect(() => { // Initial focus ref to prevent unneccessary focus after first render. @@ -538,7 +530,7 @@ function ComposerWithSuggestions({ Date: Mon, 9 Oct 2023 22:05:49 +0530 Subject: [PATCH 08/12] removed `focusWithScrolledToBottom` --- .../ComposerWithSuggestions.js | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index d8068c6ff55e..62d219f2daf5 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -448,25 +448,16 @@ function ComposerWithSuggestions({ textInputRef.current.blur(); }, []); - const focusWithScrolledToBottom = useCallback(() => { - // `shouldAutoFocus` determines if component should focus or not. - if (isNativeApp || !shouldAutoFocus) { - return; - } - - // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. - updateMultilineInputRange(textInputRef.current); - }, [shouldAutoFocus]); - useEffect(() => { // Initial focus ref to prevent unneccessary focus after first render. - if (initialFocusedRef.current) { + if (initialFocusedRef.current || isNativeApp || !shouldAutoFocus) { return; } + // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. + updateMultilineInputRange(textInputRef.current); initialFocusedRef.current = true; - focusWithScrolledToBottom(); - }, [focusWithScrolledToBottom]); + }, [shouldAutoFocus]); useEffect(() => { const unsubscribeNavigationBlur = navigation.addListener('blur', () => KeyDownListener.removeKeyDownPressListner(focusComposerOnKeyPress)); From 317d06018681a770b910993990eb7399d41ca442 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Tue, 10 Oct 2023 21:44:03 +0530 Subject: [PATCH 09/12] fix: remove extra code and handle web scroll to bottom --- .../report/ReportActionCompose/ComposerWithSuggestions.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 62d219f2daf5..d780199aadf6 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -51,7 +51,6 @@ const debouncedBroadcastUserIsTyping = _.debounce((reportID) => { }, 100); const willBlurTextInputOnTapOutside = willBlurTextInputOnTapOutsideFunc(); -const isNativeApp = !willBlurTextInputOnTapOutside; // We want consistent auto focus behavior on input between native and mWeb so we have some auto focus management code that will // prevent auto focus on existing chat for mobile device @@ -126,7 +125,6 @@ function ComposerWithSuggestions({ const textInputRef = useRef(null); const insertedEmojisRef = useRef([]); - const initialFocusedRef = useRef(false); // 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); @@ -449,14 +447,12 @@ function ComposerWithSuggestions({ }, []); useEffect(() => { - // Initial focus ref to prevent unneccessary focus after first render. - if (initialFocusedRef.current || isNativeApp || !shouldAutoFocus) { + if (!shouldAutoFocus && !willBlurTextInputOnTapOutside) { return; } // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. updateMultilineInputRange(textInputRef.current); - initialFocusedRef.current = true; }, [shouldAutoFocus]); useEffect(() => { From 82778eeacf0e8f2eae02d1e7d6b4a23d3e79c6b3 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Wed, 11 Oct 2023 19:49:27 +0530 Subject: [PATCH 10/12] removed extra useEffect and extra checks --- .../ReportActionCompose/ComposerWithSuggestions.js | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index 1f019be42100..c81a60d2a234 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -447,15 +447,6 @@ function ComposerWithSuggestions({ textInputRef.current.blur(); }, []); - useEffect(() => { - if (!shouldAutoFocus && !willBlurTextInputOnTapOutside) { - return; - } - - // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. - updateMultilineInputRange(textInputRef.current); - }, [shouldAutoFocus]); - useEffect(() => { const unsubscribeNavigationBlur = navigation.addListener('blur', () => KeyDownListener.removeKeyDownPressListner(focusComposerOnKeyPress)); const unsubscribeNavigationFocus = navigation.addListener('focus', () => { @@ -493,9 +484,13 @@ function ComposerWithSuggestions({ }, [focus, prevIsFocused, prevIsModalVisible, isFocused, modal.isVisible, isNextModalWillOpenRef]); useEffect(() => { + // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. + updateMultilineInputRange(textInputRef.current); + if (value.length === 0) { return; } + Report.setReportWithDraft(reportID, true); // eslint-disable-next-line react-hooks/exhaustive-deps From 8972e44a74c4fb587bedbeebdb6aa38411c91959 Mon Sep 17 00:00:00 2001 From: jeet-dhandha Date: Thu, 12 Oct 2023 23:07:15 +0530 Subject: [PATCH 11/12] updated checks --- src/libs/UpdateMultilineInputRange/index.ios.js | 7 +++++-- src/libs/UpdateMultilineInputRange/index.js | 4 +++- .../report/ReportActionCompose/ComposerWithSuggestions.js | 2 +- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/src/libs/UpdateMultilineInputRange/index.ios.js b/src/libs/UpdateMultilineInputRange/index.ios.js index 85ed529a33bc..4c10f768a2a2 100644 --- a/src/libs/UpdateMultilineInputRange/index.ios.js +++ b/src/libs/UpdateMultilineInputRange/index.ios.js @@ -8,8 +8,9 @@ * See https://github.com/Expensify/App/issues/20836 for more details. * * @param {Object} input the input element + * @param {boolean} shouldAutoFocus */ -export default function updateMultilineInputRange(input) { +export default function updateMultilineInputRange(input, shouldAutoFocus = true) { if (!input) { return; } @@ -19,5 +20,7 @@ export default function updateMultilineInputRange(input) { * Issue: does not scroll multiline input when text exceeds the maximum number of lines * For more details: https://github.com/Expensify/App/pull/27702#issuecomment-1728651132 */ - input.focus(); + if (shouldAutoFocus) { + input.focus(); + } } diff --git a/src/libs/UpdateMultilineInputRange/index.js b/src/libs/UpdateMultilineInputRange/index.js index 179d30dc611d..66fb1889be21 100644 --- a/src/libs/UpdateMultilineInputRange/index.js +++ b/src/libs/UpdateMultilineInputRange/index.js @@ -8,8 +8,10 @@ * See https://github.com/Expensify/App/issues/20836 for more details. * * @param {Object} input the input element + * @param {boolean} shouldAutoFocus */ -export default function updateMultilineInputRange(input) { +// eslint-disable-next-line no-unused-vars +export default function updateMultilineInputRange(input, shouldAutoFocus = true) { if (!input) { return; } diff --git a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js index c81a60d2a234..d6821dd804cf 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -485,7 +485,7 @@ function ComposerWithSuggestions({ useEffect(() => { // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. - updateMultilineInputRange(textInputRef.current); + updateMultilineInputRange(textInputRef.current, shouldAutoFocus); if (value.length === 0) { return; From 1fbbbb0c68f3435e64f11b16fa2ca0a1f9fd50e4 Mon Sep 17 00:00:00 2001 From: jeet-dhandha <78416198+jeet-dhandha@users.noreply.github.com> Date: Fri, 13 Oct 2023 20:13:21 +0530 Subject: [PATCH 12/12] Update src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js Co-authored-by: Joel Davies --- .../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 1b407bc4bea6..7077f2a01b14 100644 --- a/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js +++ b/src/pages/home/report/ReportActionCompose/ComposerWithSuggestions.js @@ -491,7 +491,7 @@ function ComposerWithSuggestions({ }, [focus, prevIsFocused, prevIsModalVisible, isFocused, modal.isVisible, isNextModalWillOpenRef]); useEffect(() => { - // Set the `selection at end` and `scrolls input to bottom` for `Web Platforms`. + // Scrolls the composer to the bottom and sets the selection to the end, so that longer drafts are easier to edit updateMultilineInputRange(textInputRef.current, shouldAutoFocus); if (value.length === 0) {