From f6f8b092f9dac03e81cb01a73e748f3df85ea519 Mon Sep 17 00:00:00 2001 From: Adam Comella Date: Tue, 8 Jan 2019 15:56:07 -0800 Subject: [PATCH] Android TextInput: Improve application of styles for `value` prop (#22461) Summary: Prior to this change, when you passed text to `TextInput` via the `value` or `defaultValue` props, React Native didn't apply any of the styles in `buildSpannedFromShadowNode` to the text. This is because `spannedFromShadowNode` appends `value` after calling `buildSpannedFromShadowNode`. Many styles worked because their logic is included in both `buildSpannedFromShadowNode` and `ReactTextInputManager`. However, some only appear in `buildSpannedFromShadowNode` such as `textDecorationLine` (it would be good to understand why we need to duplicate styling logic in `buildSpannedFromShadowNode` & `ReactTextInputManager` and to know whether `ReactTextInputManager` should be handling `textDecorationLine`). Also, this commit improves consistency between iOS and Android if you specify both `value` and children on a `TextInput`. Prior to this, iOS concatenated the strings such that the `value` prop came before the children whereas Android put the children before the `value` prop. Now Android matches iOS's behavior and puts the `value` prop before the children. These appear to be regressions. The `value` prop used to be appended before calling `buildSpannedFromShadowNode` (this behavior appears to have been changed by accident in https://github.com/facebook/react-native/commit/80027ce6dba1b632b080dc17a08b9f904d649217#diff-4f5947f2fe0381c4a6373a30e596b8c3). The fix is to append the `value` prop before calling `buildSpannedFromShadowNode`. Additionally, we have to expose a new `start` parameter on `buildSpannedFromShadowNode` so that we can tell it to include the text from the `value` prop in the range that it styles. Without this, the start of the styled text would be immediately after `value` because `value` is appended before calling `buildSpannedFromShadowNode` Pull Request resolved: https://github.com/facebook/react-native/pull/22461 Reviewed By: mdvacca Differential Revision: D13282065 Pulled By: shergin fbshipit-source-id: 4c99094741441cf54cdec0075bfd08ff7d889e66 --- .../react/views/text/ReactBaseTextShadowNode.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) 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 05091cc32..edd08cf29 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 @@ -85,9 +85,8 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { private static void buildSpannedFromShadowNode( ReactBaseTextShadowNode textShadowNode, SpannableStringBuilder sb, - List ops) { - - int start = sb.length(); + List ops, + int start) { for (int i = 0, length = textShadowNode.getChildCount(); i < length; i++) { ReactShadowNode child = textShadowNode.getChildAt(i); @@ -95,7 +94,7 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { if (child instanceof ReactRawTextShadowNode) { sb.append(((ReactRawTextShadowNode) child).getText()); } else if (child instanceof ReactBaseTextShadowNode) { - buildSpannedFromShadowNode((ReactBaseTextShadowNode) child, sb, ops); + buildSpannedFromShadowNode((ReactBaseTextShadowNode) child, sb, ops, sb.length()); } else if (child instanceof ReactTextInlineImageShadowNode) { // We make the image take up 1 character in the span and put a corresponding character into // the text so that the image doesn't run over any following text. @@ -201,12 +200,14 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode { // a new spannable will be wiped out List ops = new ArrayList<>(); - buildSpannedFromShadowNode(textShadowNode, sb, ops); - if (text != null) { + // Handle text that is provided via a prop (e.g. the `value` and `defaultValue` props on + // TextInput). sb.append(text); } + buildSpannedFromShadowNode(textShadowNode, sb, ops, 0); + if (textShadowNode.mFontSize == UNSET) { int defaultFontSize = textShadowNode.getDefaultFontSize();