Is there any benefit to using a final animation rather than allowing users to swap out animations? I feel that could lead to some confusion.

@davidhicks980 good question! Since I personally have never wanted to swap out a transition widget's animation, I made it final here to simplify the implementation.

But I agree that there's a potential for confusion, and with that in mind it might be best to keep it flexible.

@nate-thegrate that makes sense! I was trying to figure out if there was a performance benefit, but code simplicity is not insignificant.

Ooh, speaking of code simplicity—since a bunch of transitions are being defined in the same library, a private mixin could work pretty well.

class _RenderAnimatedAlign extends RenderPositionedBox with _RenderTransition {
  // ...
}

Might end up being even more concise than the code with a final animation!

Good idea. I almost wonder whether a generic is even needed since the mixin isn't using the value and the _listenable is abstract. In code:

mixin _RenderTransition on RenderBox {
  Listenable get listenable => _listenable;
  abstract Listenable _listenable;
  set listenable(Listenable value) {
    if (value == _listenable) {
      return;
    }
    _listenable.removeListener(onListenableUpdate);
    _listenable = value;
    if (attached) {
      value.addListener(onListenableUpdate..call());
    }
  }

  // I changed the name of this method to be a little more specific -- not sure if that's a good idea.
  void onListenableUpdate();

  @override
  void attach(PipelineOwner owner) {
    super.attach(owner);
    _listenable.addListener(onListenableUpdate);
  }

  @override
  void detach() {
    _listenable.removeListener(onListenableUpdate);
    super.detach();
  }
}

Though there could be some benefits to typing the mixin. Either way, good idea!

Thanks!

Yeah, maybe the method should be renamed (though probably to handle___ instead of on___, per the style guide). I don't have a strong preference either way.

I tried removing the type argument, and it almost worked:

…and then just for kicks, I decided to see if covariant would fix it:

class _RenderAnimatedAlign extends RenderPositionedBox with _RenderTransition {
  // ...

  @override
  covariant ValueListenable<AlignmentGeometry> _listenable;
}

and it worked!

I'm slightly leaning toward keeping the type argument: if at some point we make RenderAnimatedAlign a public class, it'd be nice to have that type information on the getter & setter.

But I've never used that keyword on a member variable before; learned something new today!

I guess the good news is: we won't be needing to go through those Google Testing failures!