Repository navigation
Fix curve for popping heroes - #39844
Conversation
fluttergithubbot
added
the
framework
| final CreateRectTween createRectTween; | ||
| final HeroFlightShuttleBuilder shuttleBuilder; | ||
| final bool isUserGestureTransition; | ||
| final bool isDiverted; |
There was a problem hiding this comment.
Is this documented somewhere else? It's not immediately obvious what it means.
There was a problem hiding this comment.
It's documented more prominently here:
flutter/packages/flutter/lib/src/widgets/heroes.dart
Lines 588 to 590 in 556e3d9
dnfield
left a comment
dnfield
left a comment
There was a problem hiding this comment.
LGTM with question - if it's not documented anywhere else maybe we could document it here for now?
shihaohong
left a comment
shihaohong
left a comment
There was a problem hiding this comment.
I just had a few questions, but the rest of the changes look good
| if (toHeroes[tag] != null) { | ||
| final HeroFlightShuttleBuilder fromShuttleBuilder = fromHeroes[tag].widget.flightShuttleBuilder; | ||
| final HeroFlightShuttleBuilder toShuttleBuilder = toHeroes[tag].widget.flightShuttleBuilder; | ||
| final bool isDiverted = _flights[tag] != null; |
There was a problem hiding this comment.
Line 832 has _flights[tag] != null as a condition as well, but since it's in a different if/else block, it means something completely different (isAborted).
I'm not sure if this is a real issue or not, but the first time I looked at it, I was a little confused
There was a problem hiding this comment.
_flights[tag] != null means there's currently a hero in-flight with the given tag. If toHeroes[tag] != null we have a new to destination for that hero, so we divert it. If toHeroes[tag] == null the in-flight hero is no longer contained in the destination route. Therefore, we don't have a to destination for the flying hero anymore and we abort its flight. Hope this makes sense.
| return CurvedAnimation( | ||
| parent: (type == HeroFlightDirection.push) ? toRoute.animation : fromRoute.animation, | ||
| curve: Curves.fastOutSlowIn, | ||
| reverseCurve: isDiverted ? null : Curves.fastOutSlowIn.flipped, |
There was a problem hiding this comment.
Does this mean diverted heroes use Curves.fastOutSlowIn as a reverseCurve? When does this actually happen?
There was a problem hiding this comment.
Yes. A diverted hero is a hero that was in-flight and as it was flying got a new destination assigned to it. This can happen when you push a route and as the route is transitioning in you pop it again. The hero would start flying to the newly pushed route, but then as that one is popped fly back to where it came from. Usually, the pop should use the reverse curve, but in this particular case we do not want to switch curves as that would make the hero jump from the position it had on curve to the one it had on reverseCurve.
Codecov Report
@@ Coverage Diff @@
## master #39844 +/- ##
==========================================
+ Coverage 57.89% 58.96% +1.06%
==========================================
Files 194 192 -2
Lines 18691 18443 -248
==========================================
+ Hits 10822 10875 +53
+ Misses 7869 7568 -301
Continue to review full report at Codecov.
|
Description
When popping a route with a Hero the Hero should animate back to its original place using a curve of
Curves.fastOutSlowIn. Prior to this change, the hero would actually use the flipped curve of that (Curves.fastOutSlowIn.flipped).Related Issues
None
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?