-
Notifications
You must be signed in to change notification settings - Fork 780
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
Radial Gradients #527
Radial Gradients #527
Conversation
3e5c3a7
to
df6ea9d
Compare
df6ea9d
to
b19942e
Compare
Generated by 🚫 Danger |
There's still rooms for improvements but I think we can already merge this then add the missing features in a future PR (support for |
|
||
func configurePoints(with startPoint: GradientStartPoint) { | ||
private func layerPoints() -> (CGPoint, CGPoint) { |
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.
Could we keep the name configurePoints
or configureGradientPoints
? It's a lot more readable imo
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.
I’d like to drop the comfigure
because it doesn’t change the layer anymore, it will simply returns the points. Beside that, I’m completely open for another name!
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.
Maybe gradientPonts
?
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.
Okay that makes sense. gradientPoints
should work 😄
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.
Great job👍, this is a very useful feature, please merge in💪.
return gradientLayer | ||
extension CALayer { | ||
|
||
fileprivate func makeImage() -> UIImage? { |
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.
I think we can use private
now
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.
Can't do that, it's still in use in the GradientDesignable
extension used for the navigation bar, otherwise, I could move it, but it's quite useful to have it in the layer extension 😬
super.init(frame: frame) | ||
layer.insertSublayer(gradientLayer, at: 0) | ||
autoresizingMask = [.flexibleWidth, .flexibleHeight] | ||
extension GradientDesignable { |
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.
Since all methods are private
, we can just mark GradientDesignable
as private
and remove the others.
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.
In fact, that's not working in swift, you can try this in a playground as an example:
final class Test {
}
private extension Test {
func test() {
print("test")
}
}
let test = Test()
test.test()
You can call the method from the private extension from outside even if the extension is private, that's why I moved the private to the methods instead of the extension.
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.
@tbaranes interesting, I didn't know that 🤣
Close #508
Currently in this PR:
GradientMode
: currently supportinglinear
andradial
Currently missing:
startPoints
(and custom) support to radial gradientAny feedbacks welcome :)