From 7efd4fabfde2213faf2ba570ab05ebfd0a57715b Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sun, 17 Sep 2017 21:59:31 -0700 Subject: [PATCH] Text to Spannable conversion is now using PRIORITY flag to enforce the order of spans Summary: When we convert nested components to Spannable object we must enforce the order of spans somehow, otherwise we will have Spannable object with unpredictable order of spans, which will produce unpredictalbe text layout. We can do it only using `Spannable.SPAN_PRIORITY` feature because Spannable objects do not maintain the order of spans internally. We also have to fix this to implement autoexpandable . Reviewed By: achen1 Differential Revision: D5811172 fbshipit-source-id: 5bc68b869e58aba27d6986581af9fe3343d116a7 --- RNTester/js/TextExample.android.js | 21 ++++++++++++ .../views/text/ReactBaseTextShadowNode.java | 34 ++++++++++++------- 2 files changed, 42 insertions(+), 13 deletions(-) diff --git a/RNTester/js/TextExample.android.js b/RNTester/js/TextExample.android.js index b5a61c00e24..17133be48cf 100644 --- a/RNTester/js/TextExample.android.js +++ b/RNTester/js/TextExample.android.js @@ -231,6 +231,27 @@ class TextExample extends React.Component<{}> { console.log('1st')}> (Normal text, + + (R)red + + (G)green + + (B)blue + + (C)cyan + + (M)magenta + + (Y)yellow + + (K)black + + + + + + + console.log('2nd')}> (and bold console.log('3rd')}> diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java index 7dc50f710c6..5b23adab70d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java @@ -65,13 +65,17 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { this.what = what; } - public void execute(SpannableStringBuilder sb) { + public void execute(SpannableStringBuilder sb, int priority) { // All spans will automatically extend to the right of the text, but not the left - except // for spans that start at the beginning of the text. int spanFlags = Spannable.SPAN_EXCLUSIVE_INCLUSIVE; if (start == 0) { spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE; } + + spanFlags &= ~Spannable.SPAN_PRIORITY; + spanFlags |= (priority << Spannable.SPAN_PRIORITY_SHIFT) & Spannable.SPAN_PRIORITY; + sb.setSpan(what, start, end, spanFlags); } } @@ -167,6 +171,7 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { // up-to-bottom, otherwise all the spannables that are withing the region for which one may set // a new spannable will be wiped out List ops = new ArrayList<>(); + buildSpannedFromShadowNode(textShadowNode, sb, ops); if (text != null) { @@ -174,22 +179,20 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { } if (textShadowNode.mFontSize == UNSET) { - sb.setSpan( - new AbsoluteSizeSpan( - textShadowNode.mAllowFontScaling - ? (int) Math.ceil(PixelUtil.toPixelFromSP(ViewDefaults.FONT_SIZE_SP)) - : (int) Math.ceil(PixelUtil.toPixelFromDIP(ViewDefaults.FONT_SIZE_SP))), - 0, - sb.length(), - Spannable.SPAN_INCLUSIVE_EXCLUSIVE); + int defaultFontSize = + textShadowNode.mAllowFontScaling + ? (int) Math.ceil(PixelUtil.toPixelFromSP(ViewDefaults.FONT_SIZE_SP)) + : (int) Math.ceil(PixelUtil.toPixelFromDIP(ViewDefaults.FONT_SIZE_SP)); + + ops.add(new SetSpanOperation(0, sb.length(), new AbsoluteSizeSpan(defaultFontSize))); } textShadowNode.mContainsImages = false; textShadowNode.mHeightOfTallestInlineImage = Float.NaN; - // While setting the Spans on the final text, we also check whether any of them are images - for (int i = ops.size() - 1; i >= 0; i--) { - SetSpanOperation op = ops.get(i); + // While setting the Spans on the final text, we also check whether any of them are images. + int priority = 0; + for (SetSpanOperation op : ops) { if (op.what instanceof TextInlineImageSpan) { int height = ((TextInlineImageSpan) op.what).getHeight(); textShadowNode.mContainsImages = true; @@ -198,8 +201,13 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { textShadowNode.mHeightOfTallestInlineImage = height; } } - op.execute(sb); + + // Actual order of calling {@code execute} does NOT matter, + // but the {@code priority} DOES matter. + op.execute(sb, priority); + priority++; } + return sb; }