From 7b2030bceaa67789f330b3838c07505263169821 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Sat, 17 Nov 2018 01:52:50 -0800 Subject: [PATCH] Fix padding for Text Views in Fabric Summary: This diff extends Text View to support not only paddingLeft, paddingRight, etc but also padding props. Reviewed By: shergin Differential Revision: D13111433 fbshipit-source-id: 3b0efe8468f20a5ffaf31e3f1f180e96a5409865 --- .../react/views/text/ReactTextShadowNode.java | 124 ++++++++---------- .../react/views/text/TextAttributeProps.java | 23 ++-- .../react/views/text/TextLayoutManager.java | 23 +--- 3 files changed, 79 insertions(+), 91 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 8b1ba78d1..98d4e02e1 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 @@ -1,13 +1,11 @@ /** * Copyright (c) Facebook, Inc. and its affiliates. * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. + *

This source code is licensed under the MIT license found in the LICENSE file in the root + * directory of this source tree. */ - package com.facebook.react.views.text; -import android.graphics.Rect; import android.os.Build; import android.text.BoringLayout; import android.text.Layout; @@ -15,15 +13,12 @@ import android.text.Spannable; import android.text.Spanned; import android.text.StaticLayout; import android.text.TextPaint; -import android.util.DisplayMetrics; import android.view.Gravity; import android.widget.TextView; import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.Arguments; import com.facebook.react.bridge.WritableArray; import com.facebook.react.bridge.WritableMap; -import com.facebook.react.uimanager.LayoutShadowNode; -import com.facebook.react.uimanager.ReactShadowNodeImpl; import com.facebook.react.uimanager.Spacing; import com.facebook.react.uimanager.UIViewOperationQueue; import com.facebook.react.uimanager.annotations.ReactProp; @@ -62,16 +57,17 @@ public class ReactTextShadowNode extends ReactBaseTextShadowNode { YogaMeasureMode widthMode, float height, YogaMeasureMode heightMode) { + // TODO(5578671): Handle text direction (see View#getTextDirectionHeuristic) TextPaint textPaint = sTextPaintInstance; textPaint.setTextSize(mFontSize != UNSET ? mFontSize : getDefaultFontSize()); Layout layout; - Spanned text = Assertions.assertNotNull( - mPreparedSpannableText, - "Spannable element has not been prepared in onBeforeLayout"); + Spanned text = + Assertions.assertNotNull( + mPreparedSpannableText, + "Spannable element has not been prepared in onBeforeLayout"); BoringLayout.Metrics boring = BoringLayout.isBoring(text, textPaint); - float desiredWidth = boring == null ? - Layout.getDesiredWidth(text, textPaint) : Float.NaN; + float desiredWidth = boring == null ? Layout.getDesiredWidth(text, textPaint) : Float.NaN; // technically, width should never be negative, but there is currently a bug in boolean unconstrainedWidth = widthMode == YogaMeasureMode.UNDEFINED || width < 0; @@ -89,70 +85,64 @@ public class ReactTextShadowNode extends ReactBaseTextShadowNode { break; } - if (boring == null && - (unconstrainedWidth || - (!YogaConstants.isUndefined(desiredWidth) && desiredWidth <= width))) { + if (boring == null + && (unconstrainedWidth + || (!YogaConstants.isUndefined(desiredWidth) && desiredWidth <= width))) { // Is used when the width is not known and the text is not boring, ie. if it contains // unicode characters. int hintWidth = (int) Math.ceil(desiredWidth); if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { - layout = new StaticLayout( - text, - textPaint, - hintWidth, - alignment, - 1.f, - 0.f, - mIncludeFontPadding); + layout = + new StaticLayout( + text, textPaint, hintWidth, alignment, 1.f, 0.f, mIncludeFontPadding); } else { - layout = StaticLayout.Builder.obtain(text, 0, text.length(), textPaint, hintWidth) - .setAlignment(alignment) - .setLineSpacing(0.f, 1.f) - .setIncludePad(mIncludeFontPadding) - .setBreakStrategy(mTextBreakStrategy) - .setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NORMAL) - .build(); + layout = + StaticLayout.Builder.obtain(text, 0, text.length(), textPaint, hintWidth) + .setAlignment(alignment) + .setLineSpacing(0.f, 1.f) + .setIncludePad(mIncludeFontPadding) + .setBreakStrategy(mTextBreakStrategy) + .setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NORMAL) + .build(); } } else if (boring != null && (unconstrainedWidth || boring.width <= width)) { // Is used for single-line, boring text when the width is either unknown or bigger // than the width of the text. - layout = BoringLayout.make( - text, - textPaint, - boring.width, - alignment, - 1.f, - 0.f, - boring, - mIncludeFontPadding); + layout = + BoringLayout.make( + text, + textPaint, + boring.width, + alignment, + 1.f, + 0.f, + boring, + mIncludeFontPadding); } else { // Is used for multiline, boring text and the width is known. if (Build.VERSION.SDK_INT < Build.VERSION_CODES.M) { - layout = new StaticLayout( - text, - textPaint, - (int) width, - alignment, - 1.f, - 0.f, - mIncludeFontPadding); + layout = + new StaticLayout( + text, textPaint, (int) width, alignment, 1.f, 0.f, mIncludeFontPadding); } else { - layout = StaticLayout.Builder.obtain(text, 0, text.length(), textPaint, (int) width) - .setAlignment(alignment) - .setLineSpacing(0.f, 1.f) - .setIncludePad(mIncludeFontPadding) - .setBreakStrategy(mTextBreakStrategy) - .setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NORMAL) - .build(); + layout = + StaticLayout.Builder.obtain(text, 0, text.length(), textPaint, (int) width) + .setAlignment(alignment) + .setLineSpacing(0.f, 1.f) + .setIncludePad(mIncludeFontPadding) + .setBreakStrategy(mTextBreakStrategy) + .setHyphenationFrequency(Layout.HYPHENATION_FREQUENCY_NORMAL) + .build(); } } if (mShouldNotifyOnTextLayout) { WritableArray lines = - FontMetricsUtil.getFontMetrics(text, layout, sTextPaintInstance, getThemedContext()); + FontMetricsUtil.getFontMetrics( + text, layout, sTextPaintInstance, getThemedContext()); WritableMap event = Arguments.createMap(); event.putArray("lines", lines); getThemedContext() @@ -161,7 +151,8 @@ public class ReactTextShadowNode extends ReactBaseTextShadowNode { } if (mNumberOfLines != UNSET && mNumberOfLines < layout.getLineCount()) { - return YogaMeasureOutput.make(layout.getWidth(), layout.getLineBottom(mNumberOfLines - 1)); + return YogaMeasureOutput.make( + layout.getWidth(), layout.getLineBottom(mNumberOfLines - 1)); } else { return YogaMeasureOutput.make(layout.getWidth(), layout.getHeight()); } @@ -215,17 +206,16 @@ public class ReactTextShadowNode extends ReactBaseTextShadowNode { if (mPreparedSpannableText != null) { ReactTextUpdate reactTextUpdate = - new ReactTextUpdate( - mPreparedSpannableText, - UNSET, - mContainsImages, - getPadding(Spacing.START), - getPadding(Spacing.TOP), - getPadding(Spacing.END), - getPadding(Spacing.BOTTOM), - getTextAlign(), - mTextBreakStrategy - ); + new ReactTextUpdate( + mPreparedSpannableText, + UNSET, + mContainsImages, + getPadding(Spacing.START), + getPadding(Spacing.TOP), + getPadding(Spacing.END), + getPadding(Spacing.BOTTOM), + getTextAlign(), + mTextBreakStrategy); uiViewOperationQueue.enqueueUpdateExtraData(getReactTag(), reactTextUpdate); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java index fdb7bb5cb..cbf49e34d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java @@ -384,33 +384,40 @@ public class TextAttributeProps { : -1; } - //TODO remove this from here + //TODO T31905686 remove this from here and add support to RTL private YogaDirection getLayoutDirection() { return YogaDirection.LTR; } public float getBottomPadding() { - // TODO convert into constants - return getFloatProp("bottomPadding", 0f); + return getPaddingProp(ViewProps.PADDING_BOTTOM); } public float getLeftPadding() { - return getFloatProp("leftPadding", 0f); + return getPaddingProp(ViewProps.PADDING_LEFT); } public float getStartPadding() { - return getFloatProp("startPadding", 0f); + return getPaddingProp(ViewProps.PADDING_START); } public float getEndPadding() { - return getFloatProp("endPadding", 0f); + return getPaddingProp(ViewProps.PADDING_END); } public float getTopPadding() { - return getFloatProp("topPadding", 0f); + return getPaddingProp(ViewProps.PADDING_TOP); } public float getRightPadding() { - return getFloatProp("rightPadding", 0f); + return getPaddingProp(ViewProps.PADDING_RIGHT); + } + + private float getPaddingProp(String paddingType) { + if (mProps.hasKey(ViewProps.PADDING)) { + return PixelUtil.toPixelFromDIP(getFloatProp(ViewProps.PADDING, 0f)); + } + + return PixelUtil.toPixelFromDIP(getFloatProp(paddingType, 0f)); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java index 69990f087..27b865279 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java @@ -29,8 +29,10 @@ import com.facebook.react.bridge.ReadableMap; import com.facebook.react.bridge.ReadableNativeMap; import com.facebook.react.uimanager.PixelUtil; import com.facebook.react.uimanager.ReactStylesDiffMap; +import com.facebook.react.uimanager.ViewDefaults; import com.facebook.yoga.YogaConstants; import com.facebook.yoga.YogaMeasureMode; +import java.awt.font.TextAttribute; import java.util.ArrayList; import java.util.List; @@ -95,11 +97,9 @@ public class TextLayoutManager { new CustomLetterSpacingSpan(textAttributes.mLetterSpacing))); } } - if (textAttributes.mFontSize != UNSET) { - ops.add( - new SetSpanOperation( - start, end, new AbsoluteSizeSpan((int) (textAttributes.mFontSize)))); - } + ops.add( + new SetSpanOperation( + start, end, new AbsoluteSizeSpan(textAttributes.mFontSize))); if (textAttributes.mFontStyle != UNSET || textAttributes.mFontWeight != UNSET || textAttributes.mFontFamily != null) { @@ -163,23 +163,14 @@ public class TextLayoutManager { buildSpannedFromShadowNode(context, fragments, sb, ops); - // TODO: add support for AllowScaling in C++ -// if (textShadowNode.mFontSize == UNSET) { -// 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))); -// } -// +// TODO T31905686: add support for inline Images // textShadowNode.mContainsImages = false; // textShadowNode.mHeightOfTallestInlineImage = Float.NaN; // While setting the Spans on the final text, we also check whether any of them are images. int priority = 0; for (SetSpanOperation op : ops) { -// TODO: add support for TextInlineImage in C++ +// TODO T31905686: add support for TextInlineImage in C++ // if (op.what instanceof TextInlineImageSpan) { // int height = ((TextInlineImageSpan) op.what).getHeight(); // textShadowNode.mContainsImages = true;