Check to see if previous page is laid out before starting hero flight #169633
Conversation
| if (isUserGestureTransition && flightType == HeroFlightDirection.pop && toRoute.maintainState) { | ||
| // immediately because their page's layout is still valid. Unless due to directly | ||
| // adding routes to the pages stack causing the route to never get laid out. | ||
| final bool needsLayout = toRoute.subtreeContext?.findRenderObject()?.debugNeedsLayout ?? true; |
There was a problem hiding this comment.
@chunhtai do you happen to know if this check would still work in prod apps, or if there's a better way to check for if a route has done it's layout logic yet?
There was a problem hiding this comment.
no the getter is wrapped in an assert and will crash in release mode, and any method with debug keyword should only be called in assert anyway
There was a problem hiding this comment.
As far as I know there is no direct way to know whether a renderobject is layout dirty or not. Can you do something like RenderBox.hasSize?
870be44 to
0f9abb1
Compare
| // completed stage with the initial value at 1.0. | ||
| ? initial.status == AnimationStatus.completed | ||
| // driving the reverse transition, but should have a starting value | ||
| // approaching 1.0. In cases where the toRoute begane offstage, there |
There was a problem hiding this comment.
| // approaching 1.0. In cases where the toRoute begane offstage, there | |
| // approaching 1.0. In cases where the toRoute became offstage, there |
| if (isUserGestureTransition && flightType == HeroFlightDirection.pop && toRoute.maintainState) { | ||
| // immediately because their page's layout is still valid. Unless due to directly | ||
| // adding routes to the pages stack causing the route to never get laid out. | ||
| final bool needsLayout = toRoute.subtreeContext?.findRenderObject()?.debugNeedsLayout ?? true; |
There was a problem hiding this comment.
no the getter is wrapped in an assert and will crash in release mode, and any method with debug keyword should only be called in assert anyway
| if (isUserGestureTransition && flightType == HeroFlightDirection.pop && toRoute.maintainState) { | ||
| // immediately because their page's layout is still valid. Unless due to directly | ||
| // adding routes to the pages stack causing the route to never get laid out. | ||
| final bool needsLayout = toRoute.subtreeContext?.findRenderObject()?.debugNeedsLayout ?? true; |
There was a problem hiding this comment.
As far as I know there is no direct way to know whether a renderobject is layout dirty or not. Can you do something like RenderBox.hasSize?
| // driving the reverse transition, but should have a starting value | ||
| // approaching 1.0. In cases where the toRoute begane offstage, there | ||
| // is a slight delay while the toRoute laysout before the hero's | ||
| // flight begins, so the animation may not begin completed. |
There was a problem hiding this comment.
| // flight begins, so the animation may not begin completed. | |
| // flight begins, so the animation may begin in completed status |
| // flight begins, so the animation may not begin completed. | ||
| ? initial.value == 1.0 | ||
| ? initial.status == AnimationStatus.completed | ||
| : initial.status == AnimationStatus.forward |
There was a problem hiding this comment.
I can't wrap my head around this. Do you know what is the situation that makes this a forward?
There was a problem hiding this comment.
This is for when the transition is being controlled by a user gesture, before a formal pop has taken place. During that, the gesture controller in a Cupertino route directly updates the animation controller's value without having it go forward or back. So the status returns as forward because the value is between 1.0 and 0.0 and the direction is still forward from either the default value or from when the route transitioned onto the screen.
There was a problem hiding this comment.
This check is really awkward for this scenario.
There was a problem hiding this comment.
ah gotcha, yeah in this case I think status is not important, we should just return true.
| // immediately because their page's layout is still valid. Unless due to directly | ||
| // adding routes to the pages stack causing the route to never get laid out. | ||
| final RenderBox? fromRouteRenderBox = toRoute.subtreeContext?.findRenderObject() as RenderBox?; | ||
| final bool hasLayout = |
There was a problem hiding this comment.
| final bool hasLayout = | |
| final bool hasValidSize = |
3a0fad4 to
dfe7627
Compare
|
Hi @MitchellGoodwin, I tried this PR on master branch. Yes, it helps to prevent app from crashing. |
Would you be able to take a screen recording and file an issue? I'd like to look into that. |
|
I use the exact code sample of this #168267 with DartPad. If using stable channel, the app hangs as described in the issue. And I use main or beta channel (which has this PR). What you can see in the video is that the first time the back gesture (swipe to pop) is triggered after pushing more than 1 page, the "reverse animation" does not happen (you can see it clearly in the BackChevron of Everything is fine for pop page with BackChevron. Screen.Recording.2025-07-10.at.09.52.35.mov |
|
Thank you for the reply, I filed a new issue with your report. #171974 Lets move any discussion over to there. |
Fixes #168267
When going back to a page that never rendered with a hero transition, the app was hanging. This adds a check to see if a page needs to layout before starting the hero flight, and if so, adding a frame delay.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.