-
Notifications
You must be signed in to change notification settings - Fork 17
Add list of explicitly animatable properties #102
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #102 +/- ##
===========================================
- Coverage 86.08% 85.83% -0.25%
===========================================
Files 26 25 -1
Lines 2544 2605 +61
Branches 154 156 +2
===========================================
+ Hits 2190 2236 +46
- Misses 340 355 +15
Partials 14 14
Continue to review full report at Codecov.
|
|
Ready for review. |
README.md
Outdated
| <tr><td>🎉</td><td>Implicit and explicit additive animations.</td></tr> | ||
| <tr><td>🎉</td><td>Parameterized motion with the <a href="https://github.com/material-motion/motion-interchange-objc">Interchange</a>.</td></tr> | ||
| <tr><td>🎉</td><td>Provide velocity to animations directly from gesture recognizers.</td></tr> | ||
| <tr><td>🎉</td><td>Minimize jank by relying more on Core Animation.</td></tr> |
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 we come up with a more precise term than "jank"? What exactly are we minimizing? Is it measurable?
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.
|
Sorry, I think this is just too long for me to sit and review all at once. |
|
Ah that's fair - I can split it up into separate pieces. |
464ef6e to
64e058e
Compare
|
This PR is ready for re-review. It now includes only the animatable key paths charts and additions of the quiz tests to the unit tests. |
| XCTAssert(view.layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = view.layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration() + 0.1) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(view.layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = view.layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration() + 0.1) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration()) |
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: Please use XCTAssertEqualWithAccuracy
| func testDoesImplicitlyAnimateInCATransaction() { | ||
| CATransaction.begin() | ||
| CATransaction.setAnimationDuration(0.5) | ||
| func testDoesImplicitlyAnimate() { |
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: "What is implicitly animating?" testUnhostedLayerInWindowDoesImplicitlyAnimate?
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration()) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration()) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration()) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration()) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, CATransaction.animationDuration()) |
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: Please use XCTAssertEqualWithAccuracy
| XCTAssert(layer.animation(forKey: "opacity") is CABasicAnimation) | ||
| if let animation = layer.animation(forKey: "opacity") as? CABasicAnimation { | ||
| XCTAssertEqual(animation.keyPath, "opacity") | ||
| XCTAssertEqual(animation.duration, traits.duration) |
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: Please use XCTAssertEqualWithAccuracy
No description provided.