From 80f7891287b10d63158210ae3ba4c78f67edaeb3 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Tue, 26 Jun 2018 11:32:57 -0700 Subject: [PATCH] Fabric: Embracing non-trivial default values of some Props Summary: @public This is follow up for D8601600 and D8247652 (the last one has detailed explanation of the problem). From this commit I propose that we have to follow simple rule: If some prop has a default value which differs from the default value of its type, we have to specify it as {} in .h file and explicitly in .m file, for all other props the default value must not be specified explicitly (in .h files it must be specified as {}). The reason is that we have to embrase those cases and establish behaviour: if we change the default value in .h file, it always means that we have to change the value in .cpp file too. Reviewed By: fkgozali Differential Revision: D8601776 fbshipit-source-id: 3379aace4e2d72febb2b942a3da1cb24decf54be --- .../attributedstring/ParagraphAttributes.h | 6 ++--- .../fabric/scrollview/ScrollViewProps.cpp | 26 +++++++++---------- .../fabric/scrollview/ScrollViewProps.h | 18 ++++++------- .../fabric/text/paragraph/ParagraphProps.cpp | 4 +-- .../fabric/text/paragraph/ParagraphProps.h | 2 +- .../fabric/text/rawtext/RawTextProps.h | 2 +- ReactCommon/fabric/view/ViewProps.h | 16 ++++++------ 7 files changed, 37 insertions(+), 37 deletions(-) diff --git a/ReactCommon/fabric/attributedstring/ParagraphAttributes.h b/ReactCommon/fabric/attributedstring/ParagraphAttributes.h index b6271999a..9feaea20b 100644 --- a/ReactCommon/fabric/attributedstring/ParagraphAttributes.h +++ b/ReactCommon/fabric/attributedstring/ParagraphAttributes.h @@ -36,18 +36,18 @@ public: * Maximum number of lines which paragraph can take. * Zero value represents "no limit". */ - int maximumNumberOfLines {0}; + int maximumNumberOfLines {}; /* * In case if a text cannot fit given boundaries, defines a place where * an ellipsize should be placed. */ - EllipsizeMode ellipsizeMode {EllipsizeMode::Clip}; + EllipsizeMode ellipsizeMode {}; /* * Enables font size adjustment to fit constrained boundaries. */ - bool adjustsFontSizeToFit {false}; + bool adjustsFontSizeToFit {}; /* * In case of font size adjustment enabled, defines minimum and maximum diff --git a/ReactCommon/fabric/scrollview/ScrollViewProps.cpp b/ReactCommon/fabric/scrollview/ScrollViewProps.cpp index 1e4004d06..c3b1b4e7a 100644 --- a/ReactCommon/fabric/scrollview/ScrollViewProps.cpp +++ b/ReactCommon/fabric/scrollview/ScrollViewProps.cpp @@ -18,27 +18,27 @@ namespace react { ScrollViewProps::ScrollViewProps(const ScrollViewProps &sourceProps, const RawProps &rawProps): ViewProps(sourceProps, rawProps), - alwaysBounceHorizontal(convertRawProp(rawProps, "alwaysBounceHorizontal", sourceProps.alwaysBounceHorizontal)), - alwaysBounceVertical(convertRawProp(rawProps, "alwaysBounceVertical", sourceProps.alwaysBounceVertical)), - bounces(convertRawProp(rawProps, "bounces", sourceProps.bounces)), - bouncesZoom(convertRawProp(rawProps, "bouncesZoom", sourceProps.bouncesZoom)), + alwaysBounceHorizontal(convertRawProp(rawProps, "alwaysBounceHorizontal", sourceProps.alwaysBounceHorizontal, true)), + alwaysBounceVertical(convertRawProp(rawProps, "alwaysBounceVertical", sourceProps.alwaysBounceVertical, true)), + bounces(convertRawProp(rawProps, "bounces", sourceProps.bounces, true)), + bouncesZoom(convertRawProp(rawProps, "bouncesZoom", sourceProps.bouncesZoom, true)), canCancelContentTouches(convertRawProp(rawProps, "canCancelContentTouches", sourceProps.canCancelContentTouches)), centerContent(convertRawProp(rawProps, "centerContent", sourceProps.centerContent)), automaticallyAdjustContentInsets(convertRawProp(rawProps, "automaticallyAdjustContentInsets", sourceProps.automaticallyAdjustContentInsets)), - decelerationRate(convertRawProp(rawProps, "decelerationRate", sourceProps.decelerationRate)), + decelerationRate(convertRawProp(rawProps, "decelerationRate", sourceProps.decelerationRate, (Float)0.998)), directionalLockEnabled(convertRawProp(rawProps, "directionalLockEnabled", sourceProps.directionalLockEnabled)), indicatorStyle(convertRawProp(rawProps, "indicatorStyle", sourceProps.indicatorStyle)), keyboardDismissMode(convertRawProp(rawProps, "keyboardDismissMode", sourceProps.keyboardDismissMode)), - maximumZoomScale(convertRawProp(rawProps, "maximumZoomScale", sourceProps.maximumZoomScale)), - minimumZoomScale(convertRawProp(rawProps, "minimumZoomScale", sourceProps.minimumZoomScale)), - scrollEnabled(convertRawProp(rawProps, "scrollEnabled", sourceProps.scrollEnabled)), + maximumZoomScale(convertRawProp(rawProps, "maximumZoomScale", sourceProps.maximumZoomScale, (Float)1.0)), + minimumZoomScale(convertRawProp(rawProps, "minimumZoomScale", sourceProps.minimumZoomScale, (Float)1.0)), + scrollEnabled(convertRawProp(rawProps, "scrollEnabled", sourceProps.scrollEnabled, true)), pagingEnabled(convertRawProp(rawProps, "pagingEnabled", sourceProps.pagingEnabled)), - pinchGestureEnabled(convertRawProp(rawProps, "pinchGestureEnabled", sourceProps.pinchGestureEnabled)), - scrollsToTop(convertRawProp(rawProps, "scrollsToTop", sourceProps.scrollsToTop)), - showsHorizontalScrollIndicator(convertRawProp(rawProps, "showsHorizontalScrollIndicator", sourceProps.showsHorizontalScrollIndicator)), - showsVerticalScrollIndicator(convertRawProp(rawProps, "showsVerticalScrollIndicator", sourceProps.showsVerticalScrollIndicator)), + pinchGestureEnabled(convertRawProp(rawProps, "pinchGestureEnabled", sourceProps.pinchGestureEnabled, true)), + scrollsToTop(convertRawProp(rawProps, "scrollsToTop", sourceProps.scrollsToTop, true)), + showsHorizontalScrollIndicator(convertRawProp(rawProps, "showsHorizontalScrollIndicator", sourceProps.showsHorizontalScrollIndicator, true)), + showsVerticalScrollIndicator(convertRawProp(rawProps, "showsVerticalScrollIndicator", sourceProps.showsVerticalScrollIndicator, true)), scrollEventThrottle(convertRawProp(rawProps, "scrollEventThrottle", sourceProps.scrollEventThrottle)), - zoomScale(convertRawProp(rawProps, "zoomScale", sourceProps.zoomScale)), + zoomScale(convertRawProp(rawProps, "zoomScale", sourceProps.zoomScale, (Float)1.0)), contentInset(convertRawProp(rawProps, "contentInset", sourceProps.contentInset)), scrollIndicatorInsets(convertRawProp(rawProps, "scrollIndicatorInsets", sourceProps.scrollIndicatorInsets)), snapToInterval(convertRawProp(rawProps, "snapToInterval", sourceProps.snapToInterval)), diff --git a/ReactCommon/fabric/scrollview/ScrollViewProps.h b/ReactCommon/fabric/scrollview/ScrollViewProps.h index 190332c2f..219902470 100644 --- a/ReactCommon/fabric/scrollview/ScrollViewProps.h +++ b/ReactCommon/fabric/scrollview/ScrollViewProps.h @@ -28,26 +28,26 @@ public: const bool bounces {true}; const bool bouncesZoom {true}; const bool canCancelContentTouches {true}; - const bool centerContent {false}; - const bool automaticallyAdjustContentInsets {false}; + const bool centerContent {}; + const bool automaticallyAdjustContentInsets {}; const Float decelerationRate {0.998}; - const bool directionalLockEnabled {false}; - const ScrollViewIndicatorStyle indicatorStyle {ScrollViewIndicatorStyle::Default}; - const ScrollViewKeyboardDismissMode keyboardDismissMode {ScrollViewKeyboardDismissMode::None}; + const bool directionalLockEnabled {}; + const ScrollViewIndicatorStyle indicatorStyle {}; + const ScrollViewKeyboardDismissMode keyboardDismissMode {}; const Float maximumZoomScale {1.0}; const Float minimumZoomScale {1.0}; const bool scrollEnabled {true}; - const bool pagingEnabled {false}; + const bool pagingEnabled {}; const bool pinchGestureEnabled {true}; const bool scrollsToTop {true}; const bool showsHorizontalScrollIndicator {true}; const bool showsVerticalScrollIndicator {true}; - const Float scrollEventThrottle {0}; + const Float scrollEventThrottle {}; const Float zoomScale {1.0}; const EdgeInsets contentInset {}; const EdgeInsets scrollIndicatorInsets {}; - const int snapToInterval {0}; - const ScrollViewSnapToAlignment snapToAlignment {ScrollViewSnapToAlignment::Start}; + const int snapToInterval {}; + const ScrollViewSnapToAlignment snapToAlignment {}; #pragma mark - DebugStringConvertible diff --git a/ReactCommon/fabric/text/paragraph/ParagraphProps.cpp b/ReactCommon/fabric/text/paragraph/ParagraphProps.cpp index a6ea79c6e..ca4ff847c 100644 --- a/ReactCommon/fabric/text/paragraph/ParagraphProps.cpp +++ b/ReactCommon/fabric/text/paragraph/ParagraphProps.cpp @@ -20,8 +20,8 @@ static ParagraphAttributes convertRawProp(const RawProps &rawProps, const Paragr paragraphAttributes.maximumNumberOfLines = convertRawProp(rawProps, "numberOfLines", defaultParagraphAttributes.maximumNumberOfLines); paragraphAttributes.ellipsizeMode = convertRawProp(rawProps, "ellipsizeMode", defaultParagraphAttributes.ellipsizeMode); paragraphAttributes.adjustsFontSizeToFit = convertRawProp(rawProps, "adjustsFontSizeToFit", defaultParagraphAttributes.adjustsFontSizeToFit); - paragraphAttributes.minimumFontSize = convertRawProp(rawProps, "minimumFontSize", defaultParagraphAttributes.minimumFontSize); - paragraphAttributes.maximumFontSize = convertRawProp(rawProps, "maximumFontSize", defaultParagraphAttributes.maximumFontSize); + paragraphAttributes.minimumFontSize = convertRawProp(rawProps, "minimumFontSize", defaultParagraphAttributes.minimumFontSize, std::numeric_limits::quiet_NaN()); + paragraphAttributes.maximumFontSize = convertRawProp(rawProps, "maximumFontSize", defaultParagraphAttributes.maximumFontSize, std::numeric_limits::quiet_NaN()); return paragraphAttributes; } diff --git a/ReactCommon/fabric/text/paragraph/ParagraphProps.h b/ReactCommon/fabric/text/paragraph/ParagraphProps.h index a31e1c358..5877be6cb 100644 --- a/ReactCommon/fabric/text/paragraph/ParagraphProps.h +++ b/ReactCommon/fabric/text/paragraph/ParagraphProps.h @@ -45,7 +45,7 @@ public: /* * Defines can the text be selected (and copied) or not. */ - const bool isSelectable {false}; + const bool isSelectable {}; #pragma mark - DebugStringConvertible diff --git a/ReactCommon/fabric/text/rawtext/RawTextProps.h b/ReactCommon/fabric/text/rawtext/RawTextProps.h index 37ba902ee..45f1f3d01 100644 --- a/ReactCommon/fabric/text/rawtext/RawTextProps.h +++ b/ReactCommon/fabric/text/rawtext/RawTextProps.h @@ -29,7 +29,7 @@ public: #pragma mark - Props - const std::string text {""}; + const std::string text {}; #pragma mark - DebugStringConvertible diff --git a/ReactCommon/fabric/view/ViewProps.h b/ReactCommon/fabric/view/ViewProps.h index 2a15323e2..a533b051f 100644 --- a/ReactCommon/fabric/view/ViewProps.h +++ b/ReactCommon/fabric/view/ViewProps.h @@ -35,9 +35,9 @@ public: #pragma mark - Props // Color - const Float opacity {1}; - const SharedColor foregroundColor {nullptr}; - const SharedColor backgroundColor {nullptr}; + const Float opacity {1.0}; + const SharedColor foregroundColor {}; + const SharedColor backgroundColor {}; // Borders const EdgeInsets borderWidth {}; @@ -46,21 +46,21 @@ public: const BorderStyle borderStyle {}; // Shadow - const SharedColor shadowColor {nullptr}; + const SharedColor shadowColor {}; const Size shadowOffset {}; const Float shadowOpacity {}; const Float shadowRadius {}; // Transform const Transform transform {}; - const bool backfaceVisibility {false}; - const bool shouldRasterize {false}; - const int zIndex {0}; + const bool backfaceVisibility {}; + const bool shouldRasterize {}; + const int zIndex {}; // Events const PointerEventsMode pointerEvents {}; const EdgeInsets hitSlop {}; - const bool onLayout {false}; + const bool onLayout {}; #pragma mark - DebugStringConvertible