From 5be86695a3a43a8666cc75e1cdf9429b194a6ce3 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Wed, 26 Feb 2020 13:42:07 -0800 Subject: [PATCH] AndroidTextInput uses default padding from Android theme Summary: For backwards-compatibility with Paper, we're implementing a feature in Fabric that will allow TextInputs to use the default padding from the theme in Android. Note that this uses some pretty ugly hacks that probably shouldn't be used inside of components at all: looking directly at rawProps, overriding props/Yoga styles in the component descriptor, etc. I would (personally) really like to kill this feature entirely unless and until we can find a more elegant solution. Changelog: [Internal] TextInputs are still not pixel-perfect with Paper, but they're much closer, and the underline visual glitchiness is no longer an issue. Reviewed By: mdvacca Differential Revision: D20109605 fbshipit-source-id: 543282843e0a9f03a504d72d7a014431099bd64c --- .../textinput/ReactTextInputManager.java | 35 +++++++- .../AndroidTextInputComponentDescriptor.h | 85 ++++++++++++++++++- .../AndroidTextInputProps.cpp | 19 ++++- .../androidtextinput/AndroidTextInputProps.h | 12 +++ .../AndroidTextInputShadowNode.cpp | 6 +- .../AndroidTextInputState.cpp | 1 + .../androidtextinput/AndroidTextInputState.h | 28 +++++- 7 files changed, 178 insertions(+), 8 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java index c2bc2b41a78..af8cf64b9fc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputManager.java @@ -23,9 +23,11 @@ import android.view.Gravity; import android.view.KeyEvent; import android.view.View; import android.view.inputmethod.EditorInfo; +import android.widget.EditText; import android.widget.TextView; import androidx.annotation.Nullable; import androidx.core.content.ContextCompat; +import androidx.core.view.ViewCompat; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; import com.facebook.react.bridge.Dynamic; @@ -1229,11 +1231,42 @@ public class ReactTextInputManager extends BaseViewManager #include "AndroidTextInputShadowNode.h" +#include +#include +#include + namespace facebook { namespace react { @@ -27,7 +31,32 @@ class AndroidTextInputComponentDescriptor final textLayoutManager_ = std::make_shared(contextContainer_); } - protected: + virtual State::Shared createInitialState( + ShadowNodeFragment const &fragment, + ShadowNodeFamily::Shared const &family) const override { + int surfaceId = family->getSurfaceId(); + + float defaultThemePaddingStart = NAN; + float defaultThemePaddingEnd = NAN; + float defaultThemePaddingTop = NAN; + float defaultThemePaddingBottom = NAN; + + if (surfaceIdToThemePaddingMap_.find(surfaceId) != surfaceIdToThemePaddingMap_.end()) { + YGStyle::Edges theme = surfaceIdToThemePaddingMap_[surfaceId]; + defaultThemePaddingStart = ((YGValue)theme[YGEdgeStart]).value; + defaultThemePaddingEnd = ((YGValue)theme[YGEdgeEnd]).value; + defaultThemePaddingTop = ((YGValue)theme[YGEdgeTop]).value; + defaultThemePaddingBottom = ((YGValue)theme[YGEdgeBottom]).value; + } + + return std::make_shared( + std::make_shared(AndroidTextInputState( + 0, {}, {}, {}, {}, {}, textLayoutManager_, defaultThemePaddingStart, defaultThemePaddingEnd, defaultThemePaddingTop, defaultThemePaddingBottom)), + family); + } + +protected: + void adopt(UnsharedShadowNode shadowNode) const override { assert(std::dynamic_pointer_cast(shadowNode)); auto textInputShadowNode = @@ -40,6 +69,59 @@ class AndroidTextInputComponentDescriptor final textInputShadowNode->setContextContainer( const_cast(getContextContainer().get())); + // Get theme padding from cache, or set it from State. + // In theory, the Java ViewManager for TextInput should need to set state *exactly once* + // per surface to communicate the correct default padding, which will be cached here in C++. + // TODO T63008435: can this feature be removed entirely? + // TODO: figure out RTL/start/end/left/right stuff here + int surfaceId = textInputShadowNode->getSurfaceId(); + const AndroidTextInputState &state = textInputShadowNode->getStateData(); + if (surfaceIdToThemePaddingMap_.find(surfaceId) == surfaceIdToThemePaddingMap_.end() && !isnan(state.defaultThemePaddingStart)) { + YGStyle::Edges result; + result[YGEdgeStart] = (YGValue){state.defaultThemePaddingStart, YGUnitPoint}; + result[YGEdgeEnd] = (YGValue){state.defaultThemePaddingEnd, YGUnitPoint}; + result[YGEdgeTop] = (YGValue){state.defaultThemePaddingTop, YGUnitPoint}; + result[YGEdgeBottom] = (YGValue){state.defaultThemePaddingBottom, YGUnitPoint}; + surfaceIdToThemePaddingMap_.emplace(std::make_pair(surfaceId, result)); + } + + if (surfaceIdToThemePaddingMap_.find(surfaceId) != surfaceIdToThemePaddingMap_.end()) { + YGStyle::Edges theme = surfaceIdToThemePaddingMap_[surfaceId]; + + // Override padding + // Node is still unsealed during adoption, before layout is complete + // TODO: T62959168 account for RTL and paddingLeft when setting default paddingStart, and + // vice-versa with paddingRight/paddingEnd + YGStyle::Edges result = textInputShadowNode->getConcreteProps().yogaStyle.padding(); + bool changedPadding = false; + if (!textInputShadowNode->getConcreteProps().hasPaddingStart) { + changedPadding = true; + result[YGEdgeStart] = theme[YGEdgeStart]; + } + if (!textInputShadowNode->getConcreteProps().hasPaddingEnd) { + changedPadding = true; + result[YGEdgeEnd] = theme[YGEdgeEnd]; + } + if (!textInputShadowNode->getConcreteProps().hasPaddingTop) { + changedPadding = true; + result[YGEdgeTop] = theme[YGEdgeTop]; + } + if (!textInputShadowNode->getConcreteProps().hasPaddingBottom) { + changedPadding = true; + result[YGEdgeBottom] = theme[YGEdgeBottom]; + } + + // Note that this is expensive: on every adopt, we need to set the Yoga props again, which + // normally only happens during prop parsing. Every commit, state update, etc, will incur this + // cost. + if (changedPadding) { + // Set new props on node + const_cast(textInputShadowNode->getConcreteProps()).yogaStyle.padding() = result; + // Communicate new props to Yoga part of the node + textInputShadowNode->setProps((YogaStylableProps)textInputShadowNode->getConcreteProps()); + } + } + textInputShadowNode->dirtyLayout(); textInputShadowNode->enableMeasurement(); @@ -48,6 +130,7 @@ class AndroidTextInputComponentDescriptor final private: SharedTextLayoutManager textLayoutManager_; + mutable better::map surfaceIdToThemePaddingMap_; }; } // namespace react diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.cpp index 09d8e1506cb..f22aa5b6f42 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.cpp +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.cpp @@ -222,7 +222,16 @@ AndroidTextInputProps::AndroidTextInputProps( {0})), text(convertRawProp(rawProps, "text", sourceProps.text, {})), paragraphAttributes( - convertRawProp(rawProps, sourceProps.paragraphAttributes, {})) {} + convertRawProp(rawProps, sourceProps.paragraphAttributes, {})), + // See AndroidTextInputComponentDescriptor for usage + // TODO T63008435: can these, and this feature, be removed entirely? + hasPaddingLeft(rawProps.at("", "padding", "") != nullptr || rawProps.at("Left", "padding", "") != nullptr), + hasPaddingTop(rawProps.at("", "padding", "") != nullptr || rawProps.at("Top", "padding", "") != nullptr), + hasPaddingRight(rawProps.at("", "padding", "") != nullptr || rawProps.at("Right", "padding", "") != nullptr), + hasPaddingBottom(rawProps.at("", "padding", "") != nullptr || rawProps.at("Bottom", "padding", "") != nullptr), + hasPaddingStart(rawProps.at("", "padding", "") != nullptr || rawProps.at("Start", "padding", "") != nullptr), + hasPaddingEnd(rawProps.at("", "padding", "") != nullptr || rawProps.at("End", "padding", "") != nullptr) + { } // TODO T53300085: support this in codegen; this was hand-written folly::dynamic AndroidTextInputProps::getDynamic() const { @@ -276,6 +285,14 @@ folly::dynamic AndroidTextInputProps::getDynamic() const { props["cursorColor"] = toDynamic(cursorColor); props["mostRecentEventCount"] = mostRecentEventCount; props["text"] = text; + + props["hasPaddingStart"] = hasPaddingStart; + props["hasPaddingEnd"] = hasPaddingEnd; + props["hasPaddingLeft"] = hasPaddingLeft; + props["hasPaddingRight"] = hasPaddingRight; + props["hasPaddingTop"] = hasPaddingTop; + props["hasPaddingBottom"] = hasPaddingBottom; + return props; } diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.h b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.h index 7fb58b0ddc0..68430021892 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.h +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.h @@ -159,6 +159,18 @@ class AndroidTextInputProps final : public ViewProps, public BaseTextProps { */ ParagraphAttributes const paragraphAttributes{}; + /** + * Auxiliary information to detect if these props are set or not. + * See AndroidTextInputComponentDescriptor for usage. + * TODO T63008435: can these, and this feature, be removed entirely? + */ + const bool hasPaddingLeft{}; + const bool hasPaddingTop{}; + const bool hasPaddingRight{}; + const bool hasPaddingBottom{}; + const bool hasPaddingStart{}; + const bool hasPaddingEnd{}; + #if RN_DEBUG_STRING_CONVERTIBLE SharedDebugStringConvertibleList getDebugProps() const; #endif diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp index b655da3cad9..daba6efc7bb 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp @@ -144,7 +144,11 @@ void AndroidTextInputShadowNode::updateStateIfNeeded() { getConcreteProps().paragraphAttributes, defaultTextAttributes, ShadowView(*this), - textLayoutManager_}); + textLayoutManager_, + state.defaultThemePaddingStart, + state.defaultThemePaddingEnd, + state.defaultThemePaddingTop, + state.defaultThemePaddingBottom}); } #pragma mark - LayoutableShadowNode diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.cpp index 4e37be0aa6f..0128c17f00e 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.cpp +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.cpp @@ -21,6 +21,7 @@ folly::dynamic AndroidTextInputState::getDynamic() const { newState["attributedString"] = toDynamic(attributedString); newState["paragraphAttributes"] = toDynamic(paragraphAttributes); newState["hash"] = newState["attributedString"]["hash"]; + newState["hasThemeData"] = !isnan(defaultThemePaddingStart); return newState; } #endif diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.h b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.h index e00cb66aa2f..635eb395f4b 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.h +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputState.h @@ -67,6 +67,14 @@ class AndroidTextInputState final { */ SharedTextLayoutManager layoutManager{}; + /** + * Communicates Android theme padding back to the ShadowNode / Component Descriptor for layout. + */ + float defaultThemePaddingStart{NAN}; + float defaultThemePaddingEnd{NAN}; + float defaultThemePaddingTop{NAN}; + float defaultThemePaddingBottom{NAN}; + #ifdef ANDROID AttributedString updateAttributedString( TextAttributes const &defaultTextAttributes, @@ -117,19 +125,27 @@ class AndroidTextInputState final { ParagraphAttributes const ¶graphAttributes, TextAttributes const &defaultTextAttributes, ShadowView const &defaultParentShadowView, - SharedTextLayoutManager const &layoutManager) + SharedTextLayoutManager const &layoutManager, + float defaultThemePaddingStart, + float defaultThemePaddingEnd, + float defaultThemePaddingTop, + float defaultThemePaddingBottom) : mostRecentEventCount(mostRecentEventCount), attributedString(attributedString), reactTreeAttributedString(reactTreeAttributedString), paragraphAttributes(paragraphAttributes), defaultTextAttributes(defaultTextAttributes), defaultParentShadowView(defaultParentShadowView), - layoutManager(layoutManager) {} + layoutManager(layoutManager), + defaultThemePaddingStart(defaultThemePaddingStart), + defaultThemePaddingEnd(defaultThemePaddingEnd), + defaultThemePaddingTop(defaultThemePaddingTop), + defaultThemePaddingBottom(defaultThemePaddingBottom) {} AndroidTextInputState() = default; AndroidTextInputState( AndroidTextInputState const &previousState, folly::dynamic const &data) - : mostRecentEventCount((int64_t)data["mostRecentEventCount"].getInt()), + : mostRecentEventCount(data.getDefault("mostRecentEventCount", previousState.mostRecentEventCount).getInt()), attributedString(updateAttributedString( previousState.defaultTextAttributes, previousState.defaultParentShadowView, @@ -139,7 +155,11 @@ class AndroidTextInputState final { paragraphAttributes(previousState.paragraphAttributes), defaultTextAttributes(previousState.defaultTextAttributes), defaultParentShadowView(previousState.defaultParentShadowView), - layoutManager(previousState.layoutManager){}; + layoutManager(previousState.layoutManager), + defaultThemePaddingStart(data.getDefault("themePaddingStart", previousState.defaultThemePaddingStart).getDouble()), + defaultThemePaddingEnd(data.getDefault("themePaddingEnd", previousState.defaultThemePaddingEnd).getDouble()), + defaultThemePaddingTop(data.getDefault("themePaddingTop", previousState.defaultThemePaddingTop).getDouble()), + defaultThemePaddingBottom(data.getDefault("themePaddingBottom", previousState.defaultThemePaddingBottom).getDouble()) {}; folly::dynamic getDynamic() const; #endif };