Skip to content

Commit

Permalink
Android: Fix handling of line height with inline images
Browse files Browse the repository at this point in the history
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 #8907

Differential Revision: D3592781

Pulled By: dmmiller

fbshipit-source-id: cba6cd86eb4e3abef6a0d7a81f802bdb0958492e
  • Loading branch information
Adam Comella authored and Facebook Github Bot 8 committed Jul 20, 2016
1 parent c47f745 commit c4ffc7d
Show file tree
Hide file tree
Showing 7 changed files with 74 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,17 @@ protected static Spannable fromTextCSSNode(ReactTextShadowNode textCSSNode) {
}

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);
}
Expand Down Expand Up @@ -226,6 +231,14 @@ public void measure(
// 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))) {
Expand All @@ -236,8 +249,8 @@ public void measure(
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
Expand All @@ -247,8 +260,8 @@ public void measure(
textPaint,
boring.width,
Layout.Alignment.ALIGN_NORMAL,
1,
0,
lineSpacingMultiplier,
lineSpacingExtra,
boring,
true);
} else {
Expand All @@ -258,8 +271,8 @@ public void measure(
textPaint,
(int) width,
Layout.Alignment.ALIGN_NORMAL,
1,
0,
lineSpacingMultiplier,
lineSpacingExtra,
true);
}

Expand All @@ -269,13 +282,6 @@ public void measure(
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;
}
}
};

Expand All @@ -293,7 +299,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
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;
Expand Down Expand Up @@ -340,6 +346,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
private final boolean mIsVirtual;

protected boolean mContainsImages = false;
private float mHeightOfTallestInlineImage = Float.NaN;

public ReactTextShadowNode(boolean isVirtual) {
mIsVirtual = isVirtual;
Expand All @@ -348,6 +355,15 @@ public ReactTextShadowNode(boolean isVirtual) {
}
}

// 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) {
Expand Down Expand Up @@ -380,7 +396,7 @@ public void setNumberOfLines(int numberOfLines) {

@ReactProp(name = ViewProps.LINE_HEIGHT, defaultInt = UNSET)
public void setLineHeight(int lineHeight) {
mLineHeight = lineHeight;
mLineHeight = lineHeight == UNSET ? Float.NaN : PixelUtil.toPixelFromSP(lineHeight);
markUpdated();
}

Expand Down Expand Up @@ -530,7 +546,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,22 @@ 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;
mPaddingLeft = padding.get(Spacing.LEFT);
mPaddingTop = padding.get(Spacing.TOP);
mPaddingRight = padding.get(Spacing.RIGHT);
mPaddingBottom = padding.get(Spacing.BOTTOM);
mLineHeight = lineHeight;
}

public Spannable getText() {
Expand Down Expand Up @@ -69,4 +72,8 @@ public float getPaddingRight() {
public float getPaddingBottom() {
return mPaddingBottom;
}

public float getLineHeight() {
return mLineHeight;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
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 {
Expand All @@ -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);
Expand All @@ -50,6 +52,16 @@ public void setText(ReactTextUpdate update) {
(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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,6 @@ public void setTextAlignVertical(ReactTextView view, @Nullable String textAlignV
}
}

@ReactProp(name = ViewProps.LINE_HEIGHT, defaultFloat = Float.NaN)
public void setLineHeight(ReactTextView view, float lineHeight) {
if (Float.isNaN(lineHeight)) { // NaN will be used if property gets reset
view.setLineSpacing(0, 1);
} else {
view.setLineSpacing(PixelUtil.toPixelFromSP(lineHeight), 0);
}
}

@ReactProp(name = "selectable")
public void setSelectable(ReactTextView view, boolean isSelectable) {
view.setTextIsSelectable(isSelectable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,4 +67,14 @@ public static void possiblyUpdateInlineImageSpans(Spannable spannable, TextView
* Set the textview that will contain this span.
*/
public abstract void setTextView(TextView textView);

/**
* Get the width of the span.
*/
public abstract int getWidth();

/**
* Get the height of the span.
*/
public abstract int getHeight();
}
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,21 @@ public void draw(

canvas.save();

int transY = bottom - mDrawable.getBounds().bottom;

// Align to baseline by default
transY -= paint.getFontMetricsInt().descent;
int transY = y - mDrawable.getBounds().bottom;

canvas.translate(x, transY);
mDrawable.draw(canvas);
canvas.restore();
}

@Override
public int getWidth() {
return mWidth;
}

@Override
public int getHeight() {
return mHeight;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
if (mJsEventCount != UNSET) {
Spannable preparedSpannableText = fromTextCSSNode(this);
ReactTextUpdate reactTextUpdate =
new ReactTextUpdate(preparedSpannableText, mJsEventCount, mContainsImages, getPadding());
new ReactTextUpdate(preparedSpannableText, mJsEventCount, mContainsImages, getPadding(), getEffectiveLineHeight());
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
}
}
Expand Down

0 comments on commit c4ffc7d

Please sign in to comment.