-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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
[linear-gradient][ios] Rewrite entire module to Swift #15168
Conversation
Co-authored-by: Expo CI <34669131+expo-ci@users.noreply.github.com>
debbba4
to
513a2e1
Compare
Hi there! 👋 I'm a bot whose goal is to ensure your contributions meet our guidelines. I've found some issues in your pull request that should be addressed (click on them for more details) 👇
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🚀
|
||
if let gradient = CGGradient(colorsSpace: colorSpace, colors: colors as CFArray, locations: locations) { | ||
let size = bounds.size | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't have to remove that line, but it looks odd to me 🤷♂️
Why
Previously only the module has been rewritten to Swift. Also we kept the legacy Objective-C implementation for one SDK cycle, so it can be removed now.
How
EXLinearGradient
toExpoLinearGradient
— following inExpoModulesCore
andExpoSystemUI
footstepsunimodule.json
(expo-module.config.json
exists)Int
in "colors" prop replaced withCGColor
CGPoint
Double
replaced withCGFloat
NSNull
parsed from JSON is not properly bridged to Swift'snil
, so I fixedfromNSObject
functionstart
,end
,locations
propsTest Plan
NCL screen for LinearGradient seems to work as expected