-
Notifications
You must be signed in to change notification settings - Fork 17
Only animate CGSize and CGPoint key paths additively if additive is enabled. #72
Conversation
…nabled. Prior to this change, CGSize and CGPoint animations were being animated additively even if the additive property was disabled. Added tests to catch regressions in the future.
|
||
timing = MotionTiming(delay: 0, | ||
duration: 1, | ||
curve: MotionCurveMakeBezier(p1x: 0, p1y: 0, p2x: 0, p2y: 0), |
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'm probably misunderstanding timing curves, but doesn't this timing curve mean "never go to the 'to' value"? If all control points are zero?
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.
There are four control points in a cubic bezier, p0 and p3 represent the anchor points — respectively anchored at 0, 0, and 1,1 — and p1 and p2 are the control points. This API accepts p1 and p2.
While this particular cubic bezier is odd, it's not particularly important for the purposes of this test.
XCTAssertTrue(animation.isAdditive, "Animation is not additive when it should be.") | ||
} | ||
|
||
func testPositionKeyPathsAnimateAdditively() { |
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.
Nit/naming: test__Point__KeyPathsAnimateAdditively
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.
Done.
|
||
XCTAssertTrue(animation.isAdditive, "Animation is not additive when it should be.") | ||
} | ||
} |
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.
Are CGRect not additive no matter what?
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.
Currently no. CGRect support isn't currently implemented.
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.
Prior to this change, CGSize and CGPoint animations were being animated additively even if the additive property was disabled.
Added tests to catch regressions in the future.