From 254ebab1d2b6fac859ab1ae0c9503328fc99a6d0 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Mon, 9 Dec 2019 14:41:36 -0800 Subject: [PATCH] AndroidTextInput: don't set TextInput value to placeholder; use eventCounter prop to send tree updates via state Summary: Fix (1) placeholder (2) modifying TextInput in Fabric on Android. Changelog: [internal] Reviewed By: mdvacca Differential Revision: D18894538 fbshipit-source-id: 21103f56e6f6e108fcf6359a23c9dd9a0323250e --- .../AndroidTextInputShadowNode.cpp | 42 ++++++++++--------- .../AndroidTextInputShadowNode.h | 3 +- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp index 82661d79383..8b117ff1d4f 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp @@ -50,31 +50,23 @@ AttributedString AndroidTextInputShadowNode::getAttributedString() const { fragment.textAttributes.backgroundColor = clearColor(); fragment.parentShadowView = ShadowView(*this); attributedString.prependFragment(fragment); - - // We know this is not empty, because we at least have the `text` value - return attributedString; } - // No need to use placeholder if we have text at this point. - if (!attributedString.isEmpty()) { - return attributedString; - } - - return getPlaceholderAttributedString(false); + return attributedString; } // For measurement purposes, we want to make sure that there's at least a // single character in the string so that the measured height is greater // than zero. Otherwise, empty TextInputs with no placeholder don't // display at all. -AttributedString AndroidTextInputShadowNode::getPlaceholderAttributedString( - bool ensureMinimumLength) const { +AttributedString AndroidTextInputShadowNode::getPlaceholderAttributedString() + const { // Return placeholder text, since text and children are empty. auto textAttributedString = AttributedString{}; auto fragment = AttributedString::Fragment{}; fragment.string = getProps()->placeholder; - if (fragment.string.empty() && ensureMinimumLength) { + if (fragment.string.empty()) { fragment.string = " "; } @@ -123,13 +115,23 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() { auto defaultTextAttributes = TextAttributes::defaultTextAttributes(); defaultTextAttributes.apply(getProps()->textAttributes); - setStateData(AndroidTextInputState{state.mostRecentEventCount, - reactTreeAttributedString, - reactTreeAttributedString, - getProps()->paragraphAttributes, - defaultTextAttributes, - ShadowView(*this), - textLayoutManager_}); + // Even if we're here and updating state, it may be only to update the layout + // manager If that is the case, make sure we don't update text: pass in the + // current attributedString unchanged, and pass in zero for the "event count" + // so no changes are applied There's no way to prevent a state update from + // flowing to Java, so we just ensure it's a noop in those cases. + setStateData(AndroidTextInputState{ + (state.reactTreeAttributedString == reactTreeAttributedString + ? 0 + : getProps()->mostRecentEventCount), + (state.reactTreeAttributedString == reactTreeAttributedString + ? state.attributedString + : reactTreeAttributedString), + reactTreeAttributedString, + getProps()->paragraphAttributes, + defaultTextAttributes, + ShadowView(*this), + textLayoutManager_}); } #pragma mark - LayoutableShadowNode @@ -141,7 +143,7 @@ Size AndroidTextInputShadowNode::measure( AttributedString attributedString = state.attributedString; if (attributedString.isEmpty()) { - attributedString = getPlaceholderAttributedString(true); + attributedString = getPlaceholderAttributedString(); } if (attributedString.isEmpty()) { diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.h b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.h index 334ebb06648..ca169dbb257 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.h +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.h @@ -38,8 +38,7 @@ class AndroidTextInputShadowNode : public ConcreteViewShadowNode< * Returns a `AttributedString` which represents text content of the node. */ AttributedString getAttributedString() const; - AttributedString getPlaceholderAttributedString( - bool ensureMinimumLength) const; + AttributedString getPlaceholderAttributedString() const; /* * Associates a shared TextLayoutManager with the node.