Skip to content

Commit dfc64d5

Browse files
janicduplessiskelset
authored andcommitted
Fix copy / paste menu and simplify controlled text selection on Android (#37424)
Summary: Currently when using a TextInput with a controlled selection prop the Copy / Paste menu is constantly getting dismissed and is impossible to use. This is because Android dismisses it when certain method that affect the input text are called (https://cs.android.com/android/platform/superproject/+/refs/heads/master:frameworks/base/core/java/android/widget/Editor.java;l=1667;drc=7346c436e5a11ce08f6a80dcfeb8ef941ca30176?q=Editor, https://cs.android.com/android/platform/superproject/+/refs/heads/master:frameworks/base/core/java/android/widget/TextView.java;l=6792;drc=7346c436e5a11ce08f6a80dcfeb8ef941ca30176). The solution to fix this is to avoid calling those methods when only the selection changes. I also noticed there are a lot of differences on how selection is handled in old vs new arch and a lot of the selection handling can actually be removed as it is partially the cause of this issue. This implements 2 mitigations to avoid the issue: - Unify selection handling via commands for old arch, like fabric. Selection is currently a prop in the native component, but it is completely ignored in fabric and selection is set using commands. I removed the selection prop from the native component on Android so now it is exclusively handled with commands like it is currently for fabric. This makes it so that when the selection prop changes the native component no longer re-renders which helps mitigate this issue. More specifically for the old arch we no longer handle the `selection` prop in `ReactTextInputShadowNode`, which used to invalidate the shadow node and cause the text to be replaced and the copy / paste menu to close. - Only set placeholder if the text value changed. Calling `EditText.setHint` also causes the copy / paste menu to be dismissed. Fabric will call all props handlers when a single prop changed, so if the `selection` prop changed the `placeholder` prop handler would be called too. To fix this we can check that the value changed before calling `setHint`. ## Changelog: [ANDROID] [FIXED] - Fix copy / paste menu and simplify controlled text selection on Android Pull Request resolved: #37424 Test Plan: Tested on new and old arch in RNTester example. Before: https://github.com/facebook/react-native/assets/2677334/a915b62a-dd79-4adb-9d95-2317780431cf After: https://github.com/facebook/react-native/assets/2677334/0dd475ed-8981-410c-8908-f00998dcc425 Reviewed By: cortinico Differential Revision: D45958425 Pulled By: NickGerleman fbshipit-source-id: 7b90c1270274f6621303efa60b5398b1a49276ca # Conflicts: # packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java
1 parent bab5bab commit dfc64d5

File tree

8 files changed

+24
-146
lines changed

8 files changed

+24
-146
lines changed

packages/react-native/Libraries/Components/TextInput/AndroidTextInputNativeComponent.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,6 @@ export const __INTERNAL_VIEW_CONFIG: PartialViewConfig = {
692692
fontStyle: true,
693693
textShadowOffset: true,
694694
selectionColor: {process: require('../../StyleSheet/processColor').default},
695-
selection: true,
696695
placeholderTextColor: {
697696
process: require('../../StyleSheet/processColor').default,
698697
},

packages/react-native/Libraries/Components/TextInput/TextInput.js

Lines changed: 5 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1066,27 +1066,19 @@ function InternalTextInput(props: Props): React.Node {
10661066
accessibilityState,
10671067
id,
10681068
tabIndex,
1069+
selection: propsSelection,
10691070
...otherProps
10701071
} = props;
10711072

10721073
const inputRef = useRef<null | React.ElementRef<HostComponent<mixed>>>(null);
10731074

1074-
// Android sends a "onTextChanged" event followed by a "onSelectionChanged" event, for
1075-
// the same "most recent event count".
1076-
// For controlled selection, that means that immediately after text is updated,
1077-
// a controlled component will pass in the *previous* selection, even if the controlled
1078-
// component didn't mean to modify the selection at all.
1079-
// Therefore, we ignore selections and pass them through until the selection event has
1080-
// been sent.
1081-
// Note that this mitigation is NOT needed for Fabric.
1082-
// discovered when upgrading react-hooks
10831075
// eslint-disable-next-line react-hooks/exhaustive-deps
1084-
let selection: ?Selection =
1085-
props.selection == null
1076+
const selection: ?Selection =
1077+
propsSelection == null
10861078
? null
10871079
: {
1088-
start: props.selection.start,
1089-
end: props.selection.end ?? props.selection.start,
1080+
start: propsSelection.start,
1081+
end: propsSelection.end ?? propsSelection.start,
10901082
};
10911083

10921084
const [mostRecentEventCount, setMostRecentEventCount] = useState<number>(0);
@@ -1098,12 +1090,6 @@ function InternalTextInput(props: Props): React.Node {
10981090
|}>({selection, mostRecentEventCount});
10991091

11001092
const lastNativeSelection = lastNativeSelectionState.selection;
1101-
const lastNativeSelectionEventCount =
1102-
lastNativeSelectionState.mostRecentEventCount;
1103-
1104-
if (lastNativeSelectionEventCount < mostRecentEventCount) {
1105-
selection = null;
1106-
}
11071093

11081094
let viewCommands;
11091095
if (AndroidTextInputCommands) {
@@ -1503,7 +1489,6 @@ function InternalTextInput(props: Props): React.Node {
15031489
onScroll={_onScroll}
15041490
onSelectionChange={_onSelectionChange}
15051491
placeholder={placeholder}
1506-
selection={selection}
15071492
style={style}
15081493
text={text}
15091494
textBreakStrategy={props.textBreakStrategy}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java

Lines changed: 3 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ public class ReactTextUpdate {
2727
private final float mPaddingBottom;
2828
private final int mTextAlign;
2929
private final int mTextBreakStrategy;
30-
private final int mSelectionStart;
31-
private final int mSelectionEnd;
3230
private final int mJustificationMode;
3331

3432
/**
@@ -55,35 +53,7 @@ public ReactTextUpdate(
5553
paddingBottom,
5654
textAlign,
5755
Layout.BREAK_STRATEGY_HIGH_QUALITY,
58-
Layout.JUSTIFICATION_MODE_NONE,
59-
-1,
60-
-1);
61-
}
62-
63-
public ReactTextUpdate(
64-
Spannable text,
65-
int jsEventCounter,
66-
boolean containsImages,
67-
float paddingStart,
68-
float paddingTop,
69-
float paddingEnd,
70-
float paddingBottom,
71-
int textAlign,
72-
int textBreakStrategy,
73-
int justificationMode) {
74-
this(
75-
text,
76-
jsEventCounter,
77-
containsImages,
78-
paddingStart,
79-
paddingTop,
80-
paddingEnd,
81-
paddingBottom,
82-
textAlign,
83-
textBreakStrategy,
84-
justificationMode,
85-
-1,
86-
-1);
56+
Layout.JUSTIFICATION_MODE_NONE);
8757
}
8858

8959
public ReactTextUpdate(
@@ -103,9 +73,7 @@ public ReactTextUpdate(
10373
UNSET,
10474
textAlign,
10575
textBreakStrategy,
106-
justificationMode,
107-
-1,
108-
-1);
76+
justificationMode);
10977
}
11078

11179
public ReactTextUpdate(
@@ -118,9 +86,7 @@ public ReactTextUpdate(
11886
float paddingBottom,
11987
int textAlign,
12088
int textBreakStrategy,
121-
int justificationMode,
122-
int selectionStart,
123-
int selectionEnd) {
89+
int justificationMode) {
12490
mText = text;
12591
mJsEventCounter = jsEventCounter;
12692
mContainsImages = containsImages;
@@ -130,8 +96,6 @@ public ReactTextUpdate(
13096
mPaddingBottom = paddingBottom;
13197
mTextAlign = textAlign;
13298
mTextBreakStrategy = textBreakStrategy;
133-
mSelectionStart = selectionStart;
134-
mSelectionEnd = selectionEnd;
13599
mJustificationMode = justificationMode;
136100
}
137101

@@ -187,12 +151,4 @@ public int getTextBreakStrategy() {
187151
public int getJustificationMode() {
188152
return mJustificationMode;
189153
}
190-
191-
public int getSelectionStart() {
192-
return mSelectionStart;
193-
}
194-
195-
public int getSelectionEnd() {
196-
return mSelectionEnd;
197-
}
198154
}

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ public class ReactEditText extends AppCompatEditText
116116
private int mFontStyle = UNSET;
117117
private boolean mAutoFocus = false;
118118
private boolean mDidAttachToWindow = false;
119+
private @Nullable String mPlaceholder = null;
119120

120121
private ReactViewBackgroundManager mReactBackgroundManager;
121122

@@ -496,6 +497,13 @@ public void setInputType(int type) {
496497
setKeyListener(mKeyListener);
497498
}
498499

500+
public void setPlaceholder(@Nullable String placeholder) {
501+
if (!Objects.equals(placeholder, mPlaceholder)) {
502+
mPlaceholder = placeholder;
503+
setHint(placeholder);
504+
}
505+
}
506+
499507
public void setFontFamily(String fontFamily) {
500508
mFontFamily = fontFamily;
501509
mTypefaceDirty = true;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -328,21 +328,19 @@ public void receiveCommand(
328328

329329
if (!args.isNull(1)) {
330330
String text = args.getString(1);
331-
reactEditText.maybeSetTextFromJS(
332-
getReactTextUpdate(text, mostRecentEventCount, start, end));
331+
reactEditText.maybeSetTextFromJS(getReactTextUpdate(text, mostRecentEventCount));
333332
}
334333
reactEditText.maybeSetSelection(mostRecentEventCount, start, end);
335334
break;
336335
}
337336
}
338337

339-
private ReactTextUpdate getReactTextUpdate(
340-
String text, int mostRecentEventCount, int start, int end) {
338+
private ReactTextUpdate getReactTextUpdate(String text, int mostRecentEventCount) {
341339
SpannableStringBuilder sb = new SpannableStringBuilder();
342340
sb.append(TextTransform.apply(text, TextTransform.UNSET));
343341

344342
return new ReactTextUpdate(
345-
sb, mostRecentEventCount, false, 0, 0, 0, 0, Gravity.NO_GRAVITY, 0, 0, start, end);
343+
sb, mostRecentEventCount, false, 0, 0, 0, 0, Gravity.NO_GRAVITY, 0, 0);
346344
}
347345

348346
@Override
@@ -373,9 +371,9 @@ public void updateExtraData(ReactEditText view, Object extraData) {
373371

374372
// Ensure that selection is handled correctly on text update
375373
boolean isCurrentSelectionEmpty = view.getSelectionStart() == view.getSelectionEnd();
376-
int selectionStart = update.getSelectionStart();
377-
int selectionEnd = update.getSelectionEnd();
378-
if ((selectionStart == UNSET || selectionEnd == UNSET) && isCurrentSelectionEmpty) {
374+
int selectionStart = UNSET;
375+
int selectionEnd = UNSET;
376+
if (isCurrentSelectionEmpty) {
379377
// if selection is not set by state, shift current selection to ensure constant gap to
380378
// text end
381379
int textLength = view.getText() == null ? 0 : view.getText().length();
@@ -507,7 +505,7 @@ public void setAllowFontScaling(ReactEditText view, boolean allowFontScaling) {
507505

508506
@ReactProp(name = "placeholder")
509507
public void setPlaceholder(ReactEditText view, String placeholder) {
510-
view.setHint(placeholder);
508+
view.setPlaceholder(placeholder);
511509
}
512510

513511
@ReactProp(name = "placeholderTextColor", customType = "Color")

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java

Lines changed: 1 addition & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
import androidx.core.view.ViewCompat;
1818
import com.facebook.common.logging.FLog;
1919
import com.facebook.infer.annotation.Assertions;
20-
import com.facebook.react.bridge.ReadableMap;
2120
import com.facebook.react.common.ReactConstants;
2221
import com.facebook.react.common.annotations.VisibleForTesting;
2322
import com.facebook.react.uimanager.Spacing;
@@ -44,13 +43,10 @@ public class ReactTextInputShadowNode extends ReactBaseTextShadowNode
4443

4544
@VisibleForTesting public static final String PROP_TEXT = "text";
4645
@VisibleForTesting public static final String PROP_PLACEHOLDER = "placeholder";
47-
@VisibleForTesting public static final String PROP_SELECTION = "selection";
4846

4947
// Represents the {@code text} property only, not possible nested content.
5048
private @Nullable String mText = null;
5149
private @Nullable String mPlaceholder = null;
52-
private int mSelectionStart = UNSET;
53-
private int mSelectionEnd = UNSET;
5450

5551
public ReactTextInputShadowNode(
5652
@Nullable ReactTextViewManagerCallback reactTextViewManagerCallback) {
@@ -165,18 +161,6 @@ public void setMostRecentEventCount(int mostRecentEventCount) {
165161
@ReactProp(name = PROP_TEXT)
166162
public void setText(@Nullable String text) {
167163
mText = text;
168-
if (text != null) {
169-
// The selection shouldn't be bigger than the length of the text
170-
if (mSelectionStart > text.length()) {
171-
mSelectionStart = text.length();
172-
}
173-
if (mSelectionEnd > text.length()) {
174-
mSelectionEnd = text.length();
175-
}
176-
} else {
177-
mSelectionStart = UNSET;
178-
mSelectionEnd = UNSET;
179-
}
180164
markUpdated();
181165
}
182166

@@ -194,18 +178,6 @@ public void setPlaceholder(@Nullable String placeholder) {
194178
return mPlaceholder;
195179
}
196180

197-
@ReactProp(name = PROP_SELECTION)
198-
public void setSelection(@Nullable ReadableMap selection) {
199-
mSelectionStart = mSelectionEnd = UNSET;
200-
if (selection == null) return;
201-
202-
if (selection.hasKey("start") && selection.hasKey("end")) {
203-
mSelectionStart = selection.getInt("start");
204-
mSelectionEnd = selection.getInt("end");
205-
markUpdated();
206-
}
207-
}
208-
209181
@Override
210182
public void setTextBreakStrategy(@Nullable String textBreakStrategy) {
211183
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) {
@@ -245,9 +217,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
245217
getPadding(Spacing.BOTTOM),
246218
mTextAlign,
247219
mTextBreakStrategy,
248-
mJustificationMode,
249-
mSelectionStart,
250-
mSelectionEnd);
220+
mJustificationMode);
251221
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
252222
}
253223
}

packages/react-native/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputProps.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,6 @@ AndroidTextInputProps::AndroidTextInputProps(
134134
"selectionColor",
135135
sourceProps.selectionColor,
136136
{})),
137-
selection(CoreFeatures::enablePropIteratorSetter? sourceProps.selection :
138-
convertRawProp(context, rawProps, "selection", sourceProps.selection, {})),
139137
value(CoreFeatures::enablePropIteratorSetter? sourceProps.value : convertRawProp(context, rawProps, "value", sourceProps.value, {})),
140138
defaultValue(CoreFeatures::enablePropIteratorSetter? sourceProps.defaultValue : convertRawProp(context, rawProps,
141139
"defaultValue",
@@ -349,7 +347,6 @@ void AndroidTextInputProps::setProp(
349347
RAW_SET_PROP_SWITCH_CASE_BASIC(placeholderTextColor);
350348
RAW_SET_PROP_SWITCH_CASE_BASIC(secureTextEntry);
351349
RAW_SET_PROP_SWITCH_CASE_BASIC(selectionColor);
352-
RAW_SET_PROP_SWITCH_CASE_BASIC(selection);
353350
RAW_SET_PROP_SWITCH_CASE_BASIC(defaultValue);
354351
RAW_SET_PROP_SWITCH_CASE_BASIC(selectTextOnFocus);
355352
RAW_SET_PROP_SWITCH_CASE_BASIC(submitBehavior);
@@ -449,7 +446,6 @@ folly::dynamic AndroidTextInputProps::getDynamic() const {
449446
props["placeholderTextColor"] = toAndroidRepr(placeholderTextColor);
450447
props["secureTextEntry"] = secureTextEntry;
451448
props["selectionColor"] = toAndroidRepr(selectionColor);
452-
props["selection"] = toDynamic(selection);
453449
props["value"] = value;
454450
props["defaultValue"] = defaultValue;
455451
props["selectTextOnFocus"] = selectTextOnFocus;

packages/react-native/ReactCommon/react/renderer/components/textinput/androidtextinput/react/renderer/components/androidtextinput/AndroidTextInputProps.h

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,32 +27,6 @@
2727
namespace facebook {
2828
namespace react {
2929

30-
struct AndroidTextInputSelectionStruct {
31-
int start;
32-
int end;
33-
};
34-
35-
static inline void fromRawValue(
36-
const PropsParserContext &context,
37-
const RawValue &value,
38-
AndroidTextInputSelectionStruct &result) {
39-
auto map = (butter::map<std::string, RawValue>)value;
40-
41-
auto start = map.find("start");
42-
if (start != map.end()) {
43-
fromRawValue(context, start->second, result.start);
44-
}
45-
auto end = map.find("end");
46-
if (end != map.end()) {
47-
fromRawValue(context, end->second, result.end);
48-
}
49-
}
50-
51-
static inline std::string toString(
52-
const AndroidTextInputSelectionStruct &value) {
53-
return "[Object AndroidTextInputSelectionStruct]";
54-
}
55-
5630
struct AndroidTextInputTextShadowOffsetStruct {
5731
double width;
5832
double height;
@@ -87,13 +61,6 @@ inline folly::dynamic toDynamic(
8761
dynamicValue["height"] = value.height;
8862
return dynamicValue;
8963
}
90-
91-
inline folly::dynamic toDynamic(const AndroidTextInputSelectionStruct &value) {
92-
folly::dynamic dynamicValue = folly::dynamic::object();
93-
dynamicValue["start"] = value.start;
94-
dynamicValue["end"] = value.end;
95-
return dynamicValue;
96-
}
9764
#endif
9865

9966
class AndroidTextInputProps final : public ViewProps, public BaseTextProps {
@@ -138,7 +105,6 @@ class AndroidTextInputProps final : public ViewProps, public BaseTextProps {
138105
SharedColor placeholderTextColor{};
139106
bool secureTextEntry{false};
140107
SharedColor selectionColor{};
141-
AndroidTextInputSelectionStruct selection{};
142108
std::string value{};
143109
std::string defaultValue{};
144110
bool selectTextOnFocus{false};

0 commit comments

Comments
 (0)