Skip to content

Commit

Permalink
Fabric: Embracing non-trivial default values of some Props
Browse files Browse the repository at this point in the history
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 {<value>} 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
  • Loading branch information
shergin authored and facebook-github-bot committed Jun 26, 2018
1 parent d629e4b commit 80f7891
Show file tree
Hide file tree
Showing 7 changed files with 37 additions and 37 deletions.
6 changes: 3 additions & 3 deletions ReactCommon/fabric/attributedstring/ParagraphAttributes.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,18 @@ class ParagraphAttributes:
* 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
Expand Down
26 changes: 13 additions & 13 deletions ReactCommon/fabric/scrollview/ScrollViewProps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
Expand Down
18 changes: 9 additions & 9 deletions ReactCommon/fabric/scrollview/ScrollViewProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,26 +28,26 @@ class ScrollViewProps final:
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

Expand Down
4 changes: 2 additions & 2 deletions ReactCommon/fabric/text/paragraph/ParagraphProps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<Float>::quiet_NaN());
paragraphAttributes.maximumFontSize = convertRawProp(rawProps, "maximumFontSize", defaultParagraphAttributes.maximumFontSize, std::numeric_limits<Float>::quiet_NaN());

return paragraphAttributes;
}
Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/fabric/text/paragraph/ParagraphProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class ParagraphProps:
/*
* Defines can the text be selected (and copied) or not.
*/
const bool isSelectable {false};
const bool isSelectable {};

#pragma mark - DebugStringConvertible

Expand Down
2 changes: 1 addition & 1 deletion ReactCommon/fabric/text/rawtext/RawTextProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class RawTextProps:

#pragma mark - Props

const std::string text {""};
const std::string text {};

#pragma mark - DebugStringConvertible

Expand Down
16 changes: 8 additions & 8 deletions ReactCommon/fabric/view/ViewProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ class ViewProps:
#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 {};
Expand All @@ -46,21 +46,21 @@ class ViewProps:
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

Expand Down

0 comments on commit 80f7891

Please sign in to comment.