Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Node TintColor Tweaks #1666

Merged
merged 4 commits into from
Sep 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions Source/Private/_ASPendingState.mm
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@

#define __shouldSetNeedsDisplayForView(view) (flags.needsDisplay \
|| (flags.setOpaque && _flags.opaque != (view).opaque)\
|| (flags.setBackgroundColor && ![backgroundColor isEqual:(view).backgroundColor]))
|| (flags.setBackgroundColor && ![backgroundColor isEqual:(view).backgroundColor])\
|| (flags.setTintColor && ![tintColor isEqual:(view).tintColor]))

#define __shouldSetNeedsDisplayForLayer(layer) (flags.needsDisplay \
|| (flags.setOpaque && _flags.opaque != (layer).opaque)\
rahul-malik marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -104,6 +105,7 @@ @implementation _ASPendingState
CGRect frame; // Frame is only to be used for synchronous views wrapped by nodes (see setFrame:)
CGRect bounds;
UIColor *backgroundColor;
UIColor *tintColor;
CGFloat alpha;
CGFloat cornerRadius;
UIViewContentMode contentMode;
Expand Down Expand Up @@ -428,8 +430,16 @@ - (void)setBackgroundColor:(UIColor *)color
_stateToApplyFlags.setBackgroundColor = YES;
}

- (UIColor *)tintColor
{
return tintColor;
}

- (void)setTintColor:(UIColor *)newTintColor
{
if ([newTintColor isEqual:tintColor]) {
return;
}
tintColor = newTintColor;
_stateToApplyFlags.setTintColor = YES;
}
Expand Down Expand Up @@ -1097,7 +1107,7 @@ - (void)applyToView:(UIView *)view withSpecialPropertiesHandling:(BOOL)specialPr
}

if (flags.setTintColor)
view.tintColor = self.tintColor;
view.tintColor = tintColor;

if (flags.setOpaque) {
view.opaque = _flags.opaque;
Expand Down
33 changes: 33 additions & 0 deletions Tests/ASBridgedPropertiesTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ - (BOOL)test_isFlushScheduled;

@interface ASBridgedPropertiesTestView : UIView
@property (nonatomic, readonly) BOOL receivedSetNeedsLayout;
@property (nonatomic, readonly) NSUInteger setNeedsDisplayCount;
@end

@implementation ASBridgedPropertiesTestView
Expand All @@ -31,6 +32,12 @@ - (void)setNeedsLayout
[super setNeedsLayout];
}

- (void)setNeedsDisplay
{
_setNeedsDisplayCount += 1;
[super setNeedsDisplay];
}

@end

@interface ASBridgedPropertiesTestNode : ASDisplayNode
Expand Down Expand Up @@ -138,6 +145,32 @@ - (void)testThatReadingABridgedLayerPropertyInBackgroundThrowsAnException
});
}

- (void)testThatSettingTintColorSetNeedsDisplayOnView
{
ASPendingStateController *ctrl = [ASPendingStateController sharedInstance];

ASDisplayNode *node = [[ASDisplayNode alloc] initWithViewClass:ASBridgedPropertiesTestView.class];
ASBridgedPropertiesTestView *view = (ASBridgedPropertiesTestView *)node.view;
NSUInteger initialSetNeedsDisplayCount = view.setNeedsDisplayCount;
#if AS_AT_LEAST_IOS13
// This is called an extra time on iOS13 for unknown reasons. Need to Investigate.
if (@available(iOS 13.0, *)) {
XCTAssertEqual(initialSetNeedsDisplayCount, 2);
bolsinga marked this conversation as resolved.
Show resolved Hide resolved
} else {
XCTAssertEqual(initialSetNeedsDisplayCount, 1);
}
#endif

ASDispatchSyncOnOtherThread(^{
node.tintColor = UIColor.orangeColor;
});
XCTAssertNotEqualObjects(view.tintColor, UIColor.orangeColor);
XCTAssertEqual(view.setNeedsDisplayCount, initialSetNeedsDisplayCount);
[ctrl flush];
XCTAssertEqualObjects(view.tintColor, UIColor.orangeColor);
XCTAssertEqual(view.setNeedsDisplayCount, initialSetNeedsDisplayCount + 1);
}
rahul-malik marked this conversation as resolved.
Show resolved Hide resolved

- (void)testThatManuallyFlushingTheSyncControllerImmediatelyAppliesChanges
{
ASPendingStateController *ctrl = [ASPendingStateController sharedInstance];
Expand Down
11 changes: 11 additions & 0 deletions Tests/ASDisplayNodeTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -515,9 +515,15 @@ - (void)checkValuesMatchDefaults:(ASDisplayNode *)node isLayerBacked:(BOOL)isLay
XCTAssertEqual(NO, node.preservesSuperviewLayoutMargins, @"default preservesSuperviewLayoutMargins broken %@", hasLoadedView);
XCTAssertTrue(UIEdgeInsetsEqualToEdgeInsets(UIEdgeInsetsZero, node.safeAreaInsets), @"default safeAreaInsets broken %@", hasLoadedView);
XCTAssertEqual(YES, node.insetsLayoutMarginsFromSafeArea, @"default insetsLayoutMarginsFromSafeArea broken %@", hasLoadedView);
if (node.nodeLoaded) {
XCTAssertNotNil(node.tintColor, @"default tintColor broken %@", hasLoadedView); // It has been populated by the UIView.
} else {
XCTAssertNil(node.tintColor, @"default tintColor broken %@", hasLoadedView);
}
} else {
XCTAssertEqual(NO, node.userInteractionEnabled, @"layer-backed nodes do not support userInteractionEnabled %@", hasLoadedView);
XCTAssertEqual(NO, node.exclusiveTouch, @"layer-backed nodes do not support exclusiveTouch %@", hasLoadedView);
XCTAssertNil(node.tintColor, @"default tintColor broken %@", hasLoadedView);
}
}

Expand Down Expand Up @@ -589,6 +595,7 @@ - (void)checkValuesMatchSetValues:(ASDisplayNode *)node isLayerBacked:(BOOL)isLa
XCTAssertTrue(CATransform3DEqualToTransform(CATransform3DMakeScale(0.5, 0.5, 1.0), node.transform), @"transform broken %@", hasLoadedView);
XCTAssertTrue(CATransform3DEqualToTransform(CATransform3DMakeTranslation(1337, 7357, 7007), node.subnodeTransform), @"subnodeTransform broken %@", hasLoadedView);
XCTAssertEqualObjects([UIColor clearColor], node.backgroundColor, @"backgroundColor broken %@", hasLoadedView);
XCTAssertEqualObjects([UIColor orangeColor], node.tintColor, @"tintColor broken %@", hasLoadedView);
XCTAssertEqual(UIViewContentModeBottom, node.contentMode, @"contentMode broken %@", hasLoadedView);
XCTAssertEqual([[UIColor cyanColor] CGColor], node.shadowColor, @"shadowColor broken %@", hasLoadedView);
XCTAssertEqual(.5f, node.shadowOpacity, @"shadowOpacity broken %@", hasLoadedView);
Expand Down Expand Up @@ -663,6 +670,7 @@ - (void)checkSimpleBridgePropertiesSetPropagate:(BOOL)isLayerBacked
node.transform = CATransform3DMakeScale(0.5, 0.5, 1.0);
node.subnodeTransform = CATransform3DMakeTranslation(1337, 7357, 7007);
node.backgroundColor = [UIColor clearColor];
node.tintColor = [UIColor orangeColor];
node.contentMode = UIViewContentModeBottom;
node.shadowColor = [[UIColor cyanColor] CGColor];
node.shadowOpacity = .5f;
Expand Down Expand Up @@ -753,6 +761,7 @@ - (void)testPropertiesSetOffThreadBeforeLoadingExternalView
return view;
}];
node.backgroundColor = [UIColor blueColor];
node.tintColor = [UIColor orangeColor];
node.frame = CGRectMake(10, 20, 30, 40);
node.autoresizingMask = UIViewAutoresizingFlexibleWidth;
node.userInteractionEnabled = YES;
Expand All @@ -772,6 +781,7 @@ - (void)testPropertiesSetOnThreadAfterLoadingExternalView
[node view];

node.backgroundColor = [UIColor blueColor];
node.tintColor = [UIColor orangeColor];
node.frame = CGRectMake(10, 20, 30, 40);
node.autoresizingMask = UIViewAutoresizingFlexibleWidth;
node.userInteractionEnabled = YES;
Expand All @@ -784,6 +794,7 @@ - (void)checkExternalViewAppliedPropertiesMatch:(ASDisplayNode *)node
UIView *view = node.view;

XCTAssertEqualObjects([UIColor blueColor], view.backgroundColor, @"backgroundColor not propagated to view");
XCTAssertEqualObjects([UIColor orangeColor], view.tintColor, @"tintColor not propagated to view");
XCTAssertTrue(CGRectEqualToRect(CGRectMake(10, 20, 30, 40), view.frame), @"frame not propagated to view");
XCTAssertEqual(UIViewAutoresizingFlexibleWidth, view.autoresizingMask, @"autoresizingMask not propagated to view");
XCTAssertEqual(YES, view.userInteractionEnabled, @"userInteractionEnabled not propagated to view");
Expand Down