Skip to content

Commit

Permalink
Minimize EditText Spans 8/9: CustomStyleSpan (facebook#36577)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: facebook#36577

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]( facebook#35936 (comment)) for greater context on the platform behavior.

This change allows us to strip CustomStyleSpan. We already set all but `fontVariant` on the underlying EditText, so we just need to route that through as well.

Note that because this span is non-parcelable, it is seemingly not subject to the buggy behavior on Samsung devices of infinitely cloning the spans, but non-parcelable spans have different issues on the devices (they disappear), so moving `fontVariant` to the top-level makes sense here.

Changelog:
[Android][Fixed] - Minimize EditText Spans 8/N: CustomStyleSpan

Differential Revision: D44297384

fbshipit-source-id: 4c767a6448481e8976b8d3cc40fd7f9712c3828f
  • Loading branch information
NickGerleman authored and facebook-github-bot committed Mar 22, 2023
1 parent ef403c2 commit 0158b1c
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ public int getWeight() {
return mFontFamily;
}

public @Nullable String getFontFeatureSettings() {
return mFeatureSettings;
}

private static void apply(
Paint paint,
int style,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,15 @@ public void setFontStyle(String fontStyleString) {
}
}

@Override
public void setFontFeatureSettings(String fontFeatureSettings) {
if ((fontFeatureSettings == null && getFontFeatureSettings() != null)
|| !fontFeatureSettings.equals(getFontFeatureSettings())) {
super.setFontFeatureSettings(fontFeatureSettings);
mTypefaceDirty = true;
}
}

public void maybeUpdateTypeface() {
if (!mTypefaceDirty) {
return;
Expand All @@ -529,6 +538,17 @@ public void maybeUpdateTypeface() {
ReactTypefaceUtils.applyStyles(
getTypeface(), mFontStyle, mFontWeight, mFontFamily, getContext().getAssets());
setTypeface(newTypeface);

// Match behavior of CustomStyleSpan and enable SUBPIXEL_TEXT_FLAG when setting anything
// nonstandard
if (mFontStyle != UNSET
|| mFontWeight != UNSET
|| mFontFamily != null
|| getFontFeatureSettings() != null) {
setPaintFlags(getPaintFlags() | Paint.SUBPIXEL_TEXT_FLAG);
} else {
setPaintFlags(getPaintFlags() & (~Paint.SUBPIXEL_TEXT_FLAG));
}
}

// VisibleForTesting from {@link TextInputEventsTestCase}.
Expand Down Expand Up @@ -738,6 +758,21 @@ public boolean test(CustomLetterSpacingSpan span) {
}
});
}

stripSpansOfKind(
sb,
CustomStyleSpan.class,
new SpanPredicate<CustomStyleSpan>() {
@Override
public boolean test(CustomStyleSpan span) {
return span.getStyle() == mFontStyle
&& ((span.getFontFamily() == null && mFontFamily == null)
|| (span.getFontFamily().equals(mFontFamily)))
&& span.getWeight() == mFontWeight
&& ((span.getFontFeatureSettings() == null && getFontFeatureSettings() == null)
|| (span.getFontFeatureSettings().equals(getFontFeatureSettings())));
}
});
}

private <T> void stripSpansOfKind(
Expand Down Expand Up @@ -808,6 +843,22 @@ private void restoreStyleEquivalentSpans(SpannableStringBuilder workingText) {
spanFlags);
}
}

if (mFontStyle != UNSET
|| mFontWeight != UNSET
|| mFontFamily != null
|| getFontFeatureSettings() != null) {
workingText.setSpan(
new CustomStyleSpan(
mFontStyle,
mFontWeight,
getFontFeatureSettings(),
mFontFamily,
getContext().getAssets()),
0,
workingText.length(),
spanFlags);
}
}

private static boolean sameTextForSpan(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import com.facebook.react.views.text.ReactBaseTextShadowNode;
import com.facebook.react.views.text.ReactTextUpdate;
import com.facebook.react.views.text.ReactTextViewManagerCallback;
import com.facebook.react.views.text.ReactTypefaceUtils;
import com.facebook.react.views.text.TextAttributeProps;
import com.facebook.react.views.text.TextInlineImageSpan;
import com.facebook.react.views.text.TextLayoutManager;
Expand Down Expand Up @@ -413,6 +414,11 @@ public void setFontStyle(ReactEditText view, @Nullable String fontStyle) {
view.setFontStyle(fontStyle);
}

@ReactProp(name = ViewProps.FONT_VARIANT)
public void setFontVariant(ReactEditText view, @Nullable ReadableArray fontVariant) {
view.setFontFeatureSettings(ReactTypefaceUtils.parseFontVariant(fontVariant));
}

@ReactProp(name = ViewProps.INCLUDE_FONT_PADDING, defaultBoolean = true)
public void setIncludeFontPadding(ReactEditText view, boolean includepad) {
view.setIncludeFontPadding(includepad);
Expand Down

0 comments on commit 0158b1c

Please sign in to comment.