From a6e1e33a509ff731524038a3afb19099335045ed Mon Sep 17 00:00:00 2001 From: Emil Sjolander Date: Wed, 14 Sep 2016 11:25:57 -0700 Subject: [PATCH] Reverted commit D3855801 Summary: Introduce `overflow:scroll` so that scrolling can be implemented without the current overflow:visible hackiness. Currently we use AT_MOST to measure in the cross axis but not in the main axis. This was done to enable scrolling containers where children are not constraint in the main axis by their parent. This caused problems for non-scrolling containers though as it meant that their children cannot be measured correctly in the main axis. Introducing `overflow:scroll` fixes this. Reviewed By: astreet Differential Revision: D3855801 fbshipit-source-id: 3c365f9e6ef612fd9d9caaaa8c650e9702176e77 --- Libraries/Components/ScrollView/ScrollView.js | 2 - .../Components/View/ViewStylePropTypes.js | 1 + Libraries/StyleSheet/LayoutPropTypes.js | 13 ------ React/Base/RCTConvert.m | 3 +- React/CSSLayout/CSSLayout.c | 42 +++++++++++-------- React/CSSLayout/CSSLayout.h | 1 - React/Views/RCTViewManager.m | 2 +- .../com/facebook/csslayout/CSSOverflow.java | 1 - .../com/facebook/csslayout/LayoutEngine.java | 20 +++++---- .../react/uimanager/LayoutShadowNode.java | 7 ---- .../facebook/react/uimanager/ViewProps.java | 2 - 11 files changed, 39 insertions(+), 55 deletions(-) diff --git a/Libraries/Components/ScrollView/ScrollView.js b/Libraries/Components/ScrollView/ScrollView.js index 5b099cf49..f96b2d93a 100644 --- a/Libraries/Components/ScrollView/ScrollView.js +++ b/Libraries/Components/ScrollView/ScrollView.js @@ -547,12 +547,10 @@ const styles = StyleSheet.create({ baseVertical: { flex: 1, flexDirection: 'column', - overflow: 'scroll', }, baseHorizontal: { flex: 1, flexDirection: 'row', - overflow: 'scroll', }, contentContainerHorizontal: { flexDirection: 'row', diff --git a/Libraries/Components/View/ViewStylePropTypes.js b/Libraries/Components/View/ViewStylePropTypes.js index 9990b9971..136ab5c50 100644 --- a/Libraries/Components/View/ViewStylePropTypes.js +++ b/Libraries/Components/View/ViewStylePropTypes.js @@ -43,6 +43,7 @@ var ViewStylePropTypes = { borderBottomWidth: ReactPropTypes.number, borderLeftWidth: ReactPropTypes.number, opacity: ReactPropTypes.number, + overflow: ReactPropTypes.oneOf(['visible', 'hidden']), /** * (Android-only) Sets the elevation of a view, using Android's underlying * [elevation API](https://developer.android.com/training/material/shadows-clipping.html#Elevation). diff --git a/Libraries/StyleSheet/LayoutPropTypes.js b/Libraries/StyleSheet/LayoutPropTypes.js index ea1020ad9..3ac6aff64 100644 --- a/Libraries/StyleSheet/LayoutPropTypes.js +++ b/Libraries/StyleSheet/LayoutPropTypes.js @@ -328,19 +328,6 @@ var LayoutPropTypes = { 'stretch' ]), - /** `overflow` controls how a children are measured and displayed. - * `overflow: hidden` causes views to be clipped while `overflow: scroll` - * causes views to be measured independently of their parents main axis.` - * It works like `overflow` in CSS (default: visible). - * See https://developer.mozilla.org/en/docs/Web/CSS/overflow - * for more details. - */ - overflow: ReactPropTypes.oneOf([ - 'visible', - 'hidden', - 'scroll', - ]), - /** In React Native `flex` does not work the same way that it does in CSS. * `flex` is a number rather than a string, and it works * according to the `css-layout` library diff --git a/React/Base/RCTConvert.m b/React/Base/RCTConvert.m index 6b1997605..a27afb59e 100644 --- a/React/Base/RCTConvert.m +++ b/React/Base/RCTConvert.m @@ -612,8 +612,7 @@ RCT_ENUM_CONVERTER(css_backface_visibility_t, (@{ RCT_ENUM_CONVERTER(CSSOverflow, (@{ @"hidden": @(CSSOverflowHidden), - @"visible": @(CSSOverflowVisible), - @"scroll": @(CSSOverflowScroll), + @"visible": @(CSSOverflowVisible) }), CSSOverflowVisible, intValue) RCT_ENUM_CONVERTER(CSSFlexDirection, (@{ diff --git a/React/CSSLayout/CSSLayout.c b/React/CSSLayout/CSSLayout.c index f4d3a68ca..7a5766eda 100644 --- a/React/CSSLayout/CSSLayout.c +++ b/React/CSSLayout/CSSLayout.c @@ -451,8 +451,6 @@ print_css_node_rec(const CSSNodeRef node, const CSSPrintOptions options, const u printf("overflow: 'hidden', "); } else if (node->style.overflow == CSSOverflowVisible) { printf("overflow: 'visible', "); - } else if (node->style.overflow == CSSOverflowScroll) { - printf("overflow: 'scroll', "); } if (four_equal(node->style.margin)) { @@ -557,7 +555,8 @@ static bool isColumnDirection(const CSSFlexDirection flexDirection) { } static float getLeadingMargin(const CSSNodeRef node, const CSSFlexDirection axis) { - if (isRowDirection(axis) && !CSSValueIsUndefined(node->style.margin[CSSEdgeStart])) { + if (isRowDirection(axis) && + !CSSValueIsUndefined(node->style.margin[CSSEdgeStart])) { return node->style.margin[CSSEdgeStart]; } @@ -565,7 +564,8 @@ static float getLeadingMargin(const CSSNodeRef node, const CSSFlexDirection axis } static float getTrailingMargin(const CSSNodeRef node, const CSSFlexDirection axis) { - if (isRowDirection(axis) && !CSSValueIsUndefined(node->style.margin[CSSEdgeEnd])) { + if (isRowDirection(axis) && + !CSSValueIsUndefined(node->style.margin[CSSEdgeEnd])) { return node->style.margin[CSSEdgeEnd]; } @@ -573,7 +573,8 @@ static float getTrailingMargin(const CSSNodeRef node, const CSSFlexDirection axi } static float getLeadingPadding(const CSSNodeRef node, const CSSFlexDirection axis) { - if (isRowDirection(axis) && !CSSValueIsUndefined(node->style.padding[CSSEdgeStart]) && + if (isRowDirection(axis) && + !CSSValueIsUndefined(node->style.padding[CSSEdgeStart]) && node->style.padding[CSSEdgeStart] >= 0) { return node->style.padding[CSSEdgeStart]; } @@ -586,7 +587,8 @@ static float getLeadingPadding(const CSSNodeRef node, const CSSFlexDirection axi } static float getTrailingPadding(const CSSNodeRef node, const CSSFlexDirection axis) { - if (isRowDirection(axis) && !CSSValueIsUndefined(node->style.padding[CSSEdgeEnd]) && + if (isRowDirection(axis) && + !CSSValueIsUndefined(node->style.padding[CSSEdgeEnd]) && node->style.padding[CSSEdgeEnd] >= 0) { return node->style.padding[CSSEdgeEnd]; } @@ -599,7 +601,8 @@ static float getTrailingPadding(const CSSNodeRef node, const CSSFlexDirection ax } static float getLeadingBorder(const CSSNodeRef node, const CSSFlexDirection axis) { - if (isRowDirection(axis) && !CSSValueIsUndefined(node->style.border[CSSEdgeStart]) && + if (isRowDirection(axis) && + !CSSValueIsUndefined(node->style.border[CSSEdgeStart]) && node->style.border[CSSEdgeStart] >= 0) { return node->style.border[CSSEdgeStart]; } @@ -612,7 +615,8 @@ static float getLeadingBorder(const CSSNodeRef node, const CSSFlexDirection axis } static float getTrailingBorder(const CSSNodeRef node, const CSSFlexDirection axis) { - if (isRowDirection(axis) && !CSSValueIsUndefined(node->style.border[CSSEdgeEnd]) && + if (isRowDirection(axis) && + !CSSValueIsUndefined(node->style.border[CSSEdgeEnd]) && node->style.border[CSSEdgeEnd] >= 0) { return node->style.border[CSSEdgeEnd]; } @@ -1136,17 +1140,21 @@ static void layoutNodeImpl(const CSSNodeRef node, childHeightMeasureMode = CSSMeasureModeExactly; } - // The W3C spec doesn't say anything about the 'overflow' property, - // but all major browsers appear to implement the following logic. - if ((!isMainAxisRow && node->style.overflow == CSSOverflowScroll) || node->style.overflow != CSSOverflowScroll) { - if (CSSValueIsUndefined(childWidth) && !CSSValueIsUndefined(availableInnerWidth)) { - childWidth = availableInnerWidth; - childWidthMeasureMode = CSSMeasureModeAtMost; - } + // According to the spec, if the main size is not definite and the + // child's inline axis is parallel to the main axis (i.e. it's + // horizontal), the child should be sized using "UNDEFINED" in + // the main size. Otherwise use "AT_MOST" in the cross axis. + if (!isMainAxisRow && CSSValueIsUndefined(childWidth) && + !CSSValueIsUndefined(availableInnerWidth)) { + childWidth = availableInnerWidth; + childWidthMeasureMode = CSSMeasureModeAtMost; } - if ((isMainAxisRow && node->style.overflow == CSSOverflowScroll) || node->style.overflow != CSSOverflowScroll) { - if (CSSValueIsUndefined(childHeight) && !CSSValueIsUndefined(availableInnerHeight)) { + // The W3C spec doesn't say anything about the 'overflow' property, + // but all major browsers appear to implement the following logic. + if (node->style.overflow == CSSOverflowHidden) { + if (isMainAxisRow && CSSValueIsUndefined(childHeight) && + !CSSValueIsUndefined(availableInnerHeight)) { childHeight = availableInnerHeight; childHeightMeasureMode = CSSMeasureModeAtMost; } diff --git a/React/CSSLayout/CSSLayout.h b/React/CSSLayout/CSSLayout.h index 539b8ab03..3ebce6855 100644 --- a/React/CSSLayout/CSSLayout.h +++ b/React/CSSLayout/CSSLayout.h @@ -55,7 +55,6 @@ typedef enum CSSJustify { typedef enum CSSOverflow { CSSOverflowVisible, CSSOverflowHidden, - CSSOverflowScroll, } CSSOverflow; // Note: auto is only a valid value for alignSelf. It is NOT a valid value for diff --git a/React/Views/RCTViewManager.m b/React/Views/RCTViewManager.m index 012200d1f..ea8b92d8b 100644 --- a/React/Views/RCTViewManager.m +++ b/React/Views/RCTViewManager.m @@ -122,7 +122,7 @@ RCT_REMAP_VIEW_PROPERTY(shadowRadius, layer.shadowRadius, CGFloat) RCT_CUSTOM_VIEW_PROPERTY(overflow, CSSOverflow, RCTView) { if (json) { - view.clipsToBounds = [RCTConvert CSSOverflow:json] != CSSOverflowVisible; + view.clipsToBounds = [RCTConvert CSSOverflow:json] == CSSOverflowHidden; } else { view.clipsToBounds = defaultView.clipsToBounds; } diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSOverflow.java b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSOverflow.java index 9aae82250..29957a9a7 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/CSSOverflow.java +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/CSSOverflow.java @@ -12,5 +12,4 @@ package com.facebook.csslayout; public enum CSSOverflow { VISIBLE, HIDDEN, - SCROLL, } diff --git a/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java b/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java index 573369493..b8edf8f97 100644 --- a/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java +++ b/ReactAndroid/src/main/java/com/facebook/csslayout/LayoutEngine.java @@ -715,17 +715,19 @@ public class LayoutEngine { childHeightMeasureMode = CSSMeasureMode.EXACTLY; } - // The W3C spec doesn't say anything about the 'overflow' property, - // but all major browsers appear to implement the following logic. - if ((!isMainAxisRow && node.style.overflow == CSSOverflow.SCROLL) || node.style.overflow != CSSOverflow.SCROLL) { - if (Float.isNaN(childWidth) && !Float.isNaN(availableInnerWidth)) { - childWidth = availableInnerWidth; - childWidthMeasureMode = CSSMeasureMode.AT_MOST; - } + // According to the spec, if the main size is not definite and the + // child's inline axis is parallel to the main axis (i.e. it's + // horizontal), the child should be sized using "UNDEFINED" in + // the main size. Otherwise use "AT_MOST" in the cross axis. + if (!isMainAxisRow && Float.isNaN(childWidth) && !Float.isNaN(availableInnerWidth)) { + childWidth = availableInnerWidth; + childWidthMeasureMode = CSSMeasureMode.AT_MOST; } - if ((isMainAxisRow && node.style.overflow == CSSOverflow.SCROLL) || node.style.overflow != CSSOverflow.SCROLL) { - if (Float.isNaN(childHeight) && !Float.isNaN(availableInnerHeight)) { + // The W3C spec doesn't say anything about the 'overflow' property, + // but all major browsers appear to implement the following logic. + if (node.style.overflow == CSSOverflow.HIDDEN) { + if (isMainAxisRow && Float.isNaN(childHeight) && !Float.isNaN(availableInnerHeight)) { childHeight = availableInnerHeight; childHeightMeasureMode = CSSMeasureMode.AT_MOST; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java index ae5828323..710153085 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/LayoutShadowNode.java @@ -10,7 +10,6 @@ import com.facebook.csslayout.CSSAlign; import com.facebook.csslayout.CSSConstants; import com.facebook.csslayout.CSSFlexDirection; import com.facebook.csslayout.CSSJustify; -import com.facebook.csslayout.CSSOverflow; import com.facebook.csslayout.CSSPositionType; import com.facebook.csslayout.CSSWrap; import com.facebook.react.uimanager.annotations.ReactProp; @@ -103,12 +102,6 @@ public class LayoutShadowNode extends ReactShadowNode { justifyContent.toUpperCase(Locale.US).replace("-", "_"))); } - @ReactProp(name = ViewProps.OVERFLOW) - public void setOverflow(@Nullable String overflow) { - setOverflow(overflow == null ? CSSOverflow.VISIBLE : CSSOverflow.valueOf( - overflow.toUpperCase(Locale.US).replace("-", "_"))); - } - @ReactPropGroup(names = { ViewProps.MARGIN, ViewProps.MARGIN_VERTICAL, diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java index ef9c9956e..469e0c708 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewProps.java @@ -26,7 +26,6 @@ public class ViewProps { // !!! Keep in sync with LAYOUT_ONLY_PROPS below public static final String ALIGN_ITEMS = "alignItems"; public static final String ALIGN_SELF = "alignSelf"; - public static final String OVERFLOW = "overflow"; public static final String BOTTOM = "bottom"; public static final String COLLAPSABLE = "collapsable"; public static final String FLEX = "flex"; @@ -114,7 +113,6 @@ public class ViewProps { FLEX_DIRECTION, FLEX_WRAP, JUSTIFY_CONTENT, - OVERFLOW, /* position */ POSITION,