-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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
Fix transition speed on swipe back #28302
base: main
Are you sure you want to change the base?
Conversation
@@ -111,7 +111,9 @@ export class RouterOutlet implements ComponentInterface, NavOutlet { | |||
this.ani.easing('cubic-bezier(1, 0, 0.68, 0.28)'); | |||
newStepValue += getTimeGivenProgression([0, 0], [1, 0], [0.68, 0.28], [1, 1], step)[0]; | |||
} else { | |||
newStepValue += getTimeGivenProgression([0, 0], [0.32, 0.72], [0, 1], [1, 1], step)[0]; | |||
this.ani.easing('linear'); |
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.
Can you clarify why you made this change? Swiping back does use a non-linear easing curve
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.
Sure! So when playing with the iOS Settings app, the transition easing function on swipe back appears to be linear, or at least much, much closer to linear than the open-a-new-page animation (which is more an ease-out function).
In my testing, if you shorten the duration from 540 to 200 without changing the easing function (from ease-out-ish to linear), the animation "appears" too fast relative to iOS Settings. By changing to linear, similar to what Apple uses for swipe back animation, the swipe back animation finishes faster (which fixes the problem in #27748) but critically doesn't "appear" much faster (because of the linear easing function).
This is all me trying to match what Apple is doing for swipe back - which from my trial an error testing is:
- linear
- max of 200ms to complete.
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.
Sorry for the delay! I compared with native and native iOS does appear to still use a non-linear curve. See attached demo:
ios-demo.mov
If the easing curve was linear then I'd expect to see the page move at a constant rate when I scrub through the video. Instead, it starts fast and slows down towards the end.
However, the max duration of 540ms in Ionic is definitely too slow:
native | ionic |
---|---|
RPReplay_Final1701441575.MP4 |
RPReplay_Final1701441584.MP4 |
One thing to note is that iOS likely uses a spring-based easing curve, whereas Ionic uses a cubic bezier easing curve, so the transitions won't match exactly. However, I think we can fix this by choosing a duration between 200ms and 540ms. I'll play around with it a bit and see if I can find a good value.
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.
@liamdebeasi sound good! I agree, on closer inspection swipe back does appear to still be nonlinear. However, the swipe back animation does appear (to me at least) to be much closer to linear than the new page animation (as well as shorter duration).
RPReplay_Final1701441799.online-video-cutter.com.mp4
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.
Looks like keeping the curve + changing the max duration to 300ms (instead of 200ms) achieves a very similar result:
native | ionic |
---|---|
RPReplay_Final1701442027.MP4 |
RPReplay_Final1701442135.MP4 |
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.
@liamdebeasi, can you try comparing a swipe back with the slowest momentum possible to trigger the navigation? That's when I find the timing function differences most apparent. I am happy to take a look this weekend too and whip up some comparison videos - but I figure you already have your environment setup so :)
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.
Hmm yeah it looks way too fast now on Ionic. My thinking on the original fix was likely flipped. The dur
appears to represent the faster duration that is based on how quickly you swipe. If this is too long then we need to change the calculation. The 540 (in main) or 200 (in this branch) value is for the slow swipes.
Which kind of swiping are you running into an issue with?
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.
@liamdebeasi I believe slow swipes is the main problem, because it takes 540ms.
To your point on the slow swipe looking way too fast, that is where the change to the timing function that I originally proposed helps out. By changing the timing function to linear and max time to 200ms, the animation "appears" slower to the eye (and very similar to iOS, in slow swipe situation), but completes faster in the specific duration of time.
@@ -70,7 +70,7 @@ export const createSwipeBackGesture = ( | |||
let realDur = 0; | |||
if (missingDistance > 5) { | |||
const dur = missingDistance / Math.abs(velocity); | |||
realDur = Math.min(dur, 540); | |||
realDur = Math.min(dur, 200); |
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.
realDur = Math.min(dur, 200); | |
/** | |
* Completing a swipe to go back gesture should happen | |
* faster than a normal iOS page transition. As a result, the max duration | |
* is going to be different than in the iOS page transition file. | |
*/ | |
realDur = Math.min(dur, 300); |
Change as per my previous comment (plus some documentation in case team members are wondering why the durations between swipe to go back and the page transition no longer match)
this.ani.easing('linear'); | ||
|
||
newStepValue += step; |
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.ani.easing('linear'); | |
newStepValue += step; | |
newStepValue += getTimeGivenProgression([0, 0], [0.32, 0.72], [0, 1], [1, 1], step)[0]; |
We should be able to keep the cubic curve as per my previous comment
Issue number: #27748
What is the current behavior?
Swipe back can take too long. On the iOS settings app, swipe back is always shorter than tapping to go to a new page (even if you swipe back slowly). This is accomplished without making it visually seem "faster" by using a linear transition function for the back swipe.
A slow swipe back transition can make the app see laggy and slow when swiping back and then scrolling down in the page you swiped back to.
What is the new behavior?
Does this introduce a breaking change?
Other information
This patch has been in use in Voyager for the past couple months with great results.