mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Mimimize EditText Spans 9/9: Remove addSpansForMeasurement() (#36575)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/36575 This is part of a series of changes to minimize the number of spans committed to EditText, as a mitigation for platform issues on Samsung devices. See this [GitHub thread]( https://github.com/facebook/react-native/issues/35936#issuecomment-1411437789) for greater context on the platform behavior. D23670779 addedd a previous mechanism to add spans for measurement caching, like we needed to do as part of this change. It is called in more specific cases (e.g. when there is a text hint but no text), but it edits the live EditText spannable instead of the cache copy, and does not handle nested text at all. We are already adding spans back to the input after this, behind everything else, and can replace it with the code we have been adding. Changelog: [Android][Fixed] - Mimimize EditText Spans 9/9: Remove `addSpansForMeasurement()` Reviewed By: javache Differential Revision: D44298159 fbshipit-source-id: 1af44a39de7550b7e66e45db9ebc3523ae9ff002 # Conflicts: # ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java
This commit is contained in:
committed by
Lorenzo Sciandra
parent
62364d00ca
commit
5339e95cb9
@@ -31,8 +31,6 @@ public class ReactTextUpdate {
|
||||
private final int mSelectionEnd;
|
||||
private final int mJustificationMode;
|
||||
|
||||
public boolean mContainsMultipleFragments;
|
||||
|
||||
/**
|
||||
* @deprecated Use a non-deprecated constructor for ReactTextUpdate instead. This one remains
|
||||
* because it's being used by a unit test that isn't currently open source.
|
||||
@@ -142,13 +140,11 @@ public class ReactTextUpdate {
|
||||
int jsEventCounter,
|
||||
int textAlign,
|
||||
int textBreakStrategy,
|
||||
int justificationMode,
|
||||
boolean containsMultipleFragments) {
|
||||
int justificationMode) {
|
||||
|
||||
ReactTextUpdate reactTextUpdate =
|
||||
new ReactTextUpdate(
|
||||
text, jsEventCounter, false, textAlign, textBreakStrategy, justificationMode);
|
||||
reactTextUpdate.mContainsMultipleFragments = containsMultipleFragments;
|
||||
return reactTextUpdate;
|
||||
}
|
||||
|
||||
|
||||
@@ -64,7 +64,6 @@ import com.facebook.react.views.text.TextInlineImageSpan;
|
||||
import com.facebook.react.views.text.TextLayoutManager;
|
||||
import com.facebook.react.views.view.ReactViewBackgroundManager;
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
import java.util.Objects;
|
||||
|
||||
/**
|
||||
@@ -89,7 +88,6 @@ public class ReactEditText extends AppCompatEditText
|
||||
// *TextChanged events should be triggered. This is less expensive than removing the text
|
||||
// listeners and adding them back again after the text change is completed.
|
||||
protected boolean mIsSettingTextFromJS;
|
||||
protected boolean mIsSettingTextFromCacheUpdate = false;
|
||||
private int mDefaultGravityHorizontal;
|
||||
private int mDefaultGravityVertical;
|
||||
|
||||
@@ -370,7 +368,7 @@ public class ReactEditText extends AppCompatEditText
|
||||
}
|
||||
|
||||
super.onSelectionChanged(selStart, selEnd);
|
||||
if (!mIsSettingTextFromCacheUpdate && mSelectionWatcher != null && hasFocus()) {
|
||||
if (mSelectionWatcher != null && hasFocus()) {
|
||||
mSelectionWatcher.onSelectionChanged(selStart, selEnd);
|
||||
}
|
||||
}
|
||||
@@ -580,7 +578,7 @@ public class ReactEditText extends AppCompatEditText
|
||||
SpannableStringBuilder spannableStringBuilder =
|
||||
new SpannableStringBuilder(reactTextUpdate.getText());
|
||||
|
||||
manageSpans(spannableStringBuilder, reactTextUpdate.mContainsMultipleFragments);
|
||||
manageSpans(spannableStringBuilder);
|
||||
stripStyleEquivalentSpans(spannableStringBuilder);
|
||||
|
||||
mContainsImages = reactTextUpdate.containsImages();
|
||||
@@ -609,7 +607,7 @@ public class ReactEditText extends AppCompatEditText
|
||||
}
|
||||
|
||||
// Update cached spans (in Fabric only).
|
||||
updateCachedSpannable(false);
|
||||
updateCachedSpannable();
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -618,8 +616,7 @@ public class ReactEditText extends AppCompatEditText
|
||||
* will adapt to the new text, hence why {@link SpannableStringBuilder#replace} never removes
|
||||
* them.
|
||||
*/
|
||||
private void manageSpans(
|
||||
SpannableStringBuilder spannableStringBuilder, boolean skipAddSpansForMeasurements) {
|
||||
private void manageSpans(SpannableStringBuilder spannableStringBuilder) {
|
||||
Object[] spans = getText().getSpans(0, length(), Object.class);
|
||||
for (int spanIdx = 0; spanIdx < spans.length; spanIdx++) {
|
||||
Object span = spans[spanIdx];
|
||||
@@ -647,13 +644,6 @@ public class ReactEditText extends AppCompatEditText
|
||||
spannableStringBuilder.setSpan(span, spanStart, spanEnd, spanFlags);
|
||||
}
|
||||
}
|
||||
|
||||
// In Fabric only, apply necessary styles to entire span
|
||||
// If the Spannable was constructed from multiple fragments, we don't apply any spans that could
|
||||
// impact the whole Spannable, because that would override "local" styles per-fragment
|
||||
if (!skipAddSpansForMeasurements) {
|
||||
addSpansForMeasurement(getText());
|
||||
}
|
||||
}
|
||||
|
||||
// TODO: Replace with Predicate<T> and lambdas once Java 8 builds in OSS
|
||||
@@ -755,10 +745,10 @@ public class ReactEditText extends AppCompatEditText
|
||||
}
|
||||
|
||||
/**
|
||||
* Copy back styles represented as attributes to the underlying span, for later measurement
|
||||
* outside the ReactEditText.
|
||||
* Copy styles represented as attributes to the underlying span, for later measurement or other
|
||||
* usage outside the ReactEditText.
|
||||
*/
|
||||
private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) {
|
||||
private void addSpansFromStyleAttributes(SpannableStringBuilder workingText) {
|
||||
int spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE;
|
||||
|
||||
// Set all bits for SPAN_PRIORITY so that this span has the highest possible priority
|
||||
@@ -814,6 +804,11 @@ public class ReactEditText extends AppCompatEditText
|
||||
workingText.length(),
|
||||
spanFlags);
|
||||
}
|
||||
|
||||
float lineHeight = mTextAttributes.getEffectiveLineHeight();
|
||||
if (!Float.isNaN(lineHeight)) {
|
||||
workingText.setSpan(new CustomLineHeightSpan(lineHeight), 0, workingText.length(), spanFlags);
|
||||
}
|
||||
}
|
||||
|
||||
private static boolean sameTextForSpan(
|
||||
@@ -832,73 +827,6 @@ public class ReactEditText extends AppCompatEditText
|
||||
return true;
|
||||
}
|
||||
|
||||
// This is hacked in for Fabric. When we delete non-Fabric code, we might be able to simplify or
|
||||
// clean this up a bit.
|
||||
private void addSpansForMeasurement(Spannable spannable) {
|
||||
if (!mFabricViewStateManager.hasStateWrapper()) {
|
||||
return;
|
||||
}
|
||||
|
||||
boolean originalDisableTextDiffing = mDisableTextDiffing;
|
||||
mDisableTextDiffing = true;
|
||||
|
||||
int start = 0;
|
||||
int end = spannable.length();
|
||||
|
||||
// Remove duplicate spans we might add here
|
||||
Object[] spans = spannable.getSpans(0, length(), Object.class);
|
||||
for (Object span : spans) {
|
||||
int spanFlags = spannable.getSpanFlags(span);
|
||||
boolean isInclusive =
|
||||
(spanFlags & Spanned.SPAN_INCLUSIVE_INCLUSIVE) == Spanned.SPAN_INCLUSIVE_INCLUSIVE
|
||||
|| (spanFlags & Spanned.SPAN_INCLUSIVE_EXCLUSIVE) == Spanned.SPAN_INCLUSIVE_EXCLUSIVE;
|
||||
if (isInclusive
|
||||
&& span instanceof ReactSpan
|
||||
&& spannable.getSpanStart(span) == start
|
||||
&& spannable.getSpanEnd(span) == end) {
|
||||
spannable.removeSpan(span);
|
||||
}
|
||||
}
|
||||
|
||||
List<TextLayoutManager.SetSpanOperation> ops = new ArrayList<>();
|
||||
|
||||
if (!Float.isNaN(mTextAttributes.getLetterSpacing())) {
|
||||
ops.add(
|
||||
new TextLayoutManager.SetSpanOperation(
|
||||
start, end, new CustomLetterSpacingSpan(mTextAttributes.getLetterSpacing())));
|
||||
}
|
||||
ops.add(
|
||||
new TextLayoutManager.SetSpanOperation(
|
||||
start, end, new ReactAbsoluteSizeSpan((int) mTextAttributes.getEffectiveFontSize())));
|
||||
if (mFontStyle != UNSET || mFontWeight != UNSET || mFontFamily != null) {
|
||||
ops.add(
|
||||
new TextLayoutManager.SetSpanOperation(
|
||||
start,
|
||||
end,
|
||||
new CustomStyleSpan(
|
||||
mFontStyle,
|
||||
mFontWeight,
|
||||
null, // TODO: do we need to support FontFeatureSettings / fontVariant?
|
||||
mFontFamily,
|
||||
getReactContext(ReactEditText.this).getAssets())));
|
||||
}
|
||||
if (!Float.isNaN(mTextAttributes.getEffectiveLineHeight())) {
|
||||
ops.add(
|
||||
new TextLayoutManager.SetSpanOperation(
|
||||
start, end, new CustomLineHeightSpan(mTextAttributes.getEffectiveLineHeight())));
|
||||
}
|
||||
|
||||
int priority = 0;
|
||||
for (TextLayoutManager.SetSpanOperation op : ops) {
|
||||
// Actual order of calling {@code execute} does NOT matter,
|
||||
// but the {@code priority} DOES matter.
|
||||
op.execute(spannable, priority);
|
||||
priority++;
|
||||
}
|
||||
|
||||
mDisableTextDiffing = originalDisableTextDiffing;
|
||||
}
|
||||
|
||||
protected boolean showSoftKeyboard() {
|
||||
return mInputMethodManager.showSoftInput(this, 0);
|
||||
}
|
||||
@@ -1180,7 +1108,7 @@ public class ReactEditText extends AppCompatEditText
|
||||
* TextLayoutManager.java with some very minor modifications. There's some duplication between
|
||||
* here and TextLayoutManager, so there might be an opportunity for refactor.
|
||||
*/
|
||||
private void updateCachedSpannable(boolean resetStyles) {
|
||||
private void updateCachedSpannable() {
|
||||
// Noops in non-Fabric
|
||||
if (mFabricViewStateManager == null || !mFabricViewStateManager.hasStateWrapper()) {
|
||||
return;
|
||||
@@ -1190,12 +1118,6 @@ public class ReactEditText extends AppCompatEditText
|
||||
return;
|
||||
}
|
||||
|
||||
if (resetStyles) {
|
||||
mIsSettingTextFromCacheUpdate = true;
|
||||
addSpansForMeasurement(getText());
|
||||
mIsSettingTextFromCacheUpdate = false;
|
||||
}
|
||||
|
||||
Editable currentText = getText();
|
||||
boolean haveText = currentText != null && currentText.length() > 0;
|
||||
|
||||
@@ -1238,7 +1160,6 @@ public class ReactEditText extends AppCompatEditText
|
||||
// - android.app.Activity.dispatchKeyEvent (Activity.java:3447)
|
||||
try {
|
||||
sb.append(currentText.subSequence(0, currentText.length()));
|
||||
restoreStyleEquivalentSpans(sb);
|
||||
} catch (IndexOutOfBoundsException e) {
|
||||
ReactSoftExceptionLogger.logSoftException(TAG, e);
|
||||
}
|
||||
@@ -1254,11 +1175,9 @@ public class ReactEditText extends AppCompatEditText
|
||||
// Measure something so we have correct height, even if there's no string.
|
||||
sb.append("I");
|
||||
}
|
||||
|
||||
// Make sure that all text styles are applied when we're measurable the hint or "blank" text
|
||||
addSpansForMeasurement(sb);
|
||||
}
|
||||
|
||||
addSpansFromStyleAttributes(sb);
|
||||
TextLayoutManager.setCachedSpannabledForTag(getId(), sb);
|
||||
}
|
||||
|
||||
@@ -1273,7 +1192,7 @@ public class ReactEditText extends AppCompatEditText
|
||||
private class TextWatcherDelegator implements TextWatcher {
|
||||
@Override
|
||||
public void beforeTextChanged(CharSequence s, int start, int count, int after) {
|
||||
if (!mIsSettingTextFromCacheUpdate && !mIsSettingTextFromJS && mListeners != null) {
|
||||
if (!mIsSettingTextFromJS && mListeners != null) {
|
||||
for (TextWatcher listener : mListeners) {
|
||||
listener.beforeTextChanged(s, start, count, after);
|
||||
}
|
||||
@@ -1287,23 +1206,20 @@ public class ReactEditText extends AppCompatEditText
|
||||
TAG, "onTextChanged[" + getId() + "]: " + s + " " + start + " " + before + " " + count);
|
||||
}
|
||||
|
||||
if (!mIsSettingTextFromCacheUpdate) {
|
||||
if (!mIsSettingTextFromJS && mListeners != null) {
|
||||
for (TextWatcher listener : mListeners) {
|
||||
listener.onTextChanged(s, start, before, count);
|
||||
}
|
||||
if (!mIsSettingTextFromJS && mListeners != null) {
|
||||
for (TextWatcher listener : mListeners) {
|
||||
listener.onTextChanged(s, start, before, count);
|
||||
}
|
||||
|
||||
updateCachedSpannable(
|
||||
!mIsSettingTextFromJS && !mIsSettingTextFromState && start == 0 && before == 0);
|
||||
}
|
||||
|
||||
updateCachedSpannable();
|
||||
|
||||
onContentSizeChange();
|
||||
}
|
||||
|
||||
@Override
|
||||
public void afterTextChanged(Editable s) {
|
||||
if (!mIsSettingTextFromCacheUpdate && !mIsSettingTextFromJS && mListeners != null) {
|
||||
if (!mIsSettingTextFromJS && mListeners != null) {
|
||||
for (TextWatcher listener : mListeners) {
|
||||
listener.afterTextChanged(s);
|
||||
}
|
||||
|
||||
+2
-10
@@ -1339,9 +1339,6 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
|
||||
TextLayoutManager.getOrCreateSpannableForText(
|
||||
view.getContext(), attributedString, mReactTextViewManagerCallback);
|
||||
|
||||
boolean containsMultipleFragments =
|
||||
attributedString.getArray("fragments").toArrayList().size() > 1;
|
||||
|
||||
int textBreakStrategy =
|
||||
TextAttributeProps.getTextBreakStrategy(paragraphAttributes.getString("textBreakStrategy"));
|
||||
|
||||
@@ -1350,8 +1347,7 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
|
||||
state.getInt("mostRecentEventCount"),
|
||||
TextAttributeProps.getTextAlignment(props, TextLayoutManager.isRTL(attributedString)),
|
||||
textBreakStrategy,
|
||||
TextAttributeProps.getJustificationMode(props),
|
||||
containsMultipleFragments);
|
||||
TextAttributeProps.getJustificationMode(props, currentJustificationMode));
|
||||
}
|
||||
|
||||
public Object getReactTextUpdate(ReactEditText view, ReactStylesDiffMap props, MapBuffer state) {
|
||||
@@ -1372,9 +1368,6 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
|
||||
TextLayoutManagerMapBuffer.getOrCreateSpannableForText(
|
||||
view.getContext(), attributedString, mReactTextViewManagerCallback);
|
||||
|
||||
boolean containsMultipleFragments =
|
||||
attributedString.getMapBuffer(TextLayoutManagerMapBuffer.AS_KEY_FRAGMENTS).getCount() > 1;
|
||||
|
||||
int textBreakStrategy =
|
||||
TextAttributeProps.getTextBreakStrategy(
|
||||
paragraphAttributes.getString(TextLayoutManagerMapBuffer.PA_KEY_TEXT_BREAK_STRATEGY));
|
||||
@@ -1385,7 +1378,6 @@ public class ReactTextInputManager extends BaseViewManager<ReactEditText, Layout
|
||||
TextAttributeProps.getTextAlignment(
|
||||
props, TextLayoutManagerMapBuffer.isRTL(attributedString)),
|
||||
textBreakStrategy,
|
||||
TextAttributeProps.getJustificationMode(props),
|
||||
containsMultipleFragments);
|
||||
TextAttributeProps.getJustificationMode(props, currentJustificationMode));
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user