Permalink
Browse files

fix view clipping to operate on ui hierachy

Summary:
There was a bug in the view clipping logic.
Clipping works on uiview hierarchy, but I've been using `reactSuperview` to get clipping rect for my parent.
This is incorrect in a case where these two hierarchies don't match and there are some views between a view and its `reactSuperview`.

So we should really use normal `superview`. A minor complication is that `superview` is `nil` if we are clipped.
We could remember what our last `superview` was, but that's extra data we have to manage. Instead I use one clever trick to avoid doing so.
(Let me know if it makes sense based on my inline documentation.)

Reviewed By: mmmulani

Differential Revision: D4182647

fbshipit-source-id: 779cbcec0bd08eb270c3727c9c5cb9c080c4a2a4
  • Loading branch information...
1 parent b123cc2 commit 26e373c903011bfa46000ff045777d802970d779 @majak majak committed with Facebook Github Bot Nov 16, 2016
Showing with 41 additions and 2 deletions.
  1. +35 −0 Examples/UIExplorer/UIExplorerUnitTests/RCTSubviewClippingTests.m
  2. +6 −2 React/Views/UIView+React.m
@@ -369,4 +369,39 @@ - (void)testScrollViewClipsDuringScrolling
XCTAssert([[NSSet setWithArray:contentView.subviews] isEqualToSet:[NSSet setWithArray:(@[rowView2, rowView3])]]);
}
+
+/**
+ In this test case the react view hiearchy is
+ clippingView -> directReactChildView -> deeperChildView
+ while the uiview hierarchy is
+ clippingView -> nonReactChildView -> directReactChildView -> deeperChildView
+ */
+- (void)testClippingWhenReactHierarchyDoesntMatchUIHierarchy
+{
+ RCTView *clippingView = [RCTView new];
+ [clippingView reactSetFrame:CGRectMake(0, 0, 50, 50)];
+
+ RCTView *directReactChildView = [RCTView new];
+ [directReactChildView reactSetFrame:CGRectMake(-50, 0, 100, 50)];
+ [clippingView insertReactSubview:directReactChildView atIndex:0];
+ [clippingView didUpdateReactSubviews];
+
+ RCTView *deeperChildView = [RCTView new];
+ [deeperChildView reactSetFrame:CGRectMake(0, 0, 50, 50)];
+ [directReactChildView insertReactSubview:deeperChildView atIndex:0];
+ [directReactChildView didUpdateReactSubviews];
+
+ UIView *nonReactChildView = [UIView new];
+ [nonReactChildView setFrame:CGRectMake(50, 0, 50, 50)];
+ [clippingView addSubview:nonReactChildView];
+ [nonReactChildView addSubview:directReactChildView];
+
+ [clippingView rct_setRemovesClippedSubviews:YES];
+ [directReactChildView reactSetFrame:CGRectMake(-50, 0, 99, 50)];
+
+ XCTAssertEqual(clippingView.subviews.count, 1u);
+ XCTAssertEqual(nonReactChildView.subviews.count, 1u);
+ XCTAssertEqual(directReactChildView.subviews.count, 1u);
+}
+
@end
@@ -287,8 +287,12 @@ - (void)rct_reclip
if (!self.rct_nextClippingView && !self.rct_removesClippedSubviews) {
return;
}
- // If we are currently clipped our active clipping rect would be null rect. That's why we ask for out superview's.
- CGRect clippingRectForSuperview = self.reactSuperview ? [self.reactSuperview rct_activeClippingRect] : CGRectInfinite;
+ // If we are currently clipped our active clipping rect would be null rect. That's why we ask for our superview's.
+ // Actually it's not that simple. Clipping logic operates on uiview hierarchy. If the current view is clipped we cannot use its superview, since its nil.
+ // Fortunately we can use `reactSuperview`. UI and React view hierachies doesn't have to match, but when they don't match we don't clip.
+ // Therefore because this view is clipped it means its reactSuperview is the same as its (clipped) UI superview.
+ UIView *clippingRectSource = self.superview ? self.superview : self.reactSuperview;
+ CGRect clippingRectForSuperview = clippingRectSource ? [clippingRectSource rct_activeClippingRect] : CGRectInfinite;
if (CGRectIsNull(clippingRectForSuperview)) {
return;
}

0 comments on commit 26e373c

Please sign in to comment.