From c18cc76e58070577ae062c154f47d0f5f91e9124 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Fri, 6 Mar 2020 19:31:58 -0800 Subject: [PATCH] Comparison of AttributedString false more often than true in TextInput, resulting in janky editing behavior Summary: Instead of comparing the entire AttributedString, compare just the strings and the TextAttributes of Fragments. Concretely what I'm seeing is that the Frame of the associated parent ShadowViews are changing very frequently, making it impossible to actually modify the TextInput in some cases. However, we shouldn't forcibly reset the content of the TextInput if the frame of the parent is changing and not the actual child contents. Changelog: [Internal] Fabric TextInput bug fix Reviewed By: shergin Differential Revision: D20319359 fbshipit-source-id: 2f51f521ad76fff9da6f6c8b5e795f03c33e496f --- .../fabric/attributedstring/AttributedString.cpp | 16 ++++++++++++++++ .../fabric/attributedstring/AttributedString.h | 5 +++++ .../AndroidTextInputShadowNode.cpp | 13 ++++++++++--- 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/ReactCommon/fabric/attributedstring/AttributedString.cpp b/ReactCommon/fabric/attributedstring/AttributedString.cpp index 40efe29963..01ae1188b2 100644 --- a/ReactCommon/fabric/attributedstring/AttributedString.cpp +++ b/ReactCommon/fabric/attributedstring/AttributedString.cpp @@ -98,6 +98,22 @@ bool AttributedString::isEmpty() const { return fragments_.empty(); } +bool AttributedString::compareTextAttributesWithoutFrame( + const AttributedString &rhs) const { + if (fragments_.size() != rhs.fragments_.size()) { + return false; + } + + for (unsigned i = 0; i < fragments_.size(); i++) { + if (fragments_[i].textAttributes != rhs.fragments_[i].textAttributes || + fragments_[i].string != rhs.fragments_[i].string) { + return false; + } + } + + return true; +} + bool AttributedString::operator==(const AttributedString &rhs) const { return fragments_ == rhs.fragments_; } diff --git a/ReactCommon/fabric/attributedstring/AttributedString.h b/ReactCommon/fabric/attributedstring/AttributedString.h index 99b35d19c3..c6b07edd71 100644 --- a/ReactCommon/fabric/attributedstring/AttributedString.h +++ b/ReactCommon/fabric/attributedstring/AttributedString.h @@ -87,6 +87,11 @@ class AttributedString : public Sealable, public DebugStringConvertible { */ bool isEmpty() const; + /** + * Compares equality of TextAttributes of all Fragments on both sides. + */ + bool compareTextAttributesWithoutFrame(const AttributedString &rhs) const; + bool operator==(const AttributedString &rhs) const; bool operator!=(const AttributedString &rhs) const; diff --git a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp index daba6efc7b..376c1615ae 100644 --- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp +++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputShadowNode.cpp @@ -94,10 +94,17 @@ AttributedString AndroidTextInputShadowNode::getMostRecentAttributedString() auto reactTreeAttributedString = getAttributedString(); + // Sometimes the treeAttributedString will only differ from the state + // not by inherent properties (string or prop attributes), but by the frame of + // the parent which has changed Thus, we can't directly compare the entire + // AttributedString + bool treeAttributedStringChanged = + !state.reactTreeAttributedString.compareTextAttributesWithoutFrame( + reactTreeAttributedString); + return ( - state.reactTreeAttributedString == reactTreeAttributedString - ? state.attributedString - : reactTreeAttributedString); + !treeAttributedStringChanged ? state.attributedString + : reactTreeAttributedString); } void AndroidTextInputShadowNode::updateStateIfNeeded() {