Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

If toValue equals the target's current value then completionBlock will fire immediately #414

Open
warpling opened this issue Jul 11, 2018 · 1 comment

Comments

@warpling
Copy link
Contributor

warpling commented Jul 11, 2018

TL;DR: If the toValue of an animation equals the target's current value (even if the fromValue does not) then the completionBlock will be called immediately with a truthy completed value.

Imagine we have a red square we've hidden by setting it's alpha to 0.0.
Now we want to flash it in to alpha 1.0 and slowly fade it out.
We might add a basic animation like this:

let view = UIView(frame: CGRect(x: 0, y: 0, width: 100, height: 100))
view.backgroundColor = .magenta
view.alpha = 0.0
addSubview(view)

let flashAndFadeAnimation = POPBasicAnimation(propertyNamed: kPOPViewAlpha)!
flashAndFadeAnimation.duration = 10.0
flashAndFadeAnimation.timingFunction = CAMediaTimingFunction.cubicOut()
flashAndFadeAnimation.fromValue = 1.0
flashAndFadeAnimation.toValue   = 0.0
flashAndFadeAnimation.completionBlock = { [weak self] (animation, completed) in
   print("completed at: \(Date())")
}

print("animation added at: \(Date())")
view.pop_add(flashAndFadeAnimation, forKey: "flashAndFade")

The result is that the completionBlock will be called immediately despite the animation itself taking 10 seconds and properly animating.

This elusive bug has bitten me for years and I've only now connected the dots. 😆
I'd love to create a PR if anyone with intimate knowledge of the library could help set me off in the right direction to patch it and add the proper tests.

@warpling
Copy link
Contributor Author

warpling commented Jul 11, 2018

Two potential workarounds using the above code as an example:

A (best workaround)

Use animationDidStartBlock to set the target's value to match the fromValue. This will work even if the animation has a delayed beginTime. (If referencing self you may want to consider including [weak self] at the beginning of the block.)

flashAndFadeAnimation.animationDidStartBlock = { (animation) in
    view.alpha = flashAndFadeAnimation.fromValue as! CGFloat
}

B (simplest workaround if you aren't relying on beginTime)

Since POP actually changes the target's properties (unlike CAAnimation), you can set the target's property to the animation's fromValue before the animation is added:

view.alpha = flashAndFadeAnimation.fromValue as! CGFloat
view.pop_add(flashAndFadeAnimation, forKey: "flashAndFade")

C (quick, dirty, and error prone)

Set the toValue to something like 0.00001. But now if you then try to run this flash animation a second time the current value will now be the same as the toValue and the same problem can occur! To avoid this you might be able to set the target's value to 0.0 in the completionBlock.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant