Skip to content

fix(iOS): clean up screen animation mount observers #8355

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

Open
OskarEichler wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(iOS): clean up screen animation mount observers #8355

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
85 changes: 58 additions & 27 deletions ios/ScreenAnimationController.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,40 @@ @implementation ScreenAnimationController {
id<UIViewControllerContextTransitioning> _transitionContext;
SharedElementAnimator *_sharedElementAnimator;
BOOL _animate;
CGFloat _duration;
BOOL _observingMounts;
CGFloat _duration;
}

- (void)startObservingMounts {
if (_observingMounts) {
return;
}
_observingMounts = YES;
#ifdef RCT_NEW_ARCH_ENABLED
if (_host != nil) {
[_host.surfacePresenter addObserver:self];
} else {
[_bridge.uiManager.observerCoordinator addObserver:self];
}
#else
[_bridge.uiManager.observerCoordinator addObserver:self];
#endif
}

- (void)stopObservingMounts {
if (!_observingMounts) {
return;
}
_observingMounts = NO;
#ifdef RCT_NEW_ARCH_ENABLED
if (_host != nil) {
[_host.surfacePresenter removeObserver:self];
} else {
[_bridge.uiManager.observerCoordinator removeObserver:self];
}
#else
[_bridge.uiManager.observerCoordinator removeObserver:self];
#endif
}

- (instancetype)initWithContentTransition:(RNNEnterExitAnimation *)contentTransition
Expand Down Expand Up @@ -53,25 +86,17 @@ - (instancetype)initWithContentTransition:(RNNEnterExitAnimation *)contentTransi
#endif

- (void)animateTransition:(id<UIViewControllerContextTransitioning>)transitionContext {
#ifdef RCT_NEW_ARCH_ENABLED
if (_host != nil) {
[_host.surfacePresenter addObserver:self];
} else {
[_bridge.uiManager.observerCoordinator addObserver:self];
}
#else
[_bridge.uiManager.observerCoordinator addObserver:self];
#endif

_animate = YES;
_transitionContext = transitionContext;
[self prepareTransitionContext:transitionContext];

UIViewController *fromVC =
[transitionContext viewControllerForKey:UITransitionContextFromViewControllerKey];
if (![fromVC.navigationController.childViewControllers containsObject:fromVC]) {
[self performAnimationOnce];
}
_animate = YES;
_transitionContext = transitionContext;
[self prepareTransitionContext:transitionContext];

UIViewController *fromVC =
[transitionContext viewControllerForKey:UITransitionContextFromViewControllerKey];
if (![fromVC.navigationController.childViewControllers containsObject:fromVC]) {
[self performAnimationOnce];
} else {
[self startObservingMounts];
}
}

- (void)prepareTransitionContext:(id<UIViewControllerContextTransitioning>)transitionContext {
Expand Down Expand Up @@ -112,8 +137,9 @@ - (NSArray *)createTransitionsFromVC:(UIViewController *)fromVC
- (void)performAnimationOnce {
if (_animate) {
_animate = NO;
RCTExecuteOnMainQueue(^{
id<UIViewControllerContextTransitioning> transitionContext = self->_transitionContext;
[self stopObservingMounts];
RCTExecuteOnMainQueue(^{
id<UIViewControllerContextTransitioning> transitionContext = self->_transitionContext;
UIViewController *fromVC =
[transitionContext viewControllerForKey:UITransitionContextFromViewControllerKey];
UIViewController *toVC =
Expand All @@ -122,8 +148,8 @@ - (void)performAnimationOnce {
toVC:toVC
containerView:transitionContext.containerView];
[self animateTransitions:transitions andTransitioningContext:transitionContext];
});
}
});
}
}

- (void)animateTransitions:(NSArray<id<DisplayLinkAnimatorDelegate>> *)animators
Expand Down Expand Up @@ -192,9 +218,10 @@ - (void)uiManagerDidPerformMounting:(RCTUIManager *)manager {
}

- (void)animationEnded:(BOOL)transitionCompleted {
UIView *toView = [_transitionContext viewForKey:UITransitionContextToViewKey];
UIView *fromView = [_transitionContext viewForKey:UITransitionContextFromViewKey];
[_sharedElementAnimator animationEnded];
[self stopObservingMounts];
UIView *toView = [_transitionContext viewForKey:UITransitionContextToViewKey];
UIView *fromView = [_transitionContext viewForKey:UITransitionContextFromViewKey];
[_sharedElementAnimator animationEnded];
if (toView) {
toView.layer.transform = CATransform3DIdentity;
toView.alpha = 1.f;
Expand All @@ -206,4 +233,8 @@ - (void)animationEnded:(BOOL)transitionCompleted {
_sharedElementAnimator = nil;
}

- (void)dealloc {
[self stopObservingMounts];
}

@end
79 changes: 79 additions & 0 deletions playground/ios/NavigationTests/ScreenAnimationControllerTest.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
#import "ScreenAnimationController.h"
#import <OCMock/OCMock.h>
#import <XCTest/XCTest.h>

#ifdef RCT_NEW_ARCH_ENABLED

@interface RNNScreenAnimationSurfacePresenterSpy : NSObject

@property(nonatomic, assign) NSUInteger addObserverCount;
@property(nonatomic, assign) NSUInteger removeObserverCount;

@end

@implementation RNNScreenAnimationSurfacePresenterSpy

- (void)addObserver:(id<RCTSurfacePresenterObserver>)observer {
self.addObserverCount++;
}

- (void)removeObserver:(id<RCTSurfacePresenterObserver>)observer {
self.removeObserverCount++;
}

@end

@interface RNNScreenAnimationHostSpy : NSObject

@property(nonatomic, strong) RNNScreenAnimationSurfacePresenterSpy *surfacePresenter;

@end

@implementation RNNScreenAnimationHostSpy

@end

@interface ScreenAnimationControllerTest : XCTestCase

@end

@implementation ScreenAnimationControllerTest

- (void)testAnimationEndRemovesMountObserver {
RNNScreenAnimationSurfacePresenterSpy *surfacePresenter =
[RNNScreenAnimationSurfacePresenterSpy new];
RNNScreenAnimationHostSpy *host = [RNNScreenAnimationHostSpy new];
host.surfacePresenter = surfacePresenter;
ScreenAnimationController *controller =
[[ScreenAnimationController alloc] initWithContentTransition:nil
elementTransitions:nil
sharedElementTransitions:nil
duration:0
host:(RCTHost *)host];

UIViewController *fromViewController = [UIViewController new];
UIViewController *toViewController = [UIViewController new];
UINavigationController *navigationController =
[[UINavigationController alloc] initWithRootViewController:fromViewController];
XCTAssertNotNil(navigationController);

id transitionContext = OCMProtocolMock(@protocol(UIViewControllerContextTransitioning));
OCMStub([transitionContext viewControllerForKey:UITransitionContextFromViewControllerKey])
.andReturn(fromViewController);
OCMStub([transitionContext viewControllerForKey:UITransitionContextToViewControllerKey])
.andReturn(toViewController);
OCMStub([transitionContext viewForKey:UITransitionContextFromViewKey]).andReturn(nil);
OCMStub([transitionContext viewForKey:UITransitionContextToViewKey]).andReturn(nil);

[controller animateTransition:transitionContext];
[controller animateTransition:transitionContext];
XCTAssertEqual(surfacePresenter.addObserverCount, 1);

[controller animationEnded:NO];
[controller animationEnded:NO];
XCTAssertEqual(surfacePresenter.removeObserverCount, 1);
}

@end

#endif
4 changes: 4 additions & 0 deletions playground/ios/playground.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
objects = {

/* Begin PBXBuildFile section */
7A8B9C0D1E2F304152637485 /* ScreenAnimationControllerTest.mm in Sources */ = {isa = PBXBuildFile; fileRef = 7A8B9C0D1E2F304152637486 /* ScreenAnimationControllerTest.mm */; };
13B07FBC1A68108700A75B9A /* AppDelegate.mm in Sources */ = {isa = PBXBuildFile; fileRef = 13B07FB01A68108700A75B9A /* AppDelegate.mm */; };
13B07FBF1A68108700A75B9A /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 13B07FB51A68108700A75B9A /* Images.xcassets */; };
13B07FC11A68108700A75B9A /* main.m in Sources */ = {isa = PBXBuildFile; fileRef = 13B07FB71A68108700A75B9A /* main.m */; };
Expand Down Expand Up @@ -107,6 +108,7 @@
/* End PBXContainerItemProxy section */

/* Begin PBXFileReference section */
7A8B9C0D1E2F304152637486 /* ScreenAnimationControllerTest.mm */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.cpp.objcpp; path = ScreenAnimationControllerTest.mm; sourceTree = "<group>"; };
13B07F961A680F5B00A75B9A /* playground.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = playground.app; sourceTree = BUILT_PRODUCTS_DIR; };
13B07FAF1A68108700A75B9A /* AppDelegate.h */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.h; path = AppDelegate.h; sourceTree = "<group>"; };
13B07FB01A68108700A75B9A /* AppDelegate.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = AppDelegate.mm; sourceTree = "<group>"; };
Expand Down Expand Up @@ -395,6 +397,7 @@
E58D261C238587F4003F36BA /* NavigationTests */ = {
isa = PBXGroup;
children = (
7A8B9C0D1E2F304152637486 /* ScreenAnimationControllerTest.mm */,
E58D26442385888C003F36BA /* Options */,
E58D262E2385888B003F36BA /* utils */,
E58D26422385888C003F36BA /* RNNBasePresenterTest.mm */,
Expand Down Expand Up @@ -959,6 +962,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
7A8B9C0D1E2F304152637485 /* ScreenAnimationControllerTest.mm in Sources */,
5007B4392472D9A70002AA4E /* RNNNativeViewController.mm in Sources */,
5007B4382472D9A20002AA4E /* RNNCustomViewController.mm in Sources */,
E58D265B2385888C003F36BA /* UIViewController+RNNOptionsTest.mm in Sources */,
Expand Down