Updated AnimationOptions to match native implementation

[adeca]

• Added setStackRoot property, since it is referenced in RNNTransitionsOptions and used in RNNCommandsHandler
• Changed setRoot and showModal type to match all the other options (StackAnimationOptions), since the native code uses the same type for all of them (RNNScreenTransition on iOS).

This is an update of PR #4856 which was closed without any discussion.

Below are more details on which parts of the native code are not matching the current types:

Added setStackRoot property

In RNNNavigationOptions.h you can see that the type of the animations property is RNNTransitionsOptions:

@interface RNNNavigationOptions : RNNOptions
...
@property (nonatomic, strong) RNNTransitionsOptions* animations;
...
@end

In RNNTransitionsOptions.h you can see that there is in fact a property for setStackRoot:

@interface RNNTransitionsOptions : RNNOptions

@property (nonatomic, strong) RNNScreenTransition* push;
@property (nonatomic, strong) RNNScreenTransition* pop;
@property (nonatomic, strong) RNNScreenTransition* showModal;
@property (nonatomic, strong) RNNScreenTransition* dismissModal;
@property (nonatomic, strong) RNNScreenTransition* setStackRoot;
@property (nonatomic, strong) RNNScreenTransition* setRoot;

@end

This property is then being used in RNNCommandsHandler.h:

- (void)setStackRoot:(NSString*)componentId children:(NSArray*)children completion:(RNNTransitionCompletionBlock)completion rejection:(RCTPromiseRejectBlock)rejection {
	[self assertReady];
	
 	NSArray<RNNLayoutProtocol> *childViewControllers = [_controllerFactory createChildrenLayout:children];
	for (UIViewController<RNNLayoutProtocol>* viewController in childViewControllers) {
		[viewController renderTreeAndWait:NO perform:nil];
	}
	RNNNavigationOptions* options = [childViewControllers.lastObject getCurrentChild].resolveOptions;
	UIViewController *fromVC = [_store findComponentForId:componentId];
	__weak typeof(RNNEventEmitter*) weakEventEmitter = _eventEmitter;
	[_stackManager setStackChildren:childViewControllers fromViewController:fromVC animated:[options.animations.setStackRoot.enable getWithDefaultValue:YES] completion:^{
		[weakEventEmitter sendOnNavigationCommandCompletion:setStackRoot params:@{@"componentId": componentId}];
		completion();
	} rejection:rejection];
}

Perhaps the native code should check the push property instead of setStackRoot. If that is the case I can make that change, and and remove setStackRoot from the native and typescript code.

Changed setRoot and showModal type to match all the other options

In RNNTransitionsOptions.h you can see that the type for all of the animations is the same: RNNScreenTransition, including showModal, setRoot and dismissModal:

@interface RNNTransitionsOptions : RNNOptions

@property (nonatomic, strong) RNNScreenTransition* push;
@property (nonatomic, strong) RNNScreenTransition* pop;
@property (nonatomic, strong) RNNScreenTransition* showModal;
@property (nonatomic, strong) RNNScreenTransition* dismissModal;
@property (nonatomic, strong) RNNScreenTransition* setStackRoot;
@property (nonatomic, strong) RNNScreenTransition* setRoot;

@end

In RNNScreenTransition you can see the properties for this type, which correspond to the ones in OptionsAnimationSeparate not the ones in OptionsAnimationProperties :

@interface RNNScreenTransition : RNNOptions

@property (nonatomic, strong) RNNTransitionStateHolder* topBar;
@property (nonatomic, strong) RNNTransitionStateHolder* content;
@property (nonatomic, strong) RNNTransitionStateHolder* bottomTabs;

@property (nonatomic, strong) Bool* enable;
@property (nonatomic, strong) Bool* waitForRender;

For example the showModal property is used in RNNRootViewController and RNNNavigationController in the following method:

- (nullable id <UIViewControllerAnimatedTransitioning>)animationControllerForPresentedController:(UIViewController *)presented presentingController:(UIViewController *)presenting sourceController:(UIViewController *)source {
	return [[RNNModalAnimation alloc] initWithScreenTransition:self.resolveOptions.animations.showModal isDismiss:NO];
}

The RNNModalAnimation constructor expects an object of RNNScreenTransition type, and uses the content and topBar properties of this object in its code.

If the native code needs to be changed it's a much larger effort, so the typings are meant to reflect the current status of the native code until it's changed (if that happens).

[adeca]

Hi @guyca, I checked the code again after #4880 was merged and it seems that the changes in this PR are still relevant, since the native implementation and the type definitions are not matching yet.

Are there plans to change the native code to match the type definitions?

[kdawgwilk]

Does this also address the duplicate symbols issue in #4897

[selarom-epilef]

This pull request is necessary for the animation properties to be called a working feature

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[stale]

The issue has been closed for inactivity.