From 367b215d9d3f29a511aeb97154889edbead82739 Mon Sep 17 00:00:00 2001 From: MrRefactor Date: Mon, 4 Mar 2024 15:37:38 +0800 Subject: [PATCH 1/3] Fix problem with cursor jumping back to start when adding an emoji in edit mode --- src/libs/focusComposerWithDelay.ts | 34 ------------ src/libs/focusComposerWithDelay/index.ts | 52 +++++++++++++++++++ src/libs/focusComposerWithDelay/types.ts | 8 +++ src/libs/shouldSetSelectionRange/index.ts | 5 ++ .../shouldSetSelectionRange/index.website.ts | 5 ++ src/libs/shouldSetSelectionRange/types.ts | 3 ++ .../report/ReportActionItemMessageEdit.tsx | 31 +++++++---- .../index.android.ts | 5 ++ .../shouldUseEmojiPickerSelection/index.ts | 5 ++ .../index.website.ts | 8 +++ .../shouldUseEmojiPickerSelection/types.ts | 3 ++ 11 files changed, 116 insertions(+), 43 deletions(-) delete mode 100644 src/libs/focusComposerWithDelay.ts create mode 100644 src/libs/focusComposerWithDelay/index.ts create mode 100644 src/libs/focusComposerWithDelay/types.ts create mode 100644 src/libs/shouldSetSelectionRange/index.ts create mode 100644 src/libs/shouldSetSelectionRange/index.website.ts create mode 100644 src/libs/shouldSetSelectionRange/types.ts create mode 100644 src/pages/home/report/shouldUseEmojiPickerSelection/index.android.ts create mode 100644 src/pages/home/report/shouldUseEmojiPickerSelection/index.ts create mode 100644 src/pages/home/report/shouldUseEmojiPickerSelection/index.website.ts create mode 100644 src/pages/home/report/shouldUseEmojiPickerSelection/types.ts diff --git a/src/libs/focusComposerWithDelay.ts b/src/libs/focusComposerWithDelay.ts deleted file mode 100644 index 6a2f85f7d311..000000000000 --- a/src/libs/focusComposerWithDelay.ts +++ /dev/null @@ -1,34 +0,0 @@ -import type {TextInput} from 'react-native'; -import * as EmojiPickerAction from './actions/EmojiPickerAction'; -import ComposerFocusManager from './ComposerFocusManager'; - -type FocusComposerWithDelay = (shouldDelay?: boolean) => void; -/** - * Create a function that focuses the composer. - */ -function focusComposerWithDelay(textInput: TextInput | null): FocusComposerWithDelay { - /** - * Focus the text input - * @param [shouldDelay] Impose delay before focusing the text input - */ - return (shouldDelay = false) => { - // There could be other animations running while we trigger manual focus. - // This prevents focus from making those animations janky. - if (!textInput || EmojiPickerAction.isEmojiPickerVisible()) { - return; - } - - if (!shouldDelay) { - textInput.focus(); - return; - } - ComposerFocusManager.isReadyToFocus().then(() => { - if (!textInput) { - return; - } - textInput.focus(); - }); - }; -} - -export default focusComposerWithDelay; diff --git a/src/libs/focusComposerWithDelay/index.ts b/src/libs/focusComposerWithDelay/index.ts new file mode 100644 index 000000000000..e1737ea3e6dd --- /dev/null +++ b/src/libs/focusComposerWithDelay/index.ts @@ -0,0 +1,52 @@ +import type {TextInput} from 'react-native'; +import ComposerFocusManager from '@libs/ComposerFocusManager'; +import shouldSetSelectionRange from '@libs/shouldSetSelectionRange'; +import * as EmojiPickerAction from '@userActions/EmojiPickerAction'; +import type {FocusComposerWithDelay} from './types'; + +const setSelectionRange = shouldSetSelectionRange(); + +/** + * Create a function that focuses the composer. + */ +function focusComposerWithDelay(textInput: TextInput | HTMLTextAreaElement | null): FocusComposerWithDelay { + /** + * Focus the text input + * @param [shouldDelay] Impose delay before focusing the text input + * @param [forceSetSelection] Force selection range of text input + */ + return (shouldDelay = false, forceSetSelection = undefined) => { + // There could be other animations running while we trigger manual focus. + // This prevents focus from making those animations janky. + if (!textInput || EmojiPickerAction.isEmojiPickerVisible()) { + return; + } + + if (!shouldDelay) { + textInput.focus(); + if (forceSetSelection) { + if (setSelectionRange) { + (textInput as HTMLTextAreaElement).setSelectionRange(forceSetSelection.start, forceSetSelection.end); + } else { + (textInput as TextInput).setSelection(forceSetSelection.start, forceSetSelection.end); + } + } + return; + } + ComposerFocusManager.isReadyToFocus().then(() => { + if (!textInput) { + return; + } + textInput.focus(); + if (forceSetSelection) { + if (setSelectionRange) { + (textInput as HTMLTextAreaElement).setSelectionRange(forceSetSelection.start, forceSetSelection.end); + } else { + (textInput as TextInput).setSelection(forceSetSelection.start, forceSetSelection.end); + } + } + }); + }; +} + +export default focusComposerWithDelay; diff --git a/src/libs/focusComposerWithDelay/types.ts b/src/libs/focusComposerWithDelay/types.ts new file mode 100644 index 000000000000..86e9b0f3ac9a --- /dev/null +++ b/src/libs/focusComposerWithDelay/types.ts @@ -0,0 +1,8 @@ +type Selection = { + start: number; + end: number; +}; + +type FocusComposerWithDelay = (shouldDelay?: boolean, forceSetSelection?: Selection) => void; + +export type {Selection, FocusComposerWithDelay}; diff --git a/src/libs/shouldSetSelectionRange/index.ts b/src/libs/shouldSetSelectionRange/index.ts new file mode 100644 index 000000000000..1b28bdef07cb --- /dev/null +++ b/src/libs/shouldSetSelectionRange/index.ts @@ -0,0 +1,5 @@ +import type ShouldSetSelectionRange from './types'; + +const shouldSetSelectionRange: ShouldSetSelectionRange = () => false; + +export default shouldSetSelectionRange; diff --git a/src/libs/shouldSetSelectionRange/index.website.ts b/src/libs/shouldSetSelectionRange/index.website.ts new file mode 100644 index 000000000000..655193ebfb67 --- /dev/null +++ b/src/libs/shouldSetSelectionRange/index.website.ts @@ -0,0 +1,5 @@ +import type ShouldSetSelectionRange from './types'; + +const shouldSetSelectionRange: ShouldSetSelectionRange = () => true; + +export default shouldSetSelectionRange; diff --git a/src/libs/shouldSetSelectionRange/types.ts b/src/libs/shouldSetSelectionRange/types.ts new file mode 100644 index 000000000000..e0a80614a32e --- /dev/null +++ b/src/libs/shouldSetSelectionRange/types.ts @@ -0,0 +1,3 @@ +type ShouldSetSelectionRange = () => boolean; + +export default ShouldSetSelectionRange; diff --git a/src/pages/home/report/ReportActionItemMessageEdit.tsx b/src/pages/home/report/ReportActionItemMessageEdit.tsx index 2c9a4cbd21e8..c8a078d38d9f 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.tsx +++ b/src/pages/home/report/ReportActionItemMessageEdit.tsx @@ -25,6 +25,7 @@ import * as Browser from '@libs/Browser'; import * as ComposerUtils from '@libs/ComposerUtils'; import * as EmojiUtils from '@libs/EmojiUtils'; import focusComposerWithDelay from '@libs/focusComposerWithDelay'; +import type {Selection} from '@libs/focusComposerWithDelay/types'; import focusEditAfterCancelDelete from '@libs/focusEditAfterCancelDelete'; import onyxSubscribe from '@libs/onyxSubscribe'; import ReportActionComposeFocusManager from '@libs/ReportActionComposeFocusManager'; @@ -40,6 +41,7 @@ import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type * as OnyxTypes from '@src/types/onyx'; import * as ReportActionContextMenu from './ContextMenu/ReportActionContextMenu'; +import shouldUseEmojiPickerSelection from './shouldUseEmojiPickerSelection'; type ReportActionItemMessageEditProps = { /** All the data of the action */ @@ -105,10 +107,7 @@ function ReportActionItemMessageEdit( } return initialDraft; }); - const [selection, setSelection] = useState<{ - start: number; - end: number; - }>(getInitialSelection); + const [selection, setSelection] = useState(getInitialSelection); const [isFocused, setIsFocused] = useState(false); const {hasExceededMaxCommentLength, validateCommentMaxLength} = useHandleExceedMaxCommentLength(); const [modal, setModal] = useState({ @@ -121,6 +120,7 @@ function ReportActionItemMessageEdit( const isFocusedRef = useRef(false); const insertedEmojis = useRef([]); const draftRef = useRef(draft); + const emojiPickerSelectionRef = useRef(undefined); useEffect(() => { if (ReportActionsUtils.isDeletedAction(action) || (action.message && draftMessage === action.message[0].html)) { @@ -329,14 +329,22 @@ function ReportActionItemMessageEdit( deleteDraft(); }, [action, debouncedSaveDraft, deleteDraft, draft, reportID]); + const shouldUseEmojiPickerSelectionRef = shouldUseEmojiPickerSelection(); /** * @param emoji */ const addEmojiToTextBox = (emoji: string) => { - setSelection((prevSelection) => ({ - start: prevSelection.start + emoji.length + CONST.SPACE_LENGTH, - end: prevSelection.start + emoji.length + CONST.SPACE_LENGTH, - })); + const newSelection = { + start: selection.start + emoji.length + CONST.SPACE_LENGTH, + end: selection.start + emoji.length + CONST.SPACE_LENGTH, + }; + setSelection(newSelection); + + if (shouldUseEmojiPickerSelectionRef) { + // immediately set the selection again on android and Chrome mobile after focusing the + // input which seems to change the cursor position for a brief moment + emojiPickerSelectionRef.current = newSelection; + } updateDraft(ComposerUtils.insertText(draft, selection, `${emoji} `)); }; @@ -450,7 +458,12 @@ function ReportActionItemMessageEdit( focus(true)} + onModalHide={() => { + const emojiPickerSelection = emojiPickerSelectionRef.current ? {...emojiPickerSelectionRef.current} : undefined; + emojiPickerSelectionRef.current = undefined; + + focus(true, emojiPickerSelection); + }} onEmojiSelected={addEmojiToTextBox} id={emojiButtonID} emojiPickerID={action.reportActionID} diff --git a/src/pages/home/report/shouldUseEmojiPickerSelection/index.android.ts b/src/pages/home/report/shouldUseEmojiPickerSelection/index.android.ts new file mode 100644 index 000000000000..b8ffbc0518b0 --- /dev/null +++ b/src/pages/home/report/shouldUseEmojiPickerSelection/index.android.ts @@ -0,0 +1,5 @@ +import type ShouldUseEmojiPickerSelection from './types'; + +const shouldUseEmojiPickerSelection: ShouldUseEmojiPickerSelection = () => true; + +export default shouldUseEmojiPickerSelection; diff --git a/src/pages/home/report/shouldUseEmojiPickerSelection/index.ts b/src/pages/home/report/shouldUseEmojiPickerSelection/index.ts new file mode 100644 index 000000000000..2f94b3656e12 --- /dev/null +++ b/src/pages/home/report/shouldUseEmojiPickerSelection/index.ts @@ -0,0 +1,5 @@ +import type ShouldUseEmojiPickerSelection from './types'; + +const shouldUseEmojiPickerSelection: ShouldUseEmojiPickerSelection = () => false; + +export default shouldUseEmojiPickerSelection; diff --git a/src/pages/home/report/shouldUseEmojiPickerSelection/index.website.ts b/src/pages/home/report/shouldUseEmojiPickerSelection/index.website.ts new file mode 100644 index 000000000000..ff1982325d1d --- /dev/null +++ b/src/pages/home/report/shouldUseEmojiPickerSelection/index.website.ts @@ -0,0 +1,8 @@ +import * as Browser from '@libs/Browser'; +import type ShouldUseEmojiPickerSelection from './types'; + +const isMobileChrome = Browser.isMobileChrome(); + +const shouldUseEmojiPickerSelection: ShouldUseEmojiPickerSelection = () => isMobileChrome; + +export default shouldUseEmojiPickerSelection; diff --git a/src/pages/home/report/shouldUseEmojiPickerSelection/types.ts b/src/pages/home/report/shouldUseEmojiPickerSelection/types.ts new file mode 100644 index 000000000000..bbf96f867e7a --- /dev/null +++ b/src/pages/home/report/shouldUseEmojiPickerSelection/types.ts @@ -0,0 +1,3 @@ +type ShouldUseEmojiPickerSelection = () => boolean; + +export default ShouldUseEmojiPickerSelection; From 429726382bc6246b411f8ba8220bd4a1edaf7f33 Mon Sep 17 00:00:00 2001 From: MrRefactor Date: Mon, 4 Mar 2024 16:36:52 +0800 Subject: [PATCH 2/3] Review changes --- src/libs/focusComposerWithDelay/index.ts | 21 +++++-------------- .../setTextInputSelection.ts | 15 +++++++++++++ src/libs/focusComposerWithDelay/types.ts | 6 +++++- 3 files changed, 25 insertions(+), 17 deletions(-) create mode 100644 src/libs/focusComposerWithDelay/setTextInputSelection.ts diff --git a/src/libs/focusComposerWithDelay/index.ts b/src/libs/focusComposerWithDelay/index.ts index e1737ea3e6dd..71b420043d08 100644 --- a/src/libs/focusComposerWithDelay/index.ts +++ b/src/libs/focusComposerWithDelay/index.ts @@ -1,15 +1,12 @@ -import type {TextInput} from 'react-native'; import ComposerFocusManager from '@libs/ComposerFocusManager'; -import shouldSetSelectionRange from '@libs/shouldSetSelectionRange'; import * as EmojiPickerAction from '@userActions/EmojiPickerAction'; -import type {FocusComposerWithDelay} from './types'; - -const setSelectionRange = shouldSetSelectionRange(); +import setTextInputSelection from './setTextInputSelection'; +import type {FocusComposerWithDelay, InputType} from './types'; /** * Create a function that focuses the composer. */ -function focusComposerWithDelay(textInput: TextInput | HTMLTextAreaElement | null): FocusComposerWithDelay { +function focusComposerWithDelay(textInput: InputType | null): FocusComposerWithDelay { /** * Focus the text input * @param [shouldDelay] Impose delay before focusing the text input @@ -25,11 +22,7 @@ function focusComposerWithDelay(textInput: TextInput | HTMLTextAreaElement | nul if (!shouldDelay) { textInput.focus(); if (forceSetSelection) { - if (setSelectionRange) { - (textInput as HTMLTextAreaElement).setSelectionRange(forceSetSelection.start, forceSetSelection.end); - } else { - (textInput as TextInput).setSelection(forceSetSelection.start, forceSetSelection.end); - } + setTextInputSelection(textInput, forceSetSelection); } return; } @@ -39,11 +32,7 @@ function focusComposerWithDelay(textInput: TextInput | HTMLTextAreaElement | nul } textInput.focus(); if (forceSetSelection) { - if (setSelectionRange) { - (textInput as HTMLTextAreaElement).setSelectionRange(forceSetSelection.start, forceSetSelection.end); - } else { - (textInput as TextInput).setSelection(forceSetSelection.start, forceSetSelection.end); - } + setTextInputSelection(textInput, forceSetSelection); } }); }; diff --git a/src/libs/focusComposerWithDelay/setTextInputSelection.ts b/src/libs/focusComposerWithDelay/setTextInputSelection.ts new file mode 100644 index 000000000000..506e5b710793 --- /dev/null +++ b/src/libs/focusComposerWithDelay/setTextInputSelection.ts @@ -0,0 +1,15 @@ +import type {TextInput} from 'react-native'; +import shouldSetSelectionRange from '@libs/shouldSetSelectionRange'; +import type {InputType, Selection} from './types'; + +const setSelectionRange = shouldSetSelectionRange(); + +const setTextInputSelection = (textInput: InputType, forceSetSelection: Selection) => { + if (setSelectionRange) { + (textInput as HTMLTextAreaElement).setSelectionRange(forceSetSelection.start, forceSetSelection.end); + } else { + (textInput as TextInput).setSelection(forceSetSelection.start, forceSetSelection.end); + } +}; + +export default setTextInputSelection; diff --git a/src/libs/focusComposerWithDelay/types.ts b/src/libs/focusComposerWithDelay/types.ts index 86e9b0f3ac9a..585aa892cec2 100644 --- a/src/libs/focusComposerWithDelay/types.ts +++ b/src/libs/focusComposerWithDelay/types.ts @@ -1,3 +1,5 @@ +import type {TextInput} from 'react-native'; + type Selection = { start: number; end: number; @@ -5,4 +7,6 @@ type Selection = { type FocusComposerWithDelay = (shouldDelay?: boolean, forceSetSelection?: Selection) => void; -export type {Selection, FocusComposerWithDelay}; +type InputType = TextInput | HTMLTextAreaElement; + +export type {Selection, FocusComposerWithDelay, InputType}; From b09a9618905b5ef6736207fa7c3ef23d38a194eb Mon Sep 17 00:00:00 2001 From: MrRefactor Date: Mon, 18 Mar 2024 15:58:54 +0800 Subject: [PATCH 3/3] Review changes --- src/libs/focusComposerWithDelay/index.ts | 12 ++++++------ .../focusComposerWithDelay/setTextInputSelection.ts | 6 +++--- src/libs/focusComposerWithDelay/types.ts | 2 +- .../home/report/ReportActionItemMessageEdit.tsx | 13 +++++-------- 4 files changed, 15 insertions(+), 18 deletions(-) diff --git a/src/libs/focusComposerWithDelay/index.ts b/src/libs/focusComposerWithDelay/index.ts index 71b420043d08..75e8f6ca8a67 100644 --- a/src/libs/focusComposerWithDelay/index.ts +++ b/src/libs/focusComposerWithDelay/index.ts @@ -10,9 +10,9 @@ function focusComposerWithDelay(textInput: InputType | null): FocusComposerWithD /** * Focus the text input * @param [shouldDelay] Impose delay before focusing the text input - * @param [forceSetSelection] Force selection range of text input + * @param [forcedSelectionRange] Force selection range of text input */ - return (shouldDelay = false, forceSetSelection = undefined) => { + return (shouldDelay = false, forcedSelectionRange = undefined) => { // There could be other animations running while we trigger manual focus. // This prevents focus from making those animations janky. if (!textInput || EmojiPickerAction.isEmojiPickerVisible()) { @@ -21,8 +21,8 @@ function focusComposerWithDelay(textInput: InputType | null): FocusComposerWithD if (!shouldDelay) { textInput.focus(); - if (forceSetSelection) { - setTextInputSelection(textInput, forceSetSelection); + if (forcedSelectionRange) { + setTextInputSelection(textInput, forcedSelectionRange); } return; } @@ -31,8 +31,8 @@ function focusComposerWithDelay(textInput: InputType | null): FocusComposerWithD return; } textInput.focus(); - if (forceSetSelection) { - setTextInputSelection(textInput, forceSetSelection); + if (forcedSelectionRange) { + setTextInputSelection(textInput, forcedSelectionRange); } }); }; diff --git a/src/libs/focusComposerWithDelay/setTextInputSelection.ts b/src/libs/focusComposerWithDelay/setTextInputSelection.ts index 506e5b710793..fbbc4738702f 100644 --- a/src/libs/focusComposerWithDelay/setTextInputSelection.ts +++ b/src/libs/focusComposerWithDelay/setTextInputSelection.ts @@ -4,11 +4,11 @@ import type {InputType, Selection} from './types'; const setSelectionRange = shouldSetSelectionRange(); -const setTextInputSelection = (textInput: InputType, forceSetSelection: Selection) => { +const setTextInputSelection = (textInput: InputType, forcedSelectionRange: Selection) => { if (setSelectionRange) { - (textInput as HTMLTextAreaElement).setSelectionRange(forceSetSelection.start, forceSetSelection.end); + (textInput as HTMLTextAreaElement).setSelectionRange(forcedSelectionRange.start, forcedSelectionRange.end); } else { - (textInput as TextInput).setSelection(forceSetSelection.start, forceSetSelection.end); + (textInput as TextInput).setSelection(forcedSelectionRange.start, forcedSelectionRange.end); } }; diff --git a/src/libs/focusComposerWithDelay/types.ts b/src/libs/focusComposerWithDelay/types.ts index 585aa892cec2..4cd2f785f2bc 100644 --- a/src/libs/focusComposerWithDelay/types.ts +++ b/src/libs/focusComposerWithDelay/types.ts @@ -5,7 +5,7 @@ type Selection = { end: number; }; -type FocusComposerWithDelay = (shouldDelay?: boolean, forceSetSelection?: Selection) => void; +type FocusComposerWithDelay = (shouldDelay?: boolean, forcedSelectionRange?: Selection) => void; type InputType = TextInput | HTMLTextAreaElement; diff --git a/src/pages/home/report/ReportActionItemMessageEdit.tsx b/src/pages/home/report/ReportActionItemMessageEdit.tsx index 0b4ed49bb3d3..79db22d83ee1 100644 --- a/src/pages/home/report/ReportActionItemMessageEdit.tsx +++ b/src/pages/home/report/ReportActionItemMessageEdit.tsx @@ -69,6 +69,7 @@ const emojiButtonID = 'emojiButton'; const messageEditInput = 'messageEditInput'; const isMobileSafari = Browser.isMobileSafari(); +const shouldUseForcedSelectionRange = shouldUseEmojiPickerSelection(); function ReportActionItemMessageEdit( {action, draftMessage, reportID, index, shouldDisableEmojiPicker = false, preferredSkinTone = CONST.EMOJI_DEFAULT_SKIN_TONE}: ReportActionItemMessageEditProps, @@ -330,7 +331,6 @@ function ReportActionItemMessageEdit( deleteDraft(); }, [action, debouncedSaveDraft, deleteDraft, draft, reportID]); - const shouldUseEmojiPickerSelectionRef = shouldUseEmojiPickerSelection(); /** * @param emoji */ @@ -341,9 +341,9 @@ function ReportActionItemMessageEdit( }; setSelection(newSelection); - if (shouldUseEmojiPickerSelectionRef) { - // immediately set the selection again on android and Chrome mobile after focusing the - // input which seems to change the cursor position for a brief moment + if (shouldUseForcedSelectionRange) { + // On Android and Chrome mobile, focusing the input sets the cursor position back to the start. + // To fix this, immediately set the selection again after focusing the input. emojiPickerSelectionRef.current = newSelection; } updateDraft(ComposerUtils.insertText(draft, selection, `${emoji} `)); @@ -460,10 +460,7 @@ function ReportActionItemMessageEdit( { - const emojiPickerSelection = emojiPickerSelectionRef.current ? {...emojiPickerSelectionRef.current} : undefined; - emojiPickerSelectionRef.current = undefined; - - focus(true, emojiPickerSelection); + focus(true, emojiPickerSelectionRef.current ? {...emojiPickerSelectionRef.current} : undefined); }} onEmojiSelected={addEmojiToTextBox} id={emojiButtonID}