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 2 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 @@ -294,7 +294,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
27 changes: 22 additions & 5 deletions ios/RNSScreenStackAnimator.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@

// 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.


@implementation RNSScreenStackAnimator {
UINavigationControllerOperation _operation;
Expand All @@ -22,7 +23,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 @@ -129,6 +130,8 @@ - (void)animateSimplePushWithShadowEnabled:(BOOL)shadowEnabled
}

[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:UIViewAnimationOptionCurveDefaultTransition
animations:^{
fromViewController.view.transform = leftTransform;
toViewController.view.transform = CGAffineTransformIdentity;
Expand Down Expand Up @@ -170,6 +173,8 @@ - (void)animateSimplePushWithShadowEnabled:(BOOL)shadowEnabled

if (!transitionContext.isInteractive) {
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:UIViewAnimationOptionCurveDefaultTransition
animations:animationBlock
completion:completionBlock];
} else {
Expand Down Expand Up @@ -203,6 +208,8 @@ - (void)animateSlideFromLeftWithTransitionContext:(id<UIViewControllerContextTra
toViewController.view.transform = rightTransform;
[[transitionContext containerView] addSubview:toViewController.view];
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:UIViewAnimationOptionCurveDefaultTransition
animations:^{
fromViewController.view.transform = leftTransform;
toViewController.view.transform = CGAffineTransformIdentity;
Expand All @@ -228,6 +235,8 @@ - (void)animateSlideFromLeftWithTransitionContext:(id<UIViewControllerContextTra

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

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

if (!transitionContext.isInteractive) {
[UIView animateWithDuration:[self transitionDuration:transitionContext]
delay:0
options:UIViewAnimationOptionCurveDefaultTransition
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 @@ -91,7 +91,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