Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Contributor

@ishpaul777 ishpaul777 Aug 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you clarify why do we have these changes? this might cause regressions on ios Native

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's because setting the cursor imperatively is what's causing the bug and, in general, declarative is how you should use these props. As you mentioned here the bug that made us use imperative handling in the first place doesn't seem to be present anymore.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry but i didn't interpret this from you proposal, i thought we were just going to pass selection prop in and tested that only while checking proposal surprisingly that only seems to working, but since we are removing this also I'll test this more and get back ASAP!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you talking about the changes regarding removing the imperative handling? If so, I think that was my mistake actually, I forgot to mention we should also remove it, sorry about that. I think removing the imperative handling is better since we don't need it anymore, but we can try just passing the prop and not removing the other code, I'm pretty sure both ways work at the moment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so i noticed a bug when randomly putting cursor it adds random spaces in between

Screen.Recording.2025-09-01.at.12.15.57.AM.mov

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok so i checked that this is reproducible on main as well, and only reproducible on Hybrid app so i guess not related to the PR, will continue testing

Copy link
Contributor

@ishpaul777 ishpaul777 Sep 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LorenzoBloedow i have founce another bug while testing this on IOS it seems when adding a emoji from keyboard there is no space added at end while it did add on main

main:

Screen.Recording.2025-09-28.at.1.24.01.AM.mov

this branch:

Screen.Recording.2025-09-28.at.1.26.11.AM.mov

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also it would be very much appreciate if we can track blame history and list all issue for which this imperative handling is added or modified and test each one to make sure its not reproducible anymore, Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any updates? @LorenzoBloedow

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing without removing the imperative handling.

Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import lodashDebounce from 'lodash/debounce';
import type {ForwardedRef, RefObject} from 'react';
import React, {memo, useCallback, useEffect, useImperativeHandle, useMemo, useRef, useState} from 'react';
import type {BlurEvent, LayoutChangeEvent, MeasureInWindowOnSuccessCallback, TextInput, TextInputContentSizeChangeEvent, TextInputKeyPressEvent, TextInputScrollEvent} from 'react-native';
import {DeviceEventEmitter, InteractionManager, NativeModules, StyleSheet, View} from 'react-native';
import {DeviceEventEmitter, NativeModules, StyleSheet, View} from 'react-native';
import {useFocusedInputHandler} from 'react-native-keyboard-controller';
import type {OnyxEntry} from 'react-native-onyx';
import {useAnimatedRef, useSharedValue} from 'react-native-reanimated';
Expand All @@ -27,7 +27,6 @@ import {canSkipTriggerHotkeys, findCommonSuffixLength, insertText, insertWhiteSp
import convertToLTRForComposer from '@libs/convertToLTRForComposer';
import {containsOnlyEmojis, extractEmojis, getAddedEmojis, getPreferredSkinToneIndex, replaceAndExtractEmojis} from '@libs/EmojiUtils';
import focusComposerWithDelay from '@libs/focusComposerWithDelay';
import getPlatform from '@libs/getPlatform';
import {addKeyDownPressListener, removeKeyDownPressListener} from '@libs/KeyboardShortcut/KeyDownPressListener';
import {detectAndRewritePaste} from '@libs/MarkdownLinkHelpers';
import Parser from '@libs/Parser';
Expand All @@ -53,11 +52,6 @@ import type ChildrenProps from '@src/types/utils/ChildrenProps';
// eslint-disable-next-line no-restricted-imports
import findNodeHandle from '@src/utils/findNodeHandle';

type SyncSelection = {
position: number;
value: string;
};

type NewlyAddedChars = {startIndex: number; endIndex: number; diff: string};

type ComposerWithSuggestionsProps = Partial<ChildrenProps> & {
Expand Down Expand Up @@ -160,8 +154,6 @@ type ComposerRef = {

const {RNTextInputReset} = NativeModules;

const isIOSNative = getPlatform() === CONST.PLATFORM.IOS;

/**
* Broadcast that the user is typing. Debounced to limit how often we publish client events.
*/
Expand Down Expand Up @@ -269,9 +261,9 @@ function ComposerWithSuggestions({

const [composerHeight, setComposerHeight] = useState(0);

const textInputRef = useRef<TextInput | null>(null);
const [suggestionPosition, setSuggestionPosition] = useState<number | null>(null);

const syncSelectionWithOnChangeTextRef = useRef<SyncSelection | null>(null);
const textInputRef = useRef<TextInput | null>(null);

// The ref to check whether the comment saving is in progress
const isCommentPendingSaved = useRef(false);
Expand Down Expand Up @@ -415,10 +407,6 @@ function ComposerWithSuggestions({
if (commentValue !== newComment) {
const position = Math.max((selection.end ?? 0) + (newComment.length - commentRef.current.length), cursorPosition ?? 0);

if (commentWithSpaceInserted !== newComment && isIOSNative) {
syncSelectionWithOnChangeTextRef.current = {position, value: newComment};
}

setSelection((prevSelection) => ({
start: position,
end: position,
Expand Down Expand Up @@ -524,25 +512,18 @@ function ComposerWithSuggestions({
[shouldUseNarrowLayout, isKeyboardShown, suggestionsRef, selection.start, includeChronos, handleSendMessage, lastReportAction, reportID, updateComment, selection.end],
);

const onChangeText = useCallback(
(commentValue: string) => {
updateComment(commentValue, true);
useEffect(() => {
if (suggestionPosition === null) {
return;
}

if (isIOSNative && syncSelectionWithOnChangeTextRef.current) {
const positionSnapshot = syncSelectionWithOnChangeTextRef.current.position;
syncSelectionWithOnChangeTextRef.current = null;
textInputRef.current?.setSelection?.(suggestionPosition, suggestionPosition);
}, [suggestionPosition]);

// ensure that selection is set imperatively after all state changes are effective
// eslint-disable-next-line @typescript-eslint/no-deprecated
InteractionManager.runAfterInteractions(() => {
// note: this implementation is only available on non-web RN, thus the wrapping
// 'if' block contains a redundant (since the ref is only used on iOS) platform check
textInputRef.current?.setSelection(positionSnapshot, positionSnapshot);
});
}
},
[updateComment],
);
const onSuggestionSelected = useCallback((suggestion: TextSelection) => {
setSelection(suggestion);
setSuggestionPosition(suggestion.end ?? 0);
}, []);

const onSelectionChange = useCallback(
(e: CustomSelectionChangeEvent) => {
Expand Down Expand Up @@ -827,7 +808,7 @@ function ComposerWithSuggestions({
ref={setTextInputRef}
placeholder={inputPlaceholder}
placeholderTextColor={theme.placeholderText}
onChangeText={onChangeText}
onChangeText={updateComment}
onKeyPress={handleKeyPress}
textAlignVertical="top"
style={[styles.textInputCompose, isComposerFullSize ? styles.textInputFullCompose : styles.textInputCollapseCompose]}
Expand Down Expand Up @@ -865,7 +846,7 @@ function ComposerWithSuggestions({
// Input
value={value}
selection={selection}
setSelection={setSelection}
setSelection={onSuggestionSelected}
resetKeyboardInput={resetKeyboardInput}
/>

Expand Down
Loading