Makes private and improves animatedBox in AbstractPostListViewControl.

[diegoreymendez]

Description:

• Improves animatedBox in AbstractPostListViewController by making it non-optional, and by making it animate.
• Makes animatedBox private in AbstractPostListViewController.
• Fixes a curious bug in WPAnimatedBox.m.

Additional info:

The way in which WPAnimatedBox is being animated now is kinda hackish - since the delay we added is not guaranteed to work. A refactor to this classes would be better.

However, since the animation not working would be low-impact, I think the solution should be acceptable until I can get around to actually refactoring these classes.

To test:

1. Launch the app
2. Go to the list of blogs and pick one
3. Open to the list of pages or posts
4. Pull down the list to refresh and make sure that while loading posts the

/cc @frosty for review
/fyi @aerych (cos you know this code better).

[frosty]

It looks like the animated box is never modified, so this can be made a constant. You can also just initialise it directly: private let animatedBox = WPAnimatedBox.newAnimatedBox().

[frosty]

This looks good, @diegoreymendez, just a few small comments in the code.

Also, if we're improving WPAnimatedBox, could we also remove the slightly odd newAnimatedBox() method? I'd suggest remove that, and replace it with an initializer:

- (instancetype)init
{
    if (self = [super init]) {
        [self setupView];
    }

    return self;
}

and then any calls to WPAnimatedBox.newAnimatedBox() can just become WPAnimatedBox() :)

[diegoreymendez]

@frosty - Addressed all feedback. Good review.

Ready for another round.

[frosty]

Yay for properties!

[frosty]

Thanks for making the changes! My only remaining query (I feel like I'm nitpicking here!) is that the name of isPreparedToAnimate confuses me. It seems as though its purpose should be to stop us running the prepareAnimation animation if we're already in the 'prepared' state (with the pages transformed down).

But at the beginning of prepareAnimation: we check:

if (!self.isPreparedToAnimate) {
   return;
}

I read this as "if we're not already prepared to animate, we don't execute the method named prepareAnimation". The logic feels backwards to me. I think it makes more sense to invert it like this: https://gist.github.com/frosty/4f96cbf20d8094bde1640bd44a6cf97b – but I might be understanding the purpose of that flag! Let me know if I am! Thanks.

[diegoreymendez]

@frosty - Agreed, I think this class needs more clarity, so read below...

Made a few changes to improve the interface for this class.

1. Removed all trace of the word "preparing" from the public interface: the rationale being that the caller doesn't care if there's preparation involved in the animation process.
2. Due to point 1... there's no longer need for the animation code to check if the caller has "prepared" the animation. No public interface allows the animation to run "unprepared". So I removed isPreparedToAnimate. See here.
3. Renamed method prepareAnimation to moveAnimationToFirstFrame because the term is more specific and conveys its own meaning more clearly.
4. No longer has the option of animating prepareAnimation / moveAnimationToFirstFrame. This wasn't being used at all in the app. If this ever becomes necessary, we can add it back easily. See here.
5. Calling the animation methods when the view is already animating does nothing. See here and here.

[diegoreymendez]

And... changed delayInSeconds to be of type NSTimeInterval (which is equivalent to double as of now) since it seems more descriptive for the purpose of our code than a plain CGFloat.

[diegoreymendez]

Ping @frosty! :)

[frosty]

Sorry for dropping this, @diegoreymendez! Your changes look great – much improved :)

However, when I was testing on my device, I wasn't seeing the animation happen at all. I've done some digging and it looks like it's related to moveAnimationToFirstFrame. If I remove the call to moveAnimationToFirstFrame at the end of setupView, then the animation works the first time you view the post list, but not when you toggle the tabs and view it again. I'm not entirely sure what the underlying cause is (something to do with that dispatch_after and the transforms happening on different runloops?), but this fixes it for me:

- (void)moveAnimationToFirstFrame
{
    // Transform pages all the way down
    NSArray *pages = @[_page1, _page2, _page3];
    for (UIView *view in pages) {
        if (CGAffineTransformEqualToTransform(view.transform, CGAffineTransformIdentity)) {
            CGFloat YOrigin = CGRectGetMinY(view.frame);
            view.transform = CGAffineTransformMakeTranslation(0, CGRectGetHeight(self.frame) - YOrigin);
        }
    }
}

(Check whether the page needs transforming before applying the transformation).

Let me know what you think!

[diegoreymendez]

This PR is ready for another look @frosty. Very nice finds in your review. I used your code suggestion as a starting point, but instead of checking if the transform was identity, I'm now enforcing it before the code that follows up.

[frosty]

Good solution @diegoreymendez! This fixes the problem for me :)

!

[diegoreymendez]

Thanks @frosty!