Skip to content

Commit 1f912b9

Browse files
Adam Comellafacebook-github-bot
authored andcommitted
Android TextInput: Fix updating of style props (#22994)
Summary: For certain style props, each time any style prop changed, the previous version of the style would remain. For example, if you passed `"underline"` for `textDecorationLine` on a `TextInput` and then later passed `undefined` for `textDecorationLine`, the underline would remain. We solved this problem before in de586bf. The fix was to use `manageSpans` to remove the old spans we added before adding the new spans. However, that fix hardcoded the list of spans to remove. Every span type that was introduced since that commit is affected by this bug: - CustomLetterSpacingSpan - CustomLineHeightSpan - CustomTextTransformSpan - ShadowStyleSpan - StrikethroughSpan - UnderlineSpan - TextInlineImageSpan The reason this bug was reintroduced is `ReactBaseTextShadowNode` is responsible for adding spans and `ReactEditText` is responsible for removing spans. These classes fell out of sync. This fix attempts a more robust solution. Every span that React Native adds to text now implements the `ReactSpan` interface. `manageSpans` deletes all spans that React Native adds by targeting the ones that implement `ReactSpan`. `ReactBaseTextShadowNode.SetSpanOperation` has been updated so that it's a compiler error to add a span that doesn't implement `ReactSpan`. Pull Request resolved: #22994 Differential Revision: D13727580 Pulled By: mdvacca fbshipit-source-id: 07b2eb08832efafb6c2806aa3329f0e9466b09fb
1 parent 3144299 commit 1f912b9

16 files changed

+127
-44
lines changed

ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLetterSpacingSpan.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
* spans affecting font size.
2424
*/
2525
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
26-
public class CustomLetterSpacingSpan extends MetricAffectingSpan {
26+
public class CustomLetterSpacingSpan extends MetricAffectingSpan implements ReactSpan {
2727

2828
private final float mLetterSpacing;
2929

ReactAndroid/src/main/java/com/facebook/react/views/text/CustomLineHeightSpan.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
* We use a custom {@link LineHeightSpan}, because `lineSpacingExtra` is broken. Details here:
1515
* https://github.com/facebook/react-native/issues/7546
1616
*/
17-
public class CustomLineHeightSpan implements LineHeightSpan {
17+
public class CustomLineHeightSpan implements LineHeightSpan, ReactSpan {
1818
private final int mHeight;
1919

2020
CustomLineHeightSpan(float height) {

ReactAndroid/src/main/java/com/facebook/react/views/text/CustomStyleSpan.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
import android.text.TextPaint;
1616
import android.text.style.MetricAffectingSpan;
1717

18-
public class CustomStyleSpan extends MetricAffectingSpan {
18+
public class CustomStyleSpan extends MetricAffectingSpan implements ReactSpan {
1919

2020
/**
2121
* A {@link MetricAffectingSpan} that allows to change the style of the displayed font.

ReactAndroid/src/main/java/com/facebook/react/views/text/CustomTextTransformSpan.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
import android.text.style.ReplacementSpan;
1313
import java.text.BreakIterator;
1414

15-
public class CustomTextTransformSpan extends ReplacementSpan {
15+
public class CustomTextTransformSpan extends ReplacementSpan implements ReactSpan {
1616

1717
/**
1818
* A {@link ReplacementSpan} that allows declarative changing of text casing.
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text;
9+
10+
import android.text.style.AbsoluteSizeSpan;
11+
12+
/*
13+
* Wraps {@link AbsoluteSizeSpan} as a {@link ReactSpan}.
14+
*/
15+
public class ReactAbsoluteSizeSpan extends AbsoluteSizeSpan implements ReactSpan {
16+
public ReactAbsoluteSizeSpan(int size) {
17+
super(size);
18+
}
19+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text;
9+
10+
import android.text.style.BackgroundColorSpan;
11+
12+
/*
13+
* Wraps {@link BackgroundColorSpan} as a {@link ReactSpan}.
14+
*/
15+
public class ReactBackgroundColorSpan extends BackgroundColorSpan implements ReactSpan {
16+
public ReactBackgroundColorSpan(int color) {
17+
super(color);
18+
}
19+
}

ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,13 @@
1212
import android.text.Layout;
1313
import android.text.Spannable;
1414
import android.text.SpannableStringBuilder;
15-
import android.text.style.AbsoluteSizeSpan;
16-
import android.text.style.BackgroundColorSpan;
17-
import android.text.style.ForegroundColorSpan;
18-
import android.text.style.StrikethroughSpan;
19-
import android.text.style.UnderlineSpan;
2015
import android.view.Gravity;
2116
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
2217
import com.facebook.react.bridge.ReadableMap;
2318
import com.facebook.react.uimanager.IllegalViewOperationException;
2419
import com.facebook.react.uimanager.LayoutShadowNode;
2520
import com.facebook.react.uimanager.PixelUtil;
2621
import com.facebook.react.uimanager.ReactShadowNode;
27-
import com.facebook.react.uimanager.UIManagerModule;
28-
import com.facebook.react.uimanager.ViewDefaults;
2922
import com.facebook.react.uimanager.ViewProps;
3023
import com.facebook.react.uimanager.annotations.ReactProp;
3124
import com.facebook.yoga.YogaDirection;
@@ -60,9 +53,9 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {
6053

6154
private static class SetSpanOperation {
6255
protected int start, end;
63-
protected Object what;
56+
protected ReactSpan what;
6457

65-
SetSpanOperation(int start, int end, Object what) {
58+
SetSpanOperation(int start, int end, ReactSpan what) {
6659
this.start = start;
6760
this.end = end;
6861
this.what = what;
@@ -122,12 +115,12 @@ private static void buildSpannedFromShadowNode(
122115
int end = sb.length();
123116
if (end >= start) {
124117
if (textShadowNode.mIsColorSet) {
125-
ops.add(new SetSpanOperation(start, end, new ForegroundColorSpan(textShadowNode.mColor)));
118+
ops.add(new SetSpanOperation(start, end, new ReactForegroundColorSpan(textShadowNode.mColor)));
126119
}
127120
if (textShadowNode.mIsBackgroundColorSet) {
128121
ops.add(
129122
new SetSpanOperation(
130-
start, end, new BackgroundColorSpan(textShadowNode.mBackgroundColor)));
123+
start, end, new ReactBackgroundColorSpan(textShadowNode.mBackgroundColor)));
131124
}
132125
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
133126
float effectiveLetterSpacing = textAttributes.getEffectiveLetterSpacing();
@@ -143,7 +136,7 @@ private static void buildSpannedFromShadowNode(
143136
if (// `getEffectiveFontSize` always returns a value so don't need to check for anything like
144137
// `Float.NaN`.
145138
parentTextAttributes == null || parentTextAttributes.getEffectiveFontSize() != effectiveFontSize) {
146-
ops.add(new SetSpanOperation(start, end, new AbsoluteSizeSpan(effectiveFontSize)));
139+
ops.add(new SetSpanOperation(start, end, new ReactAbsoluteSizeSpan(effectiveFontSize)));
147140
}
148141
if (textShadowNode.mFontStyle != UNSET
149142
|| textShadowNode.mFontWeight != UNSET
@@ -159,10 +152,10 @@ private static void buildSpannedFromShadowNode(
159152
textShadowNode.getThemedContext().getAssets())));
160153
}
161154
if (textShadowNode.mIsUnderlineTextDecorationSet) {
162-
ops.add(new SetSpanOperation(start, end, new UnderlineSpan()));
155+
ops.add(new SetSpanOperation(start, end, new ReactUnderlineSpan()));
163156
}
164157
if (textShadowNode.mIsLineThroughTextDecorationSet) {
165-
ops.add(new SetSpanOperation(start, end, new StrikethroughSpan()));
158+
ops.add(new SetSpanOperation(start, end, new ReactStrikethroughSpan()));
166159
}
167160
if (
168161
(
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text;
9+
10+
import android.text.style.ForegroundColorSpan;
11+
12+
/*
13+
* Wraps {@link ForegroundColorSpan} as a {@link ReactSpan}.
14+
*/
15+
public class ReactForegroundColorSpan extends ForegroundColorSpan implements ReactSpan {
16+
public ReactForegroundColorSpan(int color) {
17+
super(color);
18+
}
19+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text;
9+
10+
/*
11+
* Enables us to distinguish between spans that were added by React Native and spans that were
12+
* added by something else. All spans that React Native adds should implement this interface.
13+
*/
14+
public interface ReactSpan {
15+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.views.text;
9+
10+
import android.text.style.StrikethroughSpan;
11+
12+
/*
13+
* Wraps {@link StrikethroughSpan} as a {@link ReactSpan}.
14+
*/
15+
public class ReactStrikethroughSpan extends StrikethroughSpan implements ReactSpan {
16+
}

0 commit comments

Comments
 (0)