material-motion-archive / family-tween-android

Archived February 16, 2017 :: Tween plan types for Android devices
Apache License 2.0
2 stars 0 forks source link

Should we enforce "one tween per property"? #6

Open pingpongboss opened 7 years ago

pingpongboss commented 7 years ago

Currently, SpringTo enforces "one spring per property" which allows the last SpringTo destination to be honored at any time.

Should we do the same for tweens? See discussion in http://codereview.cc/D1712

jverkoey commented 7 years ago

Platform expectation is that it's ok to add multiple animations for a single property and that the "odd" behavior is by design.

I'm open to the idea of making tweens work like SpringTo animations, but we will have to take care to think through the expected behaviors.

jverkoey commented 7 years ago

First scenario that comes to mind is non-temporally-overlapping tweens on the same property. This should probably be allowed.

pingpongboss commented 7 years ago

To further complicate, the Android platform has different expectations. I will probably move from a Property Animation backed performer to a ViewPropertyAnimator backed performer for platform versions that support it. This is recommended for performance reasons.

Property Animation (old api) shares the same expectations as Core Animations. That you self-manage all inflight animations.

ViewPropertyAnimator (new api) gives you the "feature" that existing inflight animations for a given property are cancelled when a new one is initiated.

Given that expectations in the platform may change, I lean towards establishing expectations in our own motion family independent of platform limitations.

pingpongboss commented 7 years ago

I want to give an example of how I normally write animations on android using the Property Animation api:

public class Activity {
  @Nullable private Animator currentAnimator; // non-null when inflight.

  public void onToggle(boolean state) {
    if (currentAnimator != null) { currentAnimator.cancel(); } // cancel inflight animator.

    if (state == true) {
      currentAnimator = createTransitionFromFooToBar();
    } else {
      currentAnimator = createTransitionFromBarToFoo();
    }

    currentAnimator.setListener({
      void onEnd() {
        currentAnimator = null;
      }
    });
  }
}

As you can see, you always have to be quite explicit in cancelling the previous inflight animator. There's quite a bit of boilerplate. One of the most common animation mistakes I've seen (most non-animation-minded developers make this mistake) is that they do not keep track of inflight animators. So a "timing glitch" is often seen in apps where a transition that is quickly interrupted still runs in the background, and sometimes its window is longer than the new transition and you get an animation glitch that's not caught for a long time (because our fingers are slow).

I'm curious if this is something you have to always think about with Core Animation as well. I think if we enforced "one tween per property" (like the new ViewPropertyAnimator api) we can get rid of a lot of this boilerplate and help devs do the right now. Your "non-temporally-overlapping tweens" example is a good counter point.

jverkoey commented 7 years ago

I do agree there's likely value in a more intelligent API. Do you want to draft up a feature spec in the Tween motion family regarding how to cancel previously in-flight tweens?

Some considerations to think about:

  1. What specific problems exist with the current unmanaged model?
  2. Does our proposed solution resolve the existing problems without introducing new ones?
  3. How do we decide when to remove previous animations vs when to keep them? Non-temporally-overlapping tweens is one such scenario. There may be others.
  4. Are there alternative solutions such as additive animations that may be better solutions than simply canceling animations?