Add new APIs for implicit animations. #30
Conversation
| Performs `animations` using the timing provided. | ||
|
|
||
| @param timing The timing to be used for the animation. | ||
| @param animations The block to be executed. Any animatable properties changed within this block |
There was a problem hiding this comment.
Can we document the retain semantics of the block for callers? Is it as safe (unretained) as UIKit animations?
src/MDMMotionAnimator.h
Outdated
| */ | ||
| - (void)animateWithTiming:(MDMMotionTiming)timing | ||
| animations:(nonnull void (^)(void))animations | ||
| completion:(nonnull void(^)(void))completion; |
There was a problem hiding this comment.
I would prefer to see a single API with a nullable completion block. CATransaction allows a nullable completionBlock parameter, so we can reuse the same code.
+ (void)setCompletionBlock:(nullable void (^)(void))block;— CATransaction.h
There was a problem hiding this comment.
This is keeping in line with UIView's APIs of similar naming, so I'm not certain that we should deviate or encourage use of CATransaction (which is intended primarily for use with explicit animations)
src/private/MDMBlockAnimations.h
Outdated
| @property(nonatomic, strong, readonly) CALayer *layer; | ||
| @end | ||
|
|
||
| NSArray<MDMImplicitAction *> *MDMAnimateBlock(void (^work)(void)); |
There was a problem hiding this comment.
Nit: Maybe name the block parameter "animations"?
src/private/MDMBlockAnimations.m
Outdated
| static IMP sOriginalActionForLayerImp = NULL; | ||
|
|
||
| @interface MDMActionContext: NSObject | ||
| @property(nonatomic, strong, readonly) NSArray<MDMImplicitAction *> *actions; |
src/private/MDMBlockAnimations.m
Outdated
| } | ||
|
|
||
| - (NSArray<MDMImplicitAction *> *)actions { | ||
| return _actions; |
There was a problem hiding this comment.
Please return a copy, since the array is mutable.
src/private/MDMBlockAnimations.m
Outdated
|
|
||
| MDMActionContext *context = [sActionContext lastObject]; | ||
|
|
||
| if (context == nil) { |
There was a problem hiding this comment.
This is really nitty-gritty code. It would help me immensely to provide some comments to help guide me.
src/private/MDMBlockAnimations.m
Outdated
| MDMActionContext *context = [sActionContext lastObject]; | ||
|
|
||
| if (context == nil) { | ||
| return ((id<CAAction>(*)(id,SEL,CALayer *, NSString *)) |
There was a problem hiding this comment.
Nit: Spacing between id,SEL,CALayer
src/private/MDMBlockAnimations.m
Outdated
| } | ||
| [sActionContext addObject:[[MDMActionContext alloc] init]]; | ||
|
|
||
| work(); |
There was a problem hiding this comment.
Please perform a nil check before invoking this block.
There was a problem hiding this comment.
Nil guard is at the top of this method.
src/private/MDMBlockAnimations.m
Outdated
| } | ||
| [sActionContext addObject:[[MDMActionContext alloc] init]]; | ||
|
|
||
| work(); |
|
|
||
| CATransaction.commit() | ||
|
|
||
| XCTAssertEqual(addedAnimations.count, 0) |
There was a problem hiding this comment.
Also XCTAssert the view's alpha is immediately set?
Example usage:
This new API works similarly to UIView's
animate(withDuration:...)family of APIs. This new API enables the following key features:animate(withDuration:...)is a familiar, concise API for doing in-place animations.framewill generate both apositionand aboundsanimation. Using the explicit APIs would require animating each of those properties individually.