Better (right) implementation of intrinsicContentSize

Reviewed By: emilsjolander Differential Revision: D4486767 fbshipit-source-id: d37ea11f9f48425d4d99c29e8bfb6c8ed2353f04
facebook · Feb 1, 2017 · d1990f8 · d1990f8
1 parent 31099aa
commit d1990f8
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 36 deletions.
diff --git a/Examples/UIExplorer/UIExplorerUnitTests/RCTShadowViewTests.m b/Examples/UIExplorer/UIExplorerUnitTests/RCTShadowViewTests.m
@@ -28,11 +28,10 @@ - (void)setUp
 {
   [super setUp];
 
-  self.parentView = [self _shadowViewWithConfig:^(YGNodeRef node) {
+  self.parentView = [RCTRootShadowView new];
-    YGNodeStyleSetFlexDirection(node, YGFlexDirectionColumn);
+  YGNodeStyleSetFlexDirection(self.parentView.cssNode, YGFlexDirectionColumn);
-    YGNodeStyleSetWidth(node, 440);
+  YGNodeStyleSetWidth(self.parentView.cssNode, 440);
-    YGNodeStyleSetHeight(node, 440);
+  YGNodeStyleSetHeight(self.parentView.cssNode, 440);
-  }];
   self.parentView.reactTag = @1; // must be valid rootView tag
 }
 
@@ -132,6 +131,7 @@ - (void)testAncestorCheck
 - (void)testAssignsSuggestedWidthDimension
 {
   [self _withShadowViewWithStyle:^(YGNodeRef node) {
+                                   YGNodeStyleSetPositionType(node, YGPositionTypeAbsolute);
                                    YGNodeStyleSetPosition(node, YGEdgeLeft, 0);
                                    YGNodeStyleSetPosition(node, YGEdgeTop, 0);
                                    YGNodeStyleSetHeight(node, 10);
@@ -143,6 +143,7 @@ - (void)testAssignsSuggestedWidthDimension
 - (void)testAssignsSuggestedHeightDimension
 {
   [self _withShadowViewWithStyle:^(YGNodeRef node) {
+                                   YGNodeStyleSetPositionType(node, YGPositionTypeAbsolute);
                                    YGNodeStyleSetPosition(node, YGEdgeLeft, 0);
                                    YGNodeStyleSetPosition(node, YGEdgeTop, 0);
                                    YGNodeStyleSetWidth(node, 10);
@@ -154,6 +155,7 @@ - (void)testAssignsSuggestedHeightDimension
 - (void)testDoesNotOverrideDimensionStyleWithSuggestedDimensions
 {
   [self _withShadowViewWithStyle:^(YGNodeRef node) {
+                                   YGNodeStyleSetPositionType(node, YGPositionTypeAbsolute);
                                    YGNodeStyleSetPosition(node, YGEdgeLeft, 0);
                                    YGNodeStyleSetPosition(node, YGEdgeTop, 0);
                                    YGNodeStyleSetWidth(node, 10);
@@ -189,11 +191,12 @@ - (void)_withShadowViewWithStyle:(void(^)(YGNodeRef node))configBlock
                 NSStringFromCGRect(actualRect));
 }
 
-- (RCTRootShadowView *)_shadowViewWithConfig:(void(^)(YGNodeRef node))configBlock
+- (RCTShadowView *)_shadowViewWithConfig:(void(^)(YGNodeRef node))configBlock
 {
-  RCTRootShadowView *shadowView = [RCTRootShadowView new];
+  RCTShadowView *shadowView = [RCTShadowView new];
   configBlock(shadowView.cssNode);
   return shadowView;
 }
 
+
 @end
diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m
@@ -473,11 +473,12 @@ - (void)setIntrinsicContentSize:(CGSize)size forView:(UIView *)view
   NSNumber *reactTag = view.reactTag;
   dispatch_async(RCTGetUIManagerQueue(), ^{
     RCTShadowView *shadowView = self->_shadowViewRegistry[reactTag];
-    RCTAssert(shadowView != nil, @"Could not locate root view with tag #%@", reactTag);
+    RCTAssert(shadowView != nil, @"Could not locate view with tag #%@", reactTag);
-
-    shadowView.intrinsicContentSize = size;
 
-    [self setNeedsLayout];
+    if (!CGSizeEqualToSize(shadowView.intrinsicContentSize, size)) {
+      shadowView.intrinsicContentSize = size;
+      [self setNeedsLayout];
+    }
   });
 }
 

diff --git a/React/Views/RCTShadowView.h b/React/Views/RCTShadowView.h
@@ -81,14 +81,15 @@ typedef void (^RCTApplierBlock)(NSDictionary<NSNumber *, UIView *> *viewRegistry
 
 @property (nonatomic, assign) CGRect frame;
 
-- (void)setTopLeft:(CGPoint)topLeft;
-- (void)setSize:(CGSize)size;
-
 /**
- * Set the natural size of the view, which is used when no explicit size is set.
+ * Represents the natural size of the view, which is used when explicit size is not set or is ambiguous.
- * Use UIViewNoIntrinsicMetric to ignore a dimension.
+ * Defaults to `{UIViewNoIntrinsicMetric, UIViewNoIntrinsicMetric}`.
  */
-- (void)setIntrinsicContentSize:(CGSize)size;
+@property (nonatomic, assign) CGSize intrinsicContentSize;
+
+
+- (void)setTopLeft:(CGPoint)topLeft;
+- (void)setSize:(CGSize)size;
 
 /**
  * Border. Defaults to { 0, 0, 0, 0 }.

diff --git a/React/Views/RCTShadowView.m b/React/Views/RCTShadowView.m
@@ -301,6 +301,8 @@ - (instancetype)init
       _borderMetaProps[ii] = YGValueUndefined;
     }
 
+    _intrinsicContentSize = CGSizeMake(UIViewNoIntrinsicMetric, UIViewNoIntrinsicMetric);
+
     _newView = YES;
     _propagationLifecycle = RCTUpdateLifecycleUninitialized;
     _textLifecycle = RCTUpdateLifecycleUninitialized;
@@ -556,30 +558,59 @@ - (void)setFrame:(CGRect)frame
   }
 }
 
-static inline void RCTAssignSuggestedDimension(YGNodeRef cssNode, YGDimension dimension, CGFloat amount)
+static inline YGSize RCTShadowViewMeasure(YGNodeRef node, float width, YGMeasureMode widthMode, float height, YGMeasureMode heightMode)
-{
+{
-  if (amount != UIViewNoIntrinsicMetric) {
+  RCTShadowView *shadowView = (__bridge RCTShadowView *)YGNodeGetContext(node);
-    switch (dimension) {
+
-      case YGDimensionWidth:
+  CGSize intrinsicContentSize = shadowView->_intrinsicContentSize;
-        if (YGNodeStyleGetWidth(cssNode).unit == YGUnitUndefined) {
+  // Replace `UIViewNoIntrinsicMetric` (which equals `-1`) with zero.
-          YGNodeStyleSetWidth(cssNode, amount);
+  intrinsicContentSize.width = MAX(0, intrinsicContentSize.width);
-        }
+  intrinsicContentSize.height = MAX(0, intrinsicContentSize.height);
-        break;
+
-      case YGDimensionHeight:
+  YGSize result;
-        if (YGNodeStyleGetHeight(cssNode).unit == YGUnitUndefined) {
+
-          YGNodeStyleSetHeight(cssNode, amount);
+  switch (widthMode) {
-        }
+    case YGMeasureModeUndefined:
-        break;
+      result.width = intrinsicContentSize.width;
-    }
+      break;
+    case YGMeasureModeExactly:
+      result.width = width;
+      break;
+    case YGMeasureModeAtMost:
+      result.width = MIN(width, intrinsicContentSize.width);
+      break;
+  }
+
+  switch (heightMode) {
+    case YGMeasureModeUndefined:
+      result.height = intrinsicContentSize.height;
+      break;
+    case YGMeasureModeExactly:
+      result.height = height;
+      break;
+    case YGMeasureModeAtMost:
+      result.height = MIN(height, intrinsicContentSize.height);
+      break;
   }
+
+  return result;
 }
 
-- (void)setIntrinsicContentSize:(CGSize)size
+- (void)setIntrinsicContentSize:(CGSize)intrinsicContentSize
 {
-  if (YGNodeStyleGetFlexGrow(_cssNode) == 0 && YGNodeStyleGetFlexShrink(_cssNode) == 0) {
+  if (CGSizeEqualToSize(_intrinsicContentSize, intrinsicContentSize)) {
-    RCTAssignSuggestedDimension(_cssNode, YGDimensionHeight, size.height);
+    return;
-    RCTAssignSuggestedDimension(_cssNode, YGDimensionWidth, size.width);
   }
+
+  _intrinsicContentSize = intrinsicContentSize;
+
+  if (CGSizeEqualToSize(_intrinsicContentSize, CGSizeMake(UIViewNoIntrinsicMetric, UIViewNoIntrinsicMetric))) {
+    YGNodeSetMeasureFunc(_cssNode, NULL);
+  } else {
+    YGNodeSetMeasureFunc(_cssNode, RCTShadowViewMeasure);
+  }
+
+  YGNodeMarkDirty(_cssNode);
 }
 
 - (void)setTopLeft:(CGPoint)topLeft