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: native transition for native-like animations #2413

Closed
wants to merge 4 commits into from
Closed

fix: native transition for native-like animations #2413

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f1a5a4b
fix: native transition for native-like animations (simple_push, slide…
kirillzyusko Oct 16, 2024
ff9d64c
fix: formatter, no breaking changes for `fade_from_bottom` animation …
kirillzyusko Oct 16, 2024
5b8228c
Merge branch 'main' into fix/simple-push-slide-fade-non-native-transi…
kkafar Oct 28, 2024
39b599f
Playaround with implementation of simple push
kkafar Oct 29, 2024
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
2 changes: 1 addition & 1 deletion android/src/paper/java/com/facebook/react/viewmanagers/RNSScreenManagerDelegate.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ public void setProperty(T view, String propName, @Nullable Object value) {
mViewManager.setStackAnimation(view, (String) value);
break;
case "transitionDuration":
mViewManager.setTransitionDuration(view, value == null ? 350 : ((Double) value).intValue());
mViewManager.setTransitionDuration(view, value == null ? 500 : ((Double) value).intValue());
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, why we want to have 500ms there? Is it taken from your own observations, or is it supported by iOS documentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tboba I would say both 😅 I measured duration of animation in QuickTime player and it was showing ~480ms (which is very close to 500ms and 20ms could be due to incorrect cut out of a video).
And second observation is that 7 << 16 animation takes 500ms to complete 🙃

break;
case "replaceAnimation":
mViewManager.setReplaceAnimation(view, (String) value);
Expand Down
2 changes: 1 addition & 1 deletion guides/GUIDE_FOR_LIBRARY_AUTHORS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ When using `vertical` option, options `fullScreenSwipeEnabled: true`, `customAni

### `transitionDuration` (iOS only)

Changes the duration (in milliseconds) of `slide_from_bottom`, `fade_from_bottom`, `fade` and `simple_push` transitions on iOS. Defaults to `350`.
Changes the duration (in milliseconds) of `slide_from_bottom`, `fade_from_bottom`, `fade`, `slide_from_left` and `simple_push` transitions on iOS. Defaults to `500`.

The duration of `default` and `flip` transitions isn't customizable.

Expand Down
95 changes: 65 additions & 30 deletions ios/RNSScreenStackAnimator.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@

// proportions to default transition duration
static const float RNSSlideOpenTransitionDurationProportion = 1;
static const float RNSFadeOpenTransitionDurationProportion = 0.2 / 0.35;
static const float RNSSlideCloseTransitionDurationProportion = 0.25 / 0.35;
static const float RNSFadeCloseTransitionDurationProportion = 0.15 / 0.35;
static const float RNSFadeCloseDelayTransitionDurationProportion = 0.1 / 0.35;
static const float RNSFadeOpenTransitionDurationProportion = 0.2 / 0.5;
static const float RNSSlideCloseTransitionDurationProportion = 0.25 / 0.5;
static const float RNSFadeCloseTransitionDurationProportion = 0.15 / 0.5;
static const float RNSFadeCloseDelayTransitionDurationProportion = 0.1 / 0.5;
Comment on lines +8 to +11
Copy link
Member

Choose a reason for hiding this comment

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

Where these values come from? Do you have some kind of reference here, or are these found empirically?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you talking about 0.5? I captured a transition and measured the length of the video 🙈

I think there is no any notes about that in official documentation, but it's 500ms and many iOS developers aware about that 🙃

// same value is used in other projects using similar approach for transistions
// and it looks the most similar to the value used by Apple
static constexpr float RNSShadowViewMaxAlpha = 0.1;
static const int UIViewAnimationOptionCurveDefaultTransition = 7 << 16;
Copy link
Member

Choose a reason for hiding this comment

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

Should we add a comment why is it 7 << 16?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The first time when I found it was in https://stackoverflow.com/questions/18870447/how-to-use-the-default-ios7-uianimation-curve
Should I add this as a comment above?

Copy link
Member

Choose a reason for hiding this comment

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

I was looking & playing around with this PR yesterday and came to conclusion that 7<< 16 value does not exist as valid animation options. The option parameter takes values of type UIViewAnimationOptions (mask combined from the options) & if you take a look on how it is defined:

typedef NS_OPTIONS(NSUInteger, UIViewAnimationOptions) {
    UIViewAnimationOptionLayoutSubviews            = 1 <<  0,
    UIViewAnimationOptionAllowUserInteraction      = 1 <<  1, // turn on user interaction while animating
    UIViewAnimationOptionBeginFromCurrentState     = 1 <<  2, // start all views from current value, not initial value
    UIViewAnimationOptionRepeat                    = 1 <<  3, // repeat animation indefinitely
    UIViewAnimationOptionAutoreverse               = 1 <<  4, // if repeat, run animation back and forth
    UIViewAnimationOptionOverrideInheritedDuration = 1 <<  5, // ignore nested duration
    UIViewAnimationOptionOverrideInheritedCurve    = 1 <<  6, // ignore nested curve
    UIViewAnimationOptionAllowAnimatedContent      = 1 <<  7, // animate contents (applies to transitions only)
    UIViewAnimationOptionShowHideTransitionViews   = 1 <<  8, // flip to/from hidden state instead of adding/removing
    UIViewAnimationOptionOverrideInheritedOptions  = 1 <<  9, // do not inherit any options or animation type
    
    UIViewAnimationOptionCurveEaseInOut            = 0 << 16, // default
    UIViewAnimationOptionCurveEaseIn               = 1 << 16,
    UIViewAnimationOptionCurveEaseOut              = 2 << 16,
    UIViewAnimationOptionCurveLinear               = 3 << 16,
    
    UIViewAnimationOptionTransitionNone            = 0 << 20, // default
    UIViewAnimationOptionTransitionFlipFromLeft    = 1 << 20,
    UIViewAnimationOptionTransitionFlipFromRight   = 2 << 20,
    UIViewAnimationOptionTransitionCurlUp          = 3 << 20,
    UIViewAnimationOptionTransitionCurlDown        = 4 << 20,
    UIViewAnimationOptionTransitionCrossDissolve   = 5 << 20,
    UIViewAnimationOptionTransitionFlipFromTop     = 6 << 20,
    UIViewAnimationOptionTransitionFlipFromBottom  = 7 << 20,

    UIViewAnimationOptionPreferredFramesPerSecondDefault     = 0 << 24,
    UIViewAnimationOptionPreferredFramesPerSecond60          = 3 << 24,
    UIViewAnimationOptionPreferredFramesPerSecond30          = 7 << 24,
    
} API_AVAILABLE(ios(4.0)) API_UNAVAILABLE(watchos);

using 7 << 16 rather does not make any sense (or am I getting this wrong)?

Copy link
Member

Choose a reason for hiding this comment

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

Second thing is that we would rather construct our mask explicitly from available value in lieu of using a magic constant.

// static constexpr UIViewAnimationOptions RNSUIViewAnimationOptionsCommon = UIViewAnimationOptionCurveEaseInOut;
static constexpr UIViewAnimationOptions RNSUIViewAnimationOptionsCommon = 7 << 16;
// static constexpr UIViewAnimationOptions RNSUIViewAnimationOptionsCommon = UIViewAnimationOptionCurveLinear;

@implementation RNSScreenStackAnimator {
UINavigationControllerOperation _operation;
Expand All @@ -22,7 +26,7 @@ - (instancetype)initWithOperation:(UINavigationControllerOperation)operation
{
if (self = [super init]) {
_operation = operation;
_transitionDuration = 0.35; // default duration in seconds
_transitionDuration = 0.5; // default duration in seconds
Copy link
Member

Choose a reason for hiding this comment

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

Also does it not differ from one iOS version to another? Value of 0.35s is around for longer that I'm in this project 😅 but I'm almost sure that when it was introduced it was meant to resemble native animation as close as possible, hence my impression that this differs between various iOS versions. It would be nice to have some kind of reference, because changing it here to e.g. 0.5s might look "more natively" on iOS 18 but "less natively" on iOS 16 - this is my main objection right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kkafar the curve 7 << 16 was introduced in iOS 7 (when Spring animations were added) and I believe since then that transition wasn't changed. I tested this PR on iOS 15 as well and it looks/feels like a native transition there as well (I was comparing with Settings app).

}
return self;
}
Expand Down Expand Up @@ -128,22 +132,33 @@ - (void)animateSimplePushWithShadowEnabled:(BOOL)shadowEnabled
shadowView.alpha = 0.0;
}

[UIView animateWithDuration:[self transitionDuration:transitionContext]
animations:^{
fromViewController.view.transform = leftTransform;
toViewController.view.transform = CGAffineTransformIdentity;
if (shadowView) {
shadowView.alpha = RNSShadowViewMaxAlpha;
}
}
completion:^(BOOL finished) {
if (shadowView) {
[shadowView removeFromSuperview];
}
fromViewController.view.transform = CGAffineTransformIdentity;
toViewController.view.transform = CGAffineTransformIdentity;
[transitionContext completeTransition:![transitionContext transitionWasCancelled]];
}];
// Default curve provider is as defined below, however defined this way it ignores the `animationDuration` prop.
// Damping of 1.0 seems close enough.
// id<UITimingCurveProvider> timingCurveProvider = [[UISpringTimingParameters alloc] init];

id<UITimingCurveProvider> timingCurveProvider = [[UISpringTimingParameters alloc] initWithDampingRatio:1.0];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think such config is definitely better, but animation begins very slowly (unlike native iOS animation). Compare it yourself (I took 3rd frame for a reference):

image

I think next config gives a better results:

id<UITimingCurveProvider> timingCurveProvider = [[UISpringTimingParameters alloc] initWithMass:3 stiffness:1000 damping:500 initialVelocity:CGVectorMake(0, 0)];
image

Let me see if such config supports custom duration 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, even though a new config is matching a native animation more closely - it doesn't allow to customize duration in this case 😔

Copy link
Member

Choose a reason for hiding this comment

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

Most likely it does not, but let me know.

The spring curve for default animation is just [[UISpringTimingParameters alloc] init] (according to "iOS Programming Guide" by Matt Neuburg, there is no need to guess the params or use some magic constants 😅 )

Copy link
Member

Choose a reason for hiding this comment

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

I've opened #2477, as making this work requires more changes that I've thought initially. I'm in process of writing PR description but have some meetings currently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kkafar but if we are using damping: 1, then we have following transition:

Variant 1 frame 2 frame 3 frame 4 frame 5 frame
damping: 1 image image image image image
Native iOS image image image image image

For me it looks like a native iOS animation (default) is smoother than simple_push (with damping: 1 it feels like it's moving slightly faster than I expect to).


UIViewPropertyAnimator *animator =
[[UIViewPropertyAnimator alloc] initWithDuration:[self transitionDuration:transitionContext]
timingParameters:timingCurveProvider];

[animator addAnimations:^{
fromViewController.view.transform = leftTransform;
toViewController.view.transform = CGAffineTransformIdentity;
if (shadowView) {
shadowView.alpha = RNSShadowViewMaxAlpha;
}
}];

[animator addCompletion:^(UIViewAnimatingPosition finalPosition) {
if (shadowView) {
[shadowView removeFromSuperview];
}
fromViewController.view.transform = CGAffineTransformIdentity;
toViewController.view.transform = CGAffineTransformIdentity;
[transitionContext completeTransition:![transitionContext transitionWasCancelled]];
}];
[animator startAnimation];
} else if (_operation == UINavigationControllerOperationPop) {
toViewController.view.transform = leftTransform;
[[transitionContext containerView] insertSubview:toViewController.view belowSubview:fromViewController.view];
Expand All @@ -159,7 +174,7 @@ - (void)animateSimplePushWithShadowEnabled:(BOOL)shadowEnabled
shadowView.alpha = 0.0;
}
};
void (^completionBlock)(BOOL) = ^(BOOL finished) {
void (^completionBlock)(UIViewAnimatingPosition) = ^(UIViewAnimatingPosition finalPosition) {
if (shadowView) {
[shadowView removeFromSuperview];
}
Expand All @@ -169,16 +184,24 @@ - (void)animateSimplePushWithShadowEnabled:(BOOL)shadowEnabled
};

if (!transitionContext.isInteractive) {
[UIView animateWithDuration:[self transitionDuration:transitionContext]
animations:animationBlock
completion:completionBlock];
id<UITimingCurveProvider> timingCurveProvider = [[UISpringTimingParameters alloc] initWithDampingRatio:1.0];

UIViewPropertyAnimator *animator =
[[UIViewPropertyAnimator alloc] initWithDuration:[self transitionDuration:transitionContext]
timingParameters:timingCurveProvider];

[animator addAnimations:animationBlock];
[animator addCompletion:completionBlock];
[animator startAnimation];
} else {
// we don't want the EaseInOut option when swiping to dismiss the view, it is the same in default animation option
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0.0
options:UIViewAnimationOptionCurveLinear
animations:animationBlock
completion:completionBlock];
UIViewPropertyAnimator *animator =
[[UIViewPropertyAnimator alloc] initWithDuration:[self transitionDuration:transitionContext]
curve:UIViewAnimationCurveLinear
animations:animationBlock];

[animator addCompletion:completionBlock];
[animator startAnimation];
}
}
}
Expand All @@ -203,6 +226,8 @@ - (void)animateSlideFromLeftWithTransitionContext:(id<UIViewControllerContextTra
toViewController.view.transform = rightTransform;
[[transitionContext containerView] addSubview:toViewController.view];
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:RNSUIViewAnimationOptionsCommon
animations:^{
fromViewController.view.transform = leftTransform;
toViewController.view.transform = CGAffineTransformIdentity;
Expand All @@ -228,6 +253,8 @@ - (void)animateSlideFromLeftWithTransitionContext:(id<UIViewControllerContextTra

if (!transitionContext.isInteractive) {
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:RNSUIViewAnimationOptionsCommon
animations:animationBlock
completion:completionBlock];
} else {
Expand All @@ -251,6 +278,8 @@ - (void)animateFadeWithTransitionContext:(id<UIViewControllerContextTransitionin
[[transitionContext containerView] addSubview:toViewController.view];
toViewController.view.alpha = 0.0;
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:RNSUIViewAnimationOptionsCommon
animations:^{
toViewController.view.alpha = 1.0;
}
Expand All @@ -262,6 +291,8 @@ - (void)animateFadeWithTransitionContext:(id<UIViewControllerContextTransitionin
[[transitionContext containerView] insertSubview:toViewController.view belowSubview:fromViewController.view];

[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:RNSUIViewAnimationOptionsCommon
animations:^{
fromViewController.view.alpha = 0.0;
}
Expand All @@ -284,6 +315,8 @@ - (void)animateSlideFromBottomWithTransitionContext:(id<UIViewControllerContextT
toViewController.view.transform = topBottomTransform;
[[transitionContext containerView] addSubview:toViewController.view];
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:RNSUIViewAnimationOptionsCommon
animations:^{
fromViewController.view.transform = CGAffineTransformIdentity;
toViewController.view.transform = CGAffineTransformIdentity;
Expand All @@ -309,6 +342,8 @@ - (void)animateSlideFromBottomWithTransitionContext:(id<UIViewControllerContextT

if (!transitionContext.isInteractive) {
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:RNSUIViewAnimationOptionsCommon
animations:animationBlock
completion:completionBlock];
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/fabric/ModalScreenNativeComponent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export interface NativeProps extends ViewProps {
gestureResponseDistance?: GestureResponseDistanceType;
stackPresentation?: WithDefault<StackPresentation, 'push'>;
stackAnimation?: WithDefault<StackAnimation, 'default'>;
transitionDuration?: WithDefault<Int32, 350>;
transitionDuration?: WithDefault<Int32, 500>;
replaceAnimation?: WithDefault<ReplaceAnimation, 'pop'>;
swipeDirection?: WithDefault<SwipeDirection, 'horizontal'>;
hideKeyboardOnSwipe?: boolean;
Expand Down
2 changes: 1 addition & 1 deletion src/fabric/ScreenNativeComponent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export interface NativeProps extends ViewProps {
gestureResponseDistance?: GestureResponseDistanceType;
stackPresentation?: WithDefault<StackPresentation, 'push'>;
stackAnimation?: WithDefault<StackAnimation, 'default'>;
transitionDuration?: WithDefault<Int32, 350>;
transitionDuration?: WithDefault<Int32, 500>;
replaceAnimation?: WithDefault<ReplaceAnimation, 'pop'>;
swipeDirection?: WithDefault<SwipeDirection, 'horizontal'>;
hideKeyboardOnSwipe?: boolean;
Expand Down
Loading