Cupertino navigation bars transitionBetweenRoutes fidelity update #164956
Conversation
| ); | ||
|
|
||
| Widget child = bottomNavBarBottom.child; | ||
| final Curve animationCurve = | ||
| animation.status == AnimationStatus.forward | ||
| ? Curves.easeOutQuart |
There was a problem hiding this comment.
The forward case looks a little fast. Ideally, the navbar contents would not go ahead of the content behind it, I think it's a little distracting. The reverse case looks good, in my opinion.
It looks like the Hero widget applies it's own curve as well. Is that interfering at all?
How does this look when dragging the whole time? As long as the drag gesture is still active, the base curve of the transition is linear.
There was a problem hiding this comment.
The forward case looks a little fast. Ideally, the navbar contents would not go ahead of the content behind it, I think it's a little distracting. The reverse case looks good, in my opinion.
yeah it feels too fast and the fact that it goes ahead of the content makes it look bad visually, especially when manually dragging to pop a page.
I also found a bug when swiping between pages, if I start swiping from the furthest point the large title doesn't start animating immediately creating an unpleasing visual bug, but if I start swiping to pop from a point far from the edge it animates as expected, here's a screen recording of this behaviour:
There was a problem hiding this comment.
I also found a bug when swiping between pages, if I start swiping from the furthest point the large title doesn't start animating immediately creating an unpleasing visual bug, but if I start swiping to pop from a point far from the edge it animates as expected, here's a screen recording of this behaviour
That's likely because the page transition is on a linear curve during the drag, while the navbar is still on a flipped easeOutQuart curve.
There was a problem hiding this comment.
Is it possible for the header to simply follow the content's x coordinate? If we can't get the exact curve then simply doing this should be acceptable.
There was a problem hiding this comment.
Is it possible for the header to simply follow the content's x coordinate? If we can't get the exact curve then simply doing this should be acceptable.
I also thought of this, it sounds like a reasonable thing to do if the curve requires lots of work to look somewhat right, also since this is a temporary change and will be tweaked again as mentioned in the issue description to use a spring simulation.
There was a problem hiding this comment.
I tried all the curves we have, none of them could follow the content. There's something funky happening behind the scenes that I can't put a finger on. @MitchellGoodwin mentioned that the Hero might be applying its own curve, maybe that's why.
Either way I used custom curves to approximate the native animation.
There was a problem hiding this comment.
it look more polished now, looks good for when there's only a large title, but with the search field not so much, can you make the search field that's on the page that's getting another page pushed on to (the previous page), slide with the same curve as the pages content because currently there a lot of overlapping happening with them especially when manually dragging, things are feeling better overall, the pushed title felt flimsy before this pr now it feels snappy which is good.
| ); | ||
|
|
||
| Widget child = bottomNavBarBottom.child; | ||
| final Curve animationCurve = |
There was a problem hiding this comment.
the title being ahead of the page looks jarring, maybe a custom animation curve could be used instead of the current built-in ones if they can't match current needs, because it seems like it needs a very specific curve, based on my observation of the native side, maybe you can use the base of the curve that the page content animates in, but tweak it a bit, make it slightly slower at the beginning and faster at the end but not to fast to the point where it doesn't overflow out of the page's content.
| ); | ||
|
|
||
| Widget child = bottomNavBarBottom.child; | ||
| final Curve animationCurve = | ||
| animation.status == AnimationStatus.forward | ||
| ? Curves.easeOutQuart |
There was a problem hiding this comment.
Is it possible for the header to simply follow the content's x coordinate? If we can't get the exact curve then simply doing this should be acceptable.
I also thought of this, it sounds like a reasonable thing to do if the curve requires lots of work to look somewhat right, also since this is a temporary change and will be tweaked again as mentioned in the issue description to use a spring simulation.
There was a problem hiding this comment.
looks awesome now, the curves are snappy like native and aren't overlapping aggressively like before, the bug where if you start swiping to pop from the devices border is still present, the title doesn't start animating until you swipe a longer distance, also the the large title when animating to the previous page title in the back button needs updating now seems it needs to match the speed the large title that's entering and leaving, other than that, everything looks and works fine.
| // The bottom widget animates linearly during a backswipe by a user gesture. | ||
| userGestureInProgress | ||
| // The parent of the hero animation, which is the route animation. | ||
| ? (animation as CurvedAnimation).parent |
There was a problem hiding this comment.
I think we should check to see if it is a CurvedAnimation. That way if it changes in the Hero implementation in the future, this doesn't break.
There was a problem hiding this comment.
should we check or assert?
There was a problem hiding this comment.
Good question. I think assert would be better, so this code doesn't just hang around if that happens.
| // The large title animates linearly during a backswipe by a user gesture. | ||
| userGestureInProgress | ||
| // The parent of the hero animation, which is the route animation. | ||
| ? (animation as CurvedAnimation).parent |
|
just checked the latest changes and it looks good, I also agree with Mitchels comment:
things look good so far, it isn't worth putting more effort to match the native side down to the smallest detail if it's going to be revisited and get implemented with a spring simulation sometime later. |
|
can you make it that the chevron finishes animating as soon as the page push animation finishes because currently it starts animating when the push animation finishes unlike native. |
…utter#164956) In the before Flutter video, drag happens after the 17 second mark. ## Flutter before https://github.com/user-attachments/assets/9fbcd59e-68aa-4975-9a66-f05e72071223 ## Flutter after https://github.com/user-attachments/assets/784d7f46-a8ce-4e5f-a849-3b19df6668c9 ## Flutter back swipe drag gesture https://github.com/user-attachments/assets/09d38c01-aeea-46c1-90b4-590861f8f3a2 ## Native iOS https://github.com/user-attachments/assets/f2ab96c0-766b-4452-b6d5-3f92e6b6785f ## Flutter scaled back chevron and large title after https://github.com/user-attachments/assets/ba87add7-affa-4dcc-b2f0-abbc3487d677 ## Native iOS scaled back chevron and large title https://github.com/user-attachments/assets/5c7bfe5b-5789-4ab9-8e36-770cf802b1b1 Native iOS is probably using a spring simulation. This is the closest curve we have to the native transition, but should be updated in the future with the exact values. Fixes [Cupertino navigation bars transitionBetweenRoutes fidelity update](flutter#164662)
In the before Flutter video, drag happens after the 17 second mark.
Flutter before
Flutter.before.mov
Flutter after
Flutter.after.4.mov
Flutter back swipe drag gesture
flutter.after.drag.2.mov
Native iOS
native.mov
Flutter scaled back chevron and large title after
flutter.scaled.and.label.mov
Native iOS scaled back chevron and large title
native.scaled.back.chevron.mov
Native iOS is probably using a spring simulation. This is the closest curve we have to the native transition, but should be updated in the future with the exact values.
Fixes Cupertino navigation bars transitionBetweenRoutes fidelity update