Skip to content

Commit

Permalink
Prior to this change, when you passed text to TextInput via the `va…
Browse files Browse the repository at this point in the history
…lue` 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 80027ce#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`.

Test Plan:
----------

Used a variety of props to style some `TextInputs` (e.g. `textDecorationLine`, `fontSize`, `letterSpacing`). Verified that the `TextInputs` looked the same regardless of whether the text was provided via the `value` prop, children, or both.

Changelog:
----------

[Android] [Fixed] - TextInput: Improve application of styles for `value` prop

Adam Comella
Microsoft Corp.
  • Loading branch information
Adam Comella committed Nov 30, 2018
1 parent 18f3de9 commit df28436
Showing 1 changed file with 7 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,16 @@ public void execute(SpannableStringBuilder sb, int priority) {
private static void buildSpannedFromShadowNode(
ReactBaseTextShadowNode textShadowNode,
SpannableStringBuilder sb,
List<SetSpanOperation> ops) {

int start = sb.length();
List<SetSpanOperation> ops,
int start) {

for (int i = 0, length = textShadowNode.getChildCount(); i < length; i++) {
ReactShadowNode child = textShadowNode.getChildAt(i);

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.
Expand Down Expand Up @@ -201,12 +200,14 @@ protected static Spannable spannedFromShadowNode(
// a new spannable will be wiped out
List<SetSpanOperation> 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();

Expand Down

0 comments on commit df28436

Please sign in to comment.