[PageTransitionsBuilder] Fix 'ZoomPageTransition' built more than once - #58686

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

The forward transition looks exactly the same as before, but there's still some tricky problems in the reverse one and I cannot figure it out. Please help me to implement this more elegantly if you can.

/cc @shihaohong

Oh there're some comments copied from FadeThroughTransition, I'll reword them in next commit. 😃

I'll try to take a look later today or tomorrow. However, if I don't get the chance to, I might disappear for about a week due to personal reasons. In the event that I don't get to it this week, I'll ask someone else on the team to take a look.

Thank you for putting this together and doing the investigation!

When importing the DualTransitionBuilder, the TransitionBuilder has already defined in src/widgets/framwork.dart, which is different the one in dual_transition_builder.dart: Widget Function(BuildContext context, Animation<double> animation, Widget child). So I changed its name to AnimatedTransitionBuilder.

but there's still some tricky problems in the reverse one and I cannot figure it out.

For now the problems has been solved and this animation is absolutely the same as before! 🎉 I'll add built times test case ASAP.

Test has been added, this PR is ready for review.

The analyzer is complaining about trailing whitespaces, so those'll need to be addressed

The analyzer is complaining about trailing whitespaces, so those'll need to be addressed

This test addressed some import issue that I cannot recognize. 😕

Oops, I misread the error message. It's basically complaining about this line and that it is a recursive dependency

Working on the review, please wait for a few minutes.

Seems all resolved now, is it good to go?

You still need to address @Hixie's comment right? Sorry, I think my wording might have been confusing. I was saying that this change you suggested looks okay:

/// Creates a [_ZoomPageTransition].
///
/// The [animation] and [secondaryAnimation] argument are required and must
/// not be null.

Unless there's something else that needs to be described in the constructor that I might be missing

Oh that is because I dropped one of the commit 68acb4d which is included. 😢

Sorry, I think my wording might have been confusing.

It's my bad, done it.

Tree is green now. 👀 I'm wondering whether the failed test matter which is a timeout error?

@AlexVincent525 It seems one of the test suites timed out. I restarted it and also applied the waiting for tree to green label, which will merge the PR once all checks are green.

Thank you so much for this contribution, it was a lot of work and I'm glad we got through it :)