-
Notifications
You must be signed in to change notification settings - Fork 560
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
Morphing animation scale/translate/rotation #427
Conversation
# Conflicts: # Source/svg/SVGCanvas.swift
Source/svg/SVGCanvas.swift
Outdated
@@ -19,7 +19,7 @@ class SVGCanvas: Group { | |||
layout.layout(node: self, in: size) | |||
return 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.
Need to use linter
Source/svg/SVGNodeLayout.swift
Outdated
@@ -40,6 +40,7 @@ class SVGSize { | |||
|
|||
protocol NodeLayout { | |||
|
|||
var svgSize: SVGSize { get } |
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.
Avoid using svgSize
in general protocol
Source/views/MacawView.swift
Outdated
if let view = nodesMap.getView(canvas) { | ||
rect = canvas.layout(size: view.bounds.size.toMacaw()).rect() | ||
} else { | ||
rect = Group(contents: canvas.contents).bounds |
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.
Not really effective. It's better to use utility method to get bounds for list of nodes which will be used here and in Group.bounds
@@ -31,6 +31,7 @@ open class Group: Node { | |||
tag: tag | |||
) | |||
|
|||
self.contents = contents |
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 change both contentsVar
and contents
which is not necessary, because contentsVar
will be updated automatically.
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.
node
property for contentsVar
is being set up here, not the contents which indeed will be updated automatically
No description provided.