Skip to content

Commit 974d2cb

Browse files
Adam Comellafadils
authored andcommitted
Android: Fix handling of line height with inline images
Summary: This PR was split from a commit originally in facebook#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 facebook#8907 Differential Revision: D3592781 Pulled By: dmmiller fbshipit-source-id: cba6cd86eb4e3abef6a0d7a81f802bdb0958492e
1 parent 2459a50 commit 974d2cb

File tree

7 files changed

+74
-30
lines changed

7 files changed

+74
-30
lines changed

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

Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -190,12 +190,17 @@ protected static Spannable fromTextCSSNode(ReactTextShadowNode textCSSNode) {
190190
}
191191

192192
textCSSNode.mContainsImages = false;
193+
textCSSNode.mHeightOfTallestInlineImage = Float.NaN;
193194

194195
// While setting the Spans on the final text, we also check whether any of them are images
195196
for (int i = ops.size() - 1; i >= 0; i--) {
196197
SetSpanOperation op = ops.get(i);
197198
if (op.what instanceof TextInlineImageSpan) {
199+
int height = ((TextInlineImageSpan)op.what).getHeight();
198200
textCSSNode.mContainsImages = true;
201+
if (Float.isNaN(textCSSNode.mHeightOfTallestInlineImage) || height > textCSSNode.mHeightOfTallestInlineImage) {
202+
textCSSNode.mHeightOfTallestInlineImage = height;
203+
}
199204
}
200205
op.execute(sb);
201206
}
@@ -226,6 +231,14 @@ public void measure(
226231
// technically, width should never be negative, but there is currently a bug in
227232
boolean unconstrainedWidth = widthMode == CSSMeasureMode.UNDEFINED || width < 0;
228233

234+
float effectiveLineHeight = reactCSSNode.getEffectiveLineHeight();
235+
float lineSpacingExtra = 0;
236+
float lineSpacingMultiplier = 1;
237+
if (!Float.isNaN(effectiveLineHeight)) {
238+
lineSpacingExtra = effectiveLineHeight;
239+
lineSpacingMultiplier = 0;
240+
}
241+
229242
if (boring == null &&
230243
(unconstrainedWidth ||
231244
(!CSSConstants.isUndefined(desiredWidth) && desiredWidth <= width))) {
@@ -236,8 +249,8 @@ public void measure(
236249
textPaint,
237250
(int) Math.ceil(desiredWidth),
238251
Layout.Alignment.ALIGN_NORMAL,
239-
1,
240-
0,
252+
lineSpacingMultiplier,
253+
lineSpacingExtra,
241254
true);
242255
} else if (boring != null && (unconstrainedWidth || boring.width <= width)) {
243256
// Is used for single-line, boring text when the width is either unknown or bigger
@@ -247,8 +260,8 @@ public void measure(
247260
textPaint,
248261
boring.width,
249262
Layout.Alignment.ALIGN_NORMAL,
250-
1,
251-
0,
263+
lineSpacingMultiplier,
264+
lineSpacingExtra,
252265
boring,
253266
true);
254267
} else {
@@ -258,8 +271,8 @@ public void measure(
258271
textPaint,
259272
(int) width,
260273
Layout.Alignment.ALIGN_NORMAL,
261-
1,
262-
0,
274+
lineSpacingMultiplier,
275+
lineSpacingExtra,
263276
true);
264277
}
265278

@@ -269,13 +282,6 @@ public void measure(
269282
reactCSSNode.mNumberOfLines < layout.getLineCount()) {
270283
measureOutput.height = layout.getLineBottom(reactCSSNode.mNumberOfLines - 1);
271284
}
272-
if (reactCSSNode.mLineHeight != UNSET) {
273-
int lines = reactCSSNode.mNumberOfLines != UNSET
274-
? Math.min(reactCSSNode.mNumberOfLines, layout.getLineCount())
275-
: layout.getLineCount();
276-
float lineHeight = PixelUtil.toPixelFromSP(reactCSSNode.mLineHeight);
277-
measureOutput.height = lineHeight * lines;
278-
}
279285
}
280286
};
281287

@@ -293,7 +299,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
293299
100 * (fontWeightString.charAt(0) - '0') : -1;
294300
}
295301

296-
private int mLineHeight = UNSET;
302+
private float mLineHeight = Float.NaN;
297303
private boolean mIsColorSet = false;
298304
private int mColor;
299305
private boolean mIsBackgroundColorSet = false;
@@ -340,6 +346,7 @@ private static int parseNumericFontWeight(String fontWeightString) {
340346
private final boolean mIsVirtual;
341347

342348
protected boolean mContainsImages = false;
349+
private float mHeightOfTallestInlineImage = Float.NaN;
343350

344351
public ReactTextShadowNode(boolean isVirtual) {
345352
mIsVirtual = isVirtual;
@@ -348,6 +355,15 @@ public ReactTextShadowNode(boolean isVirtual) {
348355
}
349356
}
350357

358+
// Returns a line height which takes into account the requested line height
359+
// and the height of the inline images.
360+
public float getEffectiveLineHeight() {
361+
boolean useInlineViewHeight = !Float.isNaN(mLineHeight) &&
362+
!Float.isNaN(mHeightOfTallestInlineImage) &&
363+
mHeightOfTallestInlineImage > mLineHeight;
364+
return useInlineViewHeight ? mHeightOfTallestInlineImage : mLineHeight;
365+
}
366+
351367
@Override
352368
public void onBeforeLayout() {
353369
if (mIsVirtual) {
@@ -380,7 +396,7 @@ public void setNumberOfLines(int numberOfLines) {
380396

381397
@ReactProp(name = ViewProps.LINE_HEIGHT, defaultInt = UNSET)
382398
public void setLineHeight(int lineHeight) {
383-
mLineHeight = lineHeight;
399+
mLineHeight = lineHeight == UNSET ? Float.NaN : PixelUtil.toPixelFromSP(lineHeight);
384400
markUpdated();
385401
}
386402

@@ -530,7 +546,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
530546
super.onCollectExtraUpdates(uiViewOperationQueue);
531547
if (mPreparedSpannableText != null) {
532548
ReactTextUpdate reactTextUpdate =
533-
new ReactTextUpdate(mPreparedSpannableText, UNSET, mContainsImages, getPadding());
549+
new ReactTextUpdate(mPreparedSpannableText, UNSET, mContainsImages, getPadding(), getEffectiveLineHeight());
534550
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
535551
}
536552
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,19 +27,22 @@ public class ReactTextUpdate {
2727
private final float mPaddingTop;
2828
private final float mPaddingRight;
2929
private final float mPaddingBottom;
30+
private final float mLineHeight;
3031

3132
public ReactTextUpdate(
3233
Spannable text,
3334
int jsEventCounter,
3435
boolean containsImages,
35-
Spacing padding) {
36+
Spacing padding,
37+
float lineHeight) {
3638
mText = text;
3739
mJsEventCounter = jsEventCounter;
3840
mContainsImages = containsImages;
3941
mPaddingLeft = padding.get(Spacing.LEFT);
4042
mPaddingTop = padding.get(Spacing.TOP);
4143
mPaddingRight = padding.get(Spacing.RIGHT);
4244
mPaddingBottom = padding.get(Spacing.BOTTOM);
45+
mLineHeight = lineHeight;
4346
}
4447

4548
public Spannable getText() {
@@ -69,4 +72,8 @@ public float getPaddingRight() {
6972
public float getPaddingBottom() {
7073
return mPaddingBottom;
7174
}
75+
76+
public float getLineHeight() {
77+
return mLineHeight;
78+
}
7279
}

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import android.view.ViewGroup;
1818
import android.widget.TextView;
1919

20+
import com.facebook.csslayout.FloatUtil;
2021
import com.facebook.react.uimanager.ReactCompoundView;
2122

2223
public class ReactTextView extends TextView implements ReactCompoundView {
@@ -28,6 +29,7 @@ public class ReactTextView extends TextView implements ReactCompoundView {
2829
private int mDefaultGravityHorizontal;
2930
private int mDefaultGravityVertical;
3031
private boolean mTextIsSelectable;
32+
private float mLineHeight = Float.NaN;
3133

3234
public ReactTextView(Context context) {
3335
super(context);
@@ -50,6 +52,16 @@ public void setText(ReactTextUpdate update) {
5052
(int) Math.ceil(update.getPaddingTop()),
5153
(int) Math.ceil(update.getPaddingRight()),
5254
(int) Math.ceil(update.getPaddingBottom()));
55+
56+
float nextLineHeight = update.getLineHeight();
57+
if (!FloatUtil.floatsEqual(mLineHeight, nextLineHeight)) {
58+
mLineHeight = nextLineHeight;
59+
if (Float.isNaN(mLineHeight)) { // NaN will be used if property gets reset
60+
setLineSpacing(0, 1);
61+
} else {
62+
setLineSpacing(mLineHeight, 0);
63+
}
64+
}
5365
}
5466

5567
@Override

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -103,15 +103,6 @@ public void setTextAlignVertical(ReactTextView view, @Nullable String textAlignV
103103
}
104104
}
105105

106-
@ReactProp(name = ViewProps.LINE_HEIGHT, defaultFloat = Float.NaN)
107-
public void setLineHeight(ReactTextView view, float lineHeight) {
108-
if (Float.isNaN(lineHeight)) { // NaN will be used if property gets reset
109-
view.setLineSpacing(0, 1);
110-
} else {
111-
view.setLineSpacing(PixelUtil.toPixelFromSP(lineHeight), 0);
112-
}
113-
}
114-
115106
@ReactProp(name = "selectable")
116107
public void setSelectable(ReactTextView view, boolean isSelectable) {
117108
view.setTextIsSelectable(isSelectable);

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,4 +67,14 @@ public static void possiblyUpdateInlineImageSpans(Spannable spannable, TextView
6767
* Set the textview that will contain this span.
6868
*/
6969
public abstract void setTextView(TextView textView);
70+
71+
/**
72+
* Get the width of the span.
73+
*/
74+
public abstract int getWidth();
75+
76+
/**
77+
* Get the height of the span.
78+
*/
79+
public abstract int getHeight();
7080
}

ReactAndroid/src/main/java/com/facebook/react/views/text/frescosupport/FrescoBasedReactTextInlineImageSpan.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -146,13 +146,21 @@ public void draw(
146146

147147
canvas.save();
148148

149-
int transY = bottom - mDrawable.getBounds().bottom;
150-
151149
// Align to baseline by default
152-
transY -= paint.getFontMetricsInt().descent;
150+
int transY = y - mDrawable.getBounds().bottom;
153151

154152
canvas.translate(x, transY);
155153
mDrawable.draw(canvas);
156154
canvas.restore();
157155
}
156+
157+
@Override
158+
public int getWidth() {
159+
return mWidth;
160+
}
161+
162+
@Override
163+
public int getHeight() {
164+
return mHeight;
165+
}
158166
}

ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {
119119
if (mJsEventCount != UNSET) {
120120
Spannable preparedSpannableText = fromTextCSSNode(this);
121121
ReactTextUpdate reactTextUpdate =
122-
new ReactTextUpdate(preparedSpannableText, mJsEventCount, mContainsImages, getPadding());
122+
new ReactTextUpdate(preparedSpannableText, mJsEventCount, mContainsImages, getPadding(), getEffectiveLineHeight());
123123
uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate);
124124
}
125125
}

0 commit comments

Comments
 (0)