Skip to content
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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+4 −2
Open

Fix transition speed on swipe back #28302

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core/src/components/router-outlet/router-outlet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Copy link
Contributor

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

Copy link
Contributor Author

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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

Copy link
Contributor

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

Copy link
Contributor Author

@aeharding aeharding Dec 1, 2023 •
edited
Loading

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 :)

Copy link
Contributor

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?

Copy link
Contributor Author

@aeharding aeharding Dec 1, 2023 •
edited
Loading

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.


newStepValue += step;
Comment on lines +114 to +116
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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

}

this.ani.progressEnd(shouldComplete ? 1 : 0, newStepValue, dur);
Expand Down
2 changes: 1 addition & 1 deletion core/src/utils/gesture/swipe-back.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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)

}

onEndHandler(shouldComplete, stepValue <= 0 ? 0.01 : clamp(0, stepValue, 0.9999), realDur);
Expand Down