-
Notifications
You must be signed in to change notification settings - Fork 28.6k
Check to see if previous page is laid out before starting hero flight #169633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check is really awkward for this scenario.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah gotcha, yeah in this case I think status is not important, we should just return true.
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.