From d8c9b1d78970d958dfb7323e16c7aab52c6d323b Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Tue, 15 Nov 2022 17:19:12 -0800 Subject: [PATCH] Add Fabric implementation of flow-relative padding and margin (#35342) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/35342 This is a native implementation of the JS shimmed layout-specific properties in https://github.com/facebook/react-native/pull/35316. There is an experiment splitting the prop splitting codepath in Fabric, so this change effectively has two implementations depending on whether `enablePropIteratorSetter` is enabled. None of these changes make sense to propagate Yoga. `inlineEnd`, etc are already mapped to `YGEdgeStart` and `YGEdgeEnd`, but RN's mapping used a different name. Then `blockStart`, `blockEnd`, map directly to existing edges in all cases since we don't support a writing mode. On web, the last value in the style which computes the given dimension is given precedence. E.g. if "left" comes after "start" it will be chosen. Yoga stylesheets are unordered, and precedence for edges is given based on specificity (left > start > horizontal > all). We give precedence to new renamings (e.g. start to inlineStart), but to preserve consistent behavior, we give precedence to specific edges before overwriting them with flow relative ones (e.g. marginTop has precedence over marginBlockStar). Changelog: [General][Added] - Add Fabric implementation of flow-relative padding and margin Differential Revision: D41267765 fbshipit-source-id: 2d21c6acb710b04a8577e98bf5cc7d4bb2de1815 --- .../View/ReactNativeStyleAttributes.js | 12 ++ .../NativeComponent/BaseViewConfig.android.js | 32 +++-- .../NativeComponent/BaseViewConfig.ios.js | 32 +++-- Libraries/StyleSheet/StyleSheetTypes.d.ts | 12 ++ Libraries/StyleSheet/StyleSheetTypes.js | 70 +++++++++++ .../view/YogaLayoutableShadowNode.cpp | 53 ++++++++- .../view/YogaLayoutableShadowNode.h | 10 ++ .../components/view/YogaStylableProps.cpp | 111 +++++++++++++++++- .../components/view/YogaStylableProps.h | 30 ++++- 9 files changed, 338 insertions(+), 24 deletions(-) diff --git a/Libraries/Components/View/ReactNativeStyleAttributes.js b/Libraries/Components/View/ReactNativeStyleAttributes.js index 926c0c7efa6441..06f1d6df54ef6f 100644 --- a/Libraries/Components/View/ReactNativeStyleAttributes.js +++ b/Libraries/Components/View/ReactNativeStyleAttributes.js @@ -49,9 +49,15 @@ const ReactNativeStyleAttributes: {[string]: AnyAttributeType, ...} = { justifyContent: true, left: true, margin: true, + marginBlock: true, + marginBlockEnd: true, + marginBlockStart: true, marginBottom: true, marginEnd: true, marginHorizontal: true, + marginInline: true, + marginInlineEnd: true, + marginInlineStart: true, marginLeft: true, marginRight: true, marginStart: true, @@ -63,9 +69,15 @@ const ReactNativeStyleAttributes: {[string]: AnyAttributeType, ...} = { minWidth: true, overflow: true, padding: true, + paddingBlock: true, + paddingBlockEnd: true, + paddingBlockStart: true, paddingBottom: true, paddingEnd: true, paddingHorizontal: true, + paddingInline: true, + paddingInlineEnd: true, + paddingInlineStart: true, paddingLeft: true, paddingRight: true, paddingStart: true, diff --git a/Libraries/NativeComponent/BaseViewConfig.android.js b/Libraries/NativeComponent/BaseViewConfig.android.js index 6d2a0480c109b8..b394cf551bd6be 100644 --- a/Libraries/NativeComponent/BaseViewConfig.android.js +++ b/Libraries/NativeComponent/BaseViewConfig.android.js @@ -212,24 +212,36 @@ const validAttributesForNonEventProps = { display: true, margin: true, - marginVertical: true, - marginHorizontal: true, - marginStart: true, - marginEnd: true, - marginTop: true, + marginBlock: true, + marginBlockEnd: true, + marginBlockStart: true, marginBottom: true, + marginEnd: true, + marginHorizontal: true, + marginInline: true, + marginInlineEnd: true, + marginInlineStart: true, marginLeft: true, marginRight: true, + marginStart: true, + marginTop: true, + marginVertical: true, padding: true, - paddingVertical: true, - paddingHorizontal: true, - paddingStart: true, - paddingEnd: true, - paddingTop: true, + paddingBlock: true, + paddingBlockEnd: true, + paddingBlockStart: true, paddingBottom: true, + paddingEnd: true, + paddingHorizontal: true, + paddingInline: true, + paddingInlineEnd: true, + paddingInlineStart: true, paddingLeft: true, paddingRight: true, + paddingStart: true, + paddingTop: true, + paddingVertical: true, borderWidth: true, borderStartWidth: true, diff --git a/Libraries/NativeComponent/BaseViewConfig.ios.js b/Libraries/NativeComponent/BaseViewConfig.ios.js index 093588c9c7b266..8d371596dba5d6 100644 --- a/Libraries/NativeComponent/BaseViewConfig.ios.js +++ b/Libraries/NativeComponent/BaseViewConfig.ios.js @@ -249,25 +249,37 @@ const validAttributesForNonEventProps = { // borderEndWidth: true, // borderWidth: true, - marginTop: true, - marginRight: true, + margin: true, + marginBlock: true, + marginBlockEnd: true, + marginBlockStart: true, marginBottom: true, + marginEnd: true, + marginHorizontal: true, + marginInline: true, + marginInlineEnd: true, + marginInlineStart: true, marginLeft: true, + marginRight: true, marginStart: true, - marginEnd: true, + marginTop: true, marginVertical: true, - marginHorizontal: true, - margin: true, - paddingTop: true, - paddingRight: true, + padding: true, + paddingBlock: true, + paddingBlockEnd: true, + paddingBlockStart: true, paddingBottom: true, + paddingEnd: true, + paddingHorizontal: true, + paddingInline: true, + paddingInlineEnd: true, + paddingInlineStart: true, paddingLeft: true, + paddingRight: true, paddingStart: true, - paddingEnd: true, + paddingTop: true, paddingVertical: true, - paddingHorizontal: true, - padding: true, flex: true, flexGrow: true, diff --git a/Libraries/StyleSheet/StyleSheetTypes.d.ts b/Libraries/StyleSheet/StyleSheetTypes.d.ts index 5e0b8d8e1ab309..d6a8b4f31101c6 100644 --- a/Libraries/StyleSheet/StyleSheetTypes.d.ts +++ b/Libraries/StyleSheet/StyleSheetTypes.d.ts @@ -69,9 +69,15 @@ export interface FlexStyle { | undefined; left?: number | string | undefined; margin?: number | string | undefined; + marginBlock?: number | string | undefined; + marginBlockEnd?: number | string | undefined; + marginBlockStart?: number | string | undefined; marginBottom?: number | string | undefined; marginEnd?: number | string | undefined; marginHorizontal?: number | string | undefined; + marginInline?: number | string | undefined; + marginInlineEnd?: number | string | undefined; + marginInlineStart?: number | string | undefined; marginLeft?: number | string | undefined; marginRight?: number | string | undefined; marginStart?: number | string | undefined; @@ -84,8 +90,14 @@ export interface FlexStyle { overflow?: 'visible' | 'hidden' | 'scroll' | undefined; padding?: number | string | undefined; paddingBottom?: number | string | undefined; + paddingBlock?: number | string | undefined; + paddingBlockEnd?: number | string | undefined; + paddingBlockStart?: number | string | undefined; paddingEnd?: number | string | undefined; paddingHorizontal?: number | string | undefined; + paddingInline?: number | string | undefined; + paddingInlineEnd?: number | string | undefined; + paddingInlineStart?: number | string | undefined; paddingLeft?: number | string | undefined; paddingRight?: number | string | undefined; paddingStart?: number | string | undefined; diff --git a/Libraries/StyleSheet/StyleSheetTypes.js b/Libraries/StyleSheet/StyleSheetTypes.js index 14cf6d7dc9471a..d34ae2e2f793ba 100644 --- a/Libraries/StyleSheet/StyleSheetTypes.js +++ b/Libraries/StyleSheet/StyleSheetTypes.js @@ -179,6 +179,25 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ margin?: DimensionValue, + /** Setting `marginBlock` has the same effect as setting both + * `marginTop` and `marginBottom`. + */ + marginBlock?: DimensionValue, + + /** `marginBlockEnd` works like `margin-block-end`in CSS. Because React + * Native doesn not support `writing-mode` this is always mapped to + * `margin-bottom`. See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-block-end + * for more details. + */ + marginBlockEnd?: DimensionValue, + + /** `marginBlockEnd` works like `margin-block-end`in CSS. Because React + * Native doesn not support `writing-mode` this is always mapped to + * `margin-top`. See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-block-end + * for more details. + */ + marginBlockStart?: DimensionValue, + /** `marginBottom` works like `margin-bottom` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-bottom * for more details. @@ -196,6 +215,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ marginHorizontal?: DimensionValue, + /** Setting `marginInline` has the same effect as setting + * both `marginLeft` and `marginRight`. + */ + marginInline?: DimensionValue, + + /** + * When direction is `ltr`, `marginInlineEnd` is equivalent to `marginRight`. + * When direction is `rtl`, `marginInlineEnd` is equivalent to `marginLeft`. + */ + marginInlineEnd?: DimensionValue, + + /** + * When direction is `ltr`, `marginInlineStart` is equivalent to `marginLeft`. + * When direction is `rtl`, `marginInlineStart` is equivalent to `marginRight`. + */ + marginInlineStart?: DimensionValue, + /** `marginLeft` works like `margin-left` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/margin-left * for more details. @@ -232,6 +268,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ padding?: DimensionValue, + /** Setting `paddingBlock` is like setting both of + * `paddingTop` and `paddingBottom`. + */ + paddingBlock?: DimensionValue, + + /** `paddingBlockEnd` works like `padding-bottom` in CSS. + * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-bottom + * for more details. + */ + paddingBlockEnd?: DimensionValue, + + /** `paddingBlockStart` works like `padding-top` in CSS. + * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-top + * for more details. + */ + paddingBlockStart?: DimensionValue, + /** `paddingBottom` works like `padding-bottom` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-bottom * for more details. @@ -249,6 +302,23 @@ type ____LayoutStyle_Internal = $ReadOnly<{ */ paddingHorizontal?: DimensionValue, + /** Setting `paddingInline` is like setting both of + * `paddingLeft` and `paddingRight`. + */ + paddingInline?: DimensionValue, + + /** + * When direction is `ltr`, `paddingInlineEnd` is equivalent to `paddingRight`. + * When direction is `rtl`, `paddingInlineEnd` is equivalent to `paddingLeft`. + */ + paddingInlineEnd?: DimensionValue, + + /** + * When direction is `ltr`, `paddingInlineStart` is equivalent to `paddingLeft`. + * When direction is `rtl`, `paddingInlineStart` is equivalent to `paddingRight`. + */ + paddingInlineStart?: DimensionValue, + /** `paddingLeft` works like `padding-left` in CSS. * See https://developer.mozilla.org/en-US/docs/Web/CSS/padding-left * for more details. diff --git a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index a034734a213ab4..61deb7c607805a 100644 --- a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -316,13 +316,62 @@ void YogaLayoutableShadowNode::updateYogaProps() { ensureUnsealed(); auto props = static_cast(*props_); + auto styleResult = applyAliasedProps(props.yogaStyle, props); // Resetting `dirty` flag only if `yogaStyle` portion of `Props` was changed. - if (!yogaNode_.isDirty() && (props.yogaStyle != yogaNode_.getStyle())) { + if (!yogaNode_.isDirty() && (styleResult != yogaNode_.getStyle())) { yogaNode_.setDirty(true); } - yogaNode_.setStyle(props.yogaStyle); + yogaNode_.setStyle(styleResult); +} + +/*static*/ YGStyle YogaLayoutableShadowNode::applyAliasedProps( + const YGStyle &baseStyle, + const YogaStylableProps &props) { + YGStyle result{baseStyle}; + + // Aliases with precedence + if (!props.marginInline.isUndefined()) { + result.margin()[YGEdgeHorizontal] = props.marginInline; + } + if (!props.marginInlineStart.isUndefined()) { + result.margin()[YGEdgeStart] = props.marginInlineStart; + } + if (!props.marginInlineEnd.isUndefined()) { + result.margin()[YGEdgeEnd] = props.marginInlineEnd; + } + if (!props.marginBlock.isUndefined()) { + result.margin()[YGEdgeVertical] = props.marginBlock; + } + if (!props.paddingInline.isUndefined()) { + result.padding()[YGEdgeHorizontal] = props.paddingInline; + } + if (!props.paddingInlineStart.isUndefined()) { + result.padding()[YGEdgeStart] = props.paddingInlineStart; + } + if (!props.paddingInlineEnd.isUndefined()) { + result.padding()[YGEdgeEnd] = props.paddingInlineEnd; + } + if (!props.paddingBlock.isUndefined()) { + result.padding()[YGEdgeVertical] = props.paddingBlock; + } + + // Aliases without precedence + if (CompactValue(result.margin()[YGEdgeTop]).isUndefined()) { + result.margin()[YGEdgeTop] = props.marginBlockStart; + } + if (CompactValue(result.margin()[YGEdgeBottom]).isUndefined()) { + result.margin()[YGEdgeBottom] = props.marginBlockEnd; + } + if (CompactValue(result.padding()[YGEdgeTop]).isUndefined()) { + result.padding()[YGEdgeTop] = props.paddingBlockStart; + } + if (CompactValue(result.padding()[YGEdgeBottom]).isUndefined()) { + result.padding()[YGEdgeBottom] = props.paddingBlockEnd; + } + + return result; } void YogaLayoutableShadowNode::setSize(Size size) const { diff --git a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h index fe95d354930a99..84e985e3853ca9 100644 --- a/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h +++ b/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h @@ -24,6 +24,8 @@ namespace facebook { namespace react { class YogaLayoutableShadowNode : public LayoutableShadowNode { + using CompactValue = facebook::yoga::detail::CompactValue; + public: using UnsharedList = butter::small_vector< YogaLayoutableShadowNode *, @@ -173,6 +175,14 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { static void swapLeftAndRightInYogaStyleProps( YogaLayoutableShadowNode const &shadowNode); + /* + * Combine a base YGStyle with aliased properties which should be flattened + * into it. E.g. reconciling "marginInlineStart" and "marginStart". + */ + static YGStyle applyAliasedProps( + const YGStyle &baseStyle, + const YogaStylableProps &props); + #pragma mark - Consistency Ensuring Helpers void ensureConsistency() const; diff --git a/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp b/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp index f03dc21c55fb6f..614da3b2fbef88 100644 --- a/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp +++ b/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp @@ -28,7 +28,11 @@ YogaStylableProps::YogaStylableProps( yogaStyle( CoreFeatures::enablePropIteratorSetter ? sourceProps.yogaStyle - : convertRawProp(context, rawProps, sourceProps.yogaStyle)){}; + : convertRawProp(context, rawProps, sourceProps.yogaStyle)) { + if (!CoreFeatures::enablePropIteratorSetter) { + convertRawPropAliases(context, sourceProps, rawProps); + } +}; template static inline T const getFieldValue( @@ -128,6 +132,32 @@ void YogaStylableProps::setProp( REBUILD_FIELD_YG_EDGES(margin, "margin", ""); REBUILD_FIELD_YG_EDGES(padding, "padding", ""); REBUILD_FIELD_YG_EDGES(border, "border", "Width"); + + // Aliases + RAW_SET_PROP_SWITCH_CASE( + marginInline, "marginInline", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + marginInlineStart, "marginInlineStart", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + marginInlineEnd, "marginInlineEnd", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + marginBlock, "marginBlock", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + marginBlockStart, "marginBlockStart", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + marginBlockEnd, "marginBlockEnd", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + paddingInline, "paddingInline", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + paddingInlineStart, "paddingInlineStart", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + paddingInlineEnd, "paddingInlineEnd", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + paddingBlock, "paddingBlock", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + paddingBlockStart, "paddingBlockStart", CompactValue::ofUndefined()); + RAW_SET_PROP_SWITCH_CASE( + paddingBlockEnd, "paddingBlockEnd", CompactValue::ofUndefined()); } } @@ -211,4 +241,83 @@ SharedDebugStringConvertibleList YogaStylableProps::getDebugProps() const { } #endif +void YogaStylableProps::convertRawPropAliases( + const PropsParserContext &context, + YogaStylableProps const &sourceProps, + RawProps const &rawProps) { + marginInline = convertRawProp( + context, + rawProps, + "marginInline", + sourceProps.marginInline, + CompactValue::ofUndefined()); + marginInlineStart = convertRawProp( + context, + rawProps, + "marginInlineStart", + sourceProps.marginInlineStart, + CompactValue::ofUndefined()); + marginInlineEnd = convertRawProp( + context, + rawProps, + "marginInlineEnd", + sourceProps.marginInlineEnd, + CompactValue::ofUndefined()); + marginBlock = convertRawProp( + context, + rawProps, + "marginBlock", + sourceProps.marginBlock, + CompactValue::ofUndefined()); + marginBlockStart = convertRawProp( + context, + rawProps, + "marginBlockStart", + sourceProps.marginBlockStart, + CompactValue::ofUndefined()); + marginBlockEnd = convertRawProp( + context, + rawProps, + "marginBlockEnd", + sourceProps.marginBlockEnd, + CompactValue::ofUndefined()); + + paddingInline = convertRawProp( + context, + rawProps, + "paddingInline", + sourceProps.paddingInline, + CompactValue::ofUndefined()); + paddingInlineStart = convertRawProp( + context, + rawProps, + "paddingInlineStart", + sourceProps.paddingInlineStart, + CompactValue::ofUndefined()); + paddingInlineEnd = convertRawProp( + context, + rawProps, + "paddingInlineEnd", + sourceProps.paddingInlineEnd, + CompactValue::ofUndefined()); + paddingBlock = convertRawProp( + context, + rawProps, + "paddingBlock", + sourceProps.paddingBlock, + CompactValue::ofUndefined()); + paddingBlockStart = convertRawProp( + context, + rawProps, + "paddingBlockStart", + sourceProps.paddingBlockStart, + CompactValue::ofUndefined()); + paddingBlockEnd = convertRawProp( + context, + rawProps, + "paddingBlockEnd", + sourceProps.paddingBlockEnd, + CompactValue::ofUndefined()); +} + } // namespace facebook::react diff --git a/ReactCommon/react/renderer/components/view/YogaStylableProps.h b/ReactCommon/react/renderer/components/view/YogaStylableProps.h index b19a90df2f0f8a..8891436cdb0364 100644 --- a/ReactCommon/react/renderer/components/view/YogaStylableProps.h +++ b/ReactCommon/react/renderer/components/view/YogaStylableProps.h @@ -17,6 +17,8 @@ namespace facebook { namespace react { class YogaStylableProps : public Props { + using CompactValue = facebook::yoga::detail::CompactValue; + public: YogaStylableProps() = default; YogaStylableProps( @@ -37,9 +39,29 @@ class YogaStylableProps : public Props { #endif #pragma mark - Props - YGStyle yogaStyle{}; + // Duplicates of existing properties with different names, taking + // precendence. E.g. "marginBlock" instead of "marginVertical" + CompactValue marginInline; + CompactValue marginInlineStart; + CompactValue marginInlineEnd; + CompactValue marginBlock; + + CompactValue paddingInline; + CompactValue paddingInlineStart; + CompactValue paddingInlineEnd; + CompactValue paddingBlock; + + // BlockEnd/BlockStart map to top/bottom (no writing mode), but we preserve + // Yoga's precedence and prefer specific edges (e.g. top) to ones which are + // flow relative (e.g. blockStart). + CompactValue marginBlockStart; + CompactValue marginBlockEnd; + + CompactValue paddingBlockStart; + CompactValue paddingBlockEnd; + #if RN_DEBUG_STRING_CONVERTIBLE #pragma mark - DebugStringConvertible (Partial) @@ -47,6 +69,12 @@ class YogaStylableProps : public Props { SharedDebugStringConvertibleList getDebugProps() const override; #endif + + private: + void convertRawPropAliases( + const PropsParserContext &context, + YogaStylableProps const &sourceProps, + RawProps const &rawProps); }; } // namespace react