Skip to content

Conversation

@adeca
Copy link

@adeca adeca commented Mar 12, 2019

OptionsAnimations:

  • Added setStackRoot property
  • Changed setRoot and showModal type to match all the other options (OptionsAnimationSeparate), since the native code uses the same code for all of them.

OptionsAnimationSeparate:

@guyca
Copy link
Collaborator

guyca commented Mar 14, 2019

Hey there

  1. setStackRoot uses the push animation - it doesn't have an animation property
  2. Both setRoot and showModal use a simple animation declaration as they simply animate the entire screen since they add they screen to the hierarchy, unlike push/pop which adds/removes screen from a layout and therefore can manipulate internal views - TopBar and BottomTabs.
  3. The enabled property has just been added by another PR

Closing, but thanks a lot for your contribution 👍

@guyca guyca closed this Mar 14, 2019
@adeca
Copy link
Author

adeca commented Mar 14, 2019

Hi @guyca. the changes I made are based on the native implementation, which does not seem to correspond entirely to what you are saying in points 1 and 2. Perhaps it's the native implementation that is wrong?

Let me show what I based my changes on.

  1. setStackRoot uses the push animation - it doesn't have an animation 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.

  1. Both setRoot and showModal use a simple animation declaration as they simply animate the entire screen since they add they screen to the hierarchy, unlike push/pop which adds/removes screen from a layout and therefore can manipulate internal views - TopBar and BottomTabs.

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

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's needed).

  1. The enabled property has just been added by another PR

Yeah, I mentioned that PR in this PR, but it wasn't merged at the time. I'll update my fork with the latest changes.

@kyle-ssg
Copy link
Contributor

kyle-ssg commented Apr 22, 2019

setStackRoot does not use the push animation as suggested, at least in iOS. Also, if this isn't supported the docs need updating!
https://wix.github.io/react-native-navigation/#/docs/screen-api?id=setstackrootcomponentid-params

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants