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

Update ASPendingState backgroundColor type to UIColor #1592

Merged

Conversation

rahul-malik
Copy link
Contributor

@rahul-malik rahul-malik commented Jul 19, 2019

Given that UIColor is now a dynamic provider we can no longer rely on the fact that there is a 1:1 mapping between UIColor <-> CGColorRef.

I've updated the UIViewBridge and ASPendingState accordingly to use UIColor and translate to CGColorRef only when interacting with layers. This should preserve any dynamic capabilities we might need in the future when trying to re-render layer backed nodes.

Depends on #1588

@rahul-malik rahul-malik requested review from maicki and nguyenhuy July 19, 2019 17:16
}

- (void)setBackgroundColor:(UIColor *)newBackgroundColor
{
_bridge_prologue_write;

CGColorRef newBackgroundCGColor = [newBackgroundColor CGColor];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Since newBackgroundCGColor is always set below, let's leave it nil at first.

Source/Private/ASDisplayNode+UIViewBridge.mm Show resolved Hide resolved
@@ -18,7 +18,7 @@

#define __shouldSetNeedsDisplay(layer) (flags.needsDisplay \
|| (flags.setOpaque && _flags.opaque != (layer).opaque)\
|| (flags.setBackgroundColor && !CGColorEqualToColor(backgroundColor, (layer).backgroundColor)))
|| (flags.setBackgroundColor && [backgroundColor isEqual:[UIColor colorWithCGColor:(layer).backgroundColor]]))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
|| (flags.setBackgroundColor && [backgroundColor isEqual:[UIColor colorWithCGColor:(layer).backgroundColor]]))
|| (flags.setBackgroundColor && ! [backgroundColor isEqual:[UIColor colorWithCGColor:(layer).backgroundColor]]))

@rahul-malik rahul-malik force-pushed the rmalik/cgcolor-uicolor branch from 96dd70c to 5f06a7b Compare July 19, 2019 19:59
Copy link
Member

@nguyenhuy nguyenhuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@nguyenhuy nguyenhuy merged commit dc2ae68 into rmalik/fix-uiview-bridge-backgroundcolor Jul 20, 2019
rahul-malik added a commit that referenced this pull request Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants