From c4ffc7d71c1c34599d3dd303e0b5bb674fa691f5 Mon Sep 17 00:00:00 2001 From: Adam Comella Date: Wed, 20 Jul 2016 07:12:48 -0700 Subject: [PATCH] Android: Fix handling of line height with inline images Summary: This PR was split from a commit originally in #8619. /cc dmmiller When an inline image was larger than the specified line height, the image would be clipped. This changes the behavior so that the line height is changed to make room for the inline image. This is consistent with the behavior of RN for iOS. Here's how the change works. ReactTextView now receives its line height from the layout thread rather than directly from JavaScript. The reason is that the layout thread may pick a different line height. In the case that the tallest inline image is larger than the line height supplied by JavaScript, we want to use that image's height as the line height rather than the supplied line height. Also fixed a bug where the image, which is supposed to be baseline aligned, would be positioned at the wrong y location. To fix this, we use `y` (the baseline) in the `draw` method rather than trying to calculate the baseline from `bottom`. For more information see https://code.google.com/p/andro Closes https://github.com/facebook/react-native/pull/8907 Differential Revision: D3592781 Pulled By: dmmiller fbshipit-source-id: cba6cd86eb4e3abef6a0d7a81f802bdb0958492e --- .../react/views/text/ReactTextShadowNode.java | 48 ++++++++++++------- .../react/views/text/ReactTextUpdate.java | 9 +++- .../react/views/text/ReactTextView.java | 12 +++++ .../views/text/ReactTextViewManager.java | 9 ---- .../react/views/text/TextInlineImageSpan.java | 10 ++++ .../FrescoBasedReactTextInlineImageSpan.java | 14 ++++-- .../textinput/ReactTextInputShadowNode.java | 2 +- 7 files changed, 74 insertions(+), 30 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java index ae623bf0e..5178281a1 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextShadowNode.java @@ -190,12 +190,17 @@ public class ReactTextShadowNode extends LayoutShadowNode { } textCSSNode.mContainsImages = false; + textCSSNode.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); if (op.what instanceof TextInlineImageSpan) { + int height = ((TextInlineImageSpan)op.what).getHeight(); textCSSNode.mContainsImages = true; + if (Float.isNaN(textCSSNode.mHeightOfTallestInlineImage) || height > textCSSNode.mHeightOfTallestInlineImage) { + textCSSNode.mHeightOfTallestInlineImage = height; + } } op.execute(sb); } @@ -226,6 +231,14 @@ public class ReactTextShadowNode extends LayoutShadowNode { // technically, width should never be negative, but there is currently a bug in boolean unconstrainedWidth = widthMode == CSSMeasureMode.UNDEFINED || width < 0; + float effectiveLineHeight = reactCSSNode.getEffectiveLineHeight(); + float lineSpacingExtra = 0; + float lineSpacingMultiplier = 1; + if (!Float.isNaN(effectiveLineHeight)) { + lineSpacingExtra = effectiveLineHeight; + lineSpacingMultiplier = 0; + } + if (boring == null && (unconstrainedWidth || (!CSSConstants.isUndefined(desiredWidth) && desiredWidth <= width))) { @@ -236,8 +249,8 @@ public class ReactTextShadowNode extends LayoutShadowNode { textPaint, (int) Math.ceil(desiredWidth), Layout.Alignment.ALIGN_NORMAL, - 1, - 0, + lineSpacingMultiplier, + lineSpacingExtra, true); } else if (boring != null && (unconstrainedWidth || boring.width <= width)) { // Is used for single-line, boring text when the width is either unknown or bigger @@ -247,8 +260,8 @@ public class ReactTextShadowNode extends LayoutShadowNode { textPaint, boring.width, Layout.Alignment.ALIGN_NORMAL, - 1, - 0, + lineSpacingMultiplier, + lineSpacingExtra, boring, true); } else { @@ -258,8 +271,8 @@ public class ReactTextShadowNode extends LayoutShadowNode { textPaint, (int) width, Layout.Alignment.ALIGN_NORMAL, - 1, - 0, + lineSpacingMultiplier, + lineSpacingExtra, true); } @@ -269,13 +282,6 @@ public class ReactTextShadowNode extends LayoutShadowNode { reactCSSNode.mNumberOfLines < layout.getLineCount()) { measureOutput.height = layout.getLineBottom(reactCSSNode.mNumberOfLines - 1); } - if (reactCSSNode.mLineHeight != UNSET) { - int lines = reactCSSNode.mNumberOfLines != UNSET - ? Math.min(reactCSSNode.mNumberOfLines, layout.getLineCount()) - : layout.getLineCount(); - float lineHeight = PixelUtil.toPixelFromSP(reactCSSNode.mLineHeight); - measureOutput.height = lineHeight * lines; - } } }; @@ -293,7 +299,7 @@ public class ReactTextShadowNode extends LayoutShadowNode { 100 * (fontWeightString.charAt(0) - '0') : -1; } - private int mLineHeight = UNSET; + private float mLineHeight = Float.NaN; private boolean mIsColorSet = false; private int mColor; private boolean mIsBackgroundColorSet = false; @@ -340,6 +346,7 @@ public class ReactTextShadowNode extends LayoutShadowNode { private final boolean mIsVirtual; protected boolean mContainsImages = false; + private float mHeightOfTallestInlineImage = Float.NaN; public ReactTextShadowNode(boolean isVirtual) { mIsVirtual = isVirtual; @@ -348,6 +355,15 @@ public class ReactTextShadowNode extends LayoutShadowNode { } } + // Returns a line height which takes into account the requested line height + // and the height of the inline images. + public float getEffectiveLineHeight() { + boolean useInlineViewHeight = !Float.isNaN(mLineHeight) && + !Float.isNaN(mHeightOfTallestInlineImage) && + mHeightOfTallestInlineImage > mLineHeight; + return useInlineViewHeight ? mHeightOfTallestInlineImage : mLineHeight; + } + @Override public void onBeforeLayout() { if (mIsVirtual) { @@ -380,7 +396,7 @@ public class ReactTextShadowNode extends LayoutShadowNode { @ReactProp(name = ViewProps.LINE_HEIGHT, defaultInt = UNSET) public void setLineHeight(int lineHeight) { - mLineHeight = lineHeight; + mLineHeight = lineHeight == UNSET ? Float.NaN : PixelUtil.toPixelFromSP(lineHeight); markUpdated(); } @@ -530,7 +546,7 @@ public class ReactTextShadowNode extends LayoutShadowNode { super.onCollectExtraUpdates(uiViewOperationQueue); if (mPreparedSpannableText != null) { ReactTextUpdate reactTextUpdate = - new ReactTextUpdate(mPreparedSpannableText, UNSET, mContainsImages, getPadding()); + new ReactTextUpdate(mPreparedSpannableText, UNSET, mContainsImages, getPadding(), getEffectiveLineHeight()); uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java index dca68669d..5acb167c5 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextUpdate.java @@ -27,12 +27,14 @@ public class ReactTextUpdate { private final float mPaddingTop; private final float mPaddingRight; private final float mPaddingBottom; + private final float mLineHeight; public ReactTextUpdate( Spannable text, int jsEventCounter, boolean containsImages, - Spacing padding) { + Spacing padding, + float lineHeight) { mText = text; mJsEventCounter = jsEventCounter; mContainsImages = containsImages; @@ -40,6 +42,7 @@ public class ReactTextUpdate { mPaddingTop = padding.get(Spacing.TOP); mPaddingRight = padding.get(Spacing.RIGHT); mPaddingBottom = padding.get(Spacing.BOTTOM); + mLineHeight = lineHeight; } public Spannable getText() { @@ -69,4 +72,8 @@ public class ReactTextUpdate { public float getPaddingBottom() { return mPaddingBottom; } + + public float getLineHeight() { + return mLineHeight; + } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java index 71bb5ccf1..9ef3df91a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java @@ -17,6 +17,7 @@ import android.view.Gravity; import android.view.ViewGroup; import android.widget.TextView; +import com.facebook.csslayout.FloatUtil; import com.facebook.react.uimanager.ReactCompoundView; public class ReactTextView extends TextView implements ReactCompoundView { @@ -28,6 +29,7 @@ public class ReactTextView extends TextView implements ReactCompoundView { private int mDefaultGravityHorizontal; private int mDefaultGravityVertical; private boolean mTextIsSelectable; + private float mLineHeight = Float.NaN; public ReactTextView(Context context) { super(context); @@ -50,6 +52,16 @@ public class ReactTextView extends TextView implements ReactCompoundView { (int) Math.ceil(update.getPaddingTop()), (int) Math.ceil(update.getPaddingRight()), (int) Math.ceil(update.getPaddingBottom())); + + float nextLineHeight = update.getLineHeight(); + if (!FloatUtil.floatsEqual(mLineHeight, nextLineHeight)) { + mLineHeight = nextLineHeight; + if (Float.isNaN(mLineHeight)) { // NaN will be used if property gets reset + setLineSpacing(0, 1); + } else { + setLineSpacing(mLineHeight, 0); + } + } } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java index 97e780a6d..dc200c1fc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java @@ -103,15 +103,6 @@ public class ReactTextViewManager extends BaseViewManager