Browse Source

Fix button with react component doesn't get unmounted (#5457)

* Fix button with react component doesn't get unmounted

* Test e2e only on iOS
Yogev Ben David 4 years ago
parent
commit
a8175ef272
No account linked to committer's email address

+ 6
- 0
e2e/Options.test.js View File

@@ -88,6 +88,12 @@ describe('Options', () => {
88 88
     await expect(elementByLabel('Styling Options')).toBeVisible();
89 89
   });
90 90
 
91
+  it(':ios: Reseting buttons should unmount button react view', async () => {
92
+    await elementById(TestIDs.SHOW_LIFECYCLE_BTN).tap();
93
+    await elementById(TestIDs.RESET_BUTTONS).tap();
94
+    await expect(elementByLabel('Button component unmounted')).toBeVisible();
95
+  });
96
+
91 97
   xit('hides topBar onScroll down and shows it on scroll up', async () => {
92 98
     await elementById(TestIDs.PUSH_OPTIONS_BUTTON).tap();
93 99
     await elementById(TestIDs.SCROLLVIEW_SCREEN_BUTTON).tap();

+ 1
- 1
lib/ios/RNNNavigationButtons.m View File

@@ -103,7 +103,7 @@
103 103
 		componentOptions.name = [[Text alloc] initWithValue:component[@"name"]];
104 104
 		
105 105
 		RNNReactView *view = [_componentRegistry createComponentIfNotExists:componentOptions parentComponentId:self.viewController.layoutInfo.componentId reactViewReadyBlock:nil];
106
-		barButtonItem = [[RNNUIBarButtonItem alloc] init:buttonId withCustomView:view];
106
+		barButtonItem = [[RNNUIBarButtonItem alloc] init:buttonId withCustomView:view componentRegistry:_componentRegistry];
107 107
 	} else if (iconImage) {
108 108
 		barButtonItem = [[RNNUIBarButtonItem alloc] init:buttonId withIcon:iconImage];
109 109
 	} else if (title) {

+ 2
- 0
lib/ios/RNNReactComponentRegistry.h View File

@@ -11,6 +11,8 @@
11 11
 
12 12
 - (void)removeComponent:(NSString *)componentId;
13 13
 
14
+- (void)removeChildComponent:(NSString *)childId;
15
+
14 16
 - (void)clearComponentsForParentId:(NSString *)parentComponentId;
15 17
 
16 18
 - (void)clear;

+ 10
- 0
lib/ios/RNNReactComponentRegistry.m View File

@@ -48,6 +48,16 @@
48 48
 	}
49 49
 }
50 50
 
51
+- (void)removeChildComponent:(NSString *)childId {
52
+	NSMutableDictionary* parent;
53
+	while ((parent = _componentStore.objectEnumerator.nextObject)) {
54
+		if ([parent objectForKey:childId]) {
55
+			[parent removeObjectForKey:childId];
56
+			return;
57
+		}
58
+	}
59
+}
60
+
51 61
 - (void)clear {
52 62
 	[_componentStore removeAllObjects];
53 63
 }

+ 2
- 0
lib/ios/RNNReactView.h View File

@@ -12,4 +12,6 @@ typedef void (^RNNReactViewReadyCompletionBlock)(void);
12 12
 
13 13
 - (void)setAlignment:(NSString *)alignment inFrame:(CGRect)frame;
14 14
 
15
+- (NSString *)componentId;
16
+
15 17
 @end

+ 5
- 1
lib/ios/RNNReactView.m View File

@@ -22,13 +22,17 @@
22 22
 	
23 23
 	RNNReactView* appearedView = notification.object;
24 24
 	
25
-	 if (_reactViewReadyBlock && [appearedView.appProperties[@"componentId"] isEqual:self.appProperties[@"componentId"]]) {
25
+	 if (_reactViewReadyBlock && [appearedView.appProperties[@"componentId"] isEqual:self.componentId]) {
26 26
 	 	_reactViewReadyBlock();
27 27
 		 _reactViewReadyBlock = nil;
28 28
 		 [[NSNotificationCenter defaultCenter] removeObserver:self];
29 29
 	 }
30 30
 }
31 31
 
32
+- (NSString *)componentId {
33
+	return self.appProperties[@"componentId"];
34
+}
35
+
32 36
 - (void)setRootViewDidChangeIntrinsicSize:(void (^)(CGSize))rootViewDidChangeIntrinsicSize {
33 37
 		_rootViewDidChangeIntrinsicSize = rootViewDidChangeIntrinsicSize;
34 38
 		self.delegate = self;

+ 2
- 1
lib/ios/RNNUIBarButtonItem.h View File

@@ -1,6 +1,7 @@
1 1
 #import <Foundation/Foundation.h>
2 2
 #import <React/RCTRootView.h>
3 3
 #import <React/RCTRootViewDelegate.h>
4
+#import "RNNReactComponentRegistry.h"
4 5
 
5 6
 @interface RNNUIBarButtonItem : UIBarButtonItem <RCTRootViewDelegate>
6 7
 
@@ -8,7 +9,7 @@
8 9
 
9 10
 -(instancetype)init:(NSString*)buttonId withIcon:(UIImage*)iconImage;
10 11
 -(instancetype)init:(NSString*)buttonId withTitle:(NSString*)title;
11
--(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView*)reactView;
12
+-(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView *)reactView componentRegistry:(RNNReactComponentRegistry *)componentRegistry;
12 13
 -(instancetype)init:(NSString*)buttonId withSystemItem:(NSString*)systemItemName;
13 14
 
14 15
 @end

+ 10
- 1
lib/ios/RNNUIBarButtonItem.m View File

@@ -7,6 +7,7 @@
7 7
 
8 8
 @property (nonatomic, strong) NSLayoutConstraint *widthConstraint;
9 9
 @property (nonatomic, strong) NSLayoutConstraint *heightConstraint;
10
+@property (nonatomic, weak) RNNReactComponentRegistry *componentRegistry;
10 11
 
11 12
 @end
12 13
 
@@ -28,9 +29,10 @@
28 29
 	return self;
29 30
 }
30 31
 
31
--(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView *)reactView {
32
+-(instancetype)init:(NSString*)buttonId withCustomView:(RCTRootView *)reactView componentRegistry:(RNNReactComponentRegistry *)componentRegistry {
32 33
 	self = [super initWithCustomView:reactView];
33 34
 	
35
+	self.componentRegistry = componentRegistry;
34 36
 	reactView.sizeFlexibility = RCTRootViewSizeFlexibilityWidthAndHeight;
35 37
 	reactView.delegate = self;
36 38
 	reactView.backgroundColor = [UIColor clearColor];
@@ -76,4 +78,11 @@
76 78
 					  afterDelay:0];
77 79
 }
78 80
 
81
+- (void)dealloc {
82
+	if ([self.customView isKindOfClass:[RNNReactView class]]) {
83
+		RNNReactView* customView = self.customView;
84
+		[self.componentRegistry removeChildComponent:customView.componentId];
85
+	}
86
+}
87
+
79 88
 @end

+ 9
- 0
playground/src/screens/LifecycleButton.js View File

@@ -0,0 +1,9 @@
1
+const RoundedButton = require('./RoundedButton');
2
+
3
+class LifecycleButton extends RoundedButton {
4
+  componentWillUnmount() {
5
+    alert('Button component unmounted');
6
+  }
7
+}
8
+
9
+module.exports = LifecycleButton;

+ 30
- 2
playground/src/screens/OptionsScreen.js View File

@@ -1,5 +1,5 @@
1 1
 const React = require('react');
2
-const { Component } = require('react');
2
+const {Component} = require('react');
3 3
 const Root = require('../components/Root');
4 4
 const Button = require('../components/Button')
5 5
 const Navigation = require('../services/Navigation');
@@ -16,7 +16,9 @@ const {
16 16
   PUSH_BTN,
17 17
   HIDE_TOPBAR_DEFAULT_OPTIONS,
18 18
   SHOW_YELLOW_BOX_BTN,
19
-  SET_REACT_TITLE_VIEW
19
+  SET_REACT_TITLE_VIEW,
20
+  RESET_BUTTONS,
21
+  SHOW_LIFECYCLE_BTN
20 22
 } = require('../testIDs');
21 23
 
22 24
 class Options extends Component {
@@ -69,10 +71,36 @@ class Options extends Component {
69 71
         <Button label='Set React Title View' testID={SET_REACT_TITLE_VIEW} onPress={this.setReactTitleView} />
70 72
         <Button label='Show Yellow Box' testID={SHOW_YELLOW_BOX_BTN} onPress={() => console.warn('Yellow Box')} />
71 73
         <Button label='StatusBar' onPress={this.statusBarScreen} />
74
+        <Button label='Show Lifecycle button' testID={SHOW_LIFECYCLE_BTN} onPress={this.showLifecycleButton} />
75
+        <Button label='Remove all buttons' testID={RESET_BUTTONS} onPress={this.resetButtons} />
72 76
       </Root>
73 77
     );
74 78
   }
75 79
 
80
+  showLifecycleButton = () => Navigation.mergeOptions(this, {
81
+    topBar: {
82
+      rightButtons: [
83
+        {
84
+          id: 'ROUND',
85
+          testID: ROUND_BUTTON,
86
+          component: {
87
+            name: Screens.LifecycleButton,
88
+            passProps: {
89
+              title: 'Two'
90
+            }
91
+          }
92
+        }
93
+      ]
94
+    }
95
+  });
96
+
97
+  resetButtons = () => Navigation.mergeOptions(this, {
98
+    topBar: {
99
+      rightButtons: [],
100
+      leftButtons: []
101
+    }
102
+  });
103
+
76 104
   changeTitle = () => Navigation.mergeOptions(this, {
77 105
     topBar: {
78 106
       title: {

+ 1
- 0
playground/src/screens/Screens.js View File

@@ -26,6 +26,7 @@ module.exports = {
26 26
   Navigation: 'Navigation',
27 27
   NativeScreen: 'RNNCustomComponent',
28 28
   RoundButton: 'CustomRoundedButton',
29
+  LifecycleButton: 'LifecycleButton',
29 30
   ReactTitleView: 'ReactTitleView',
30 31
   EventsScreen: 'EventsScreen',
31 32
   EventsOverlay: 'EventsOverlay',

+ 3
- 0
playground/src/screens/index.js View File

@@ -28,6 +28,9 @@ function registerScreens() {
28 28
   Navigation.registerComponent(Screens.Overlay, () => require('./OverlayScreen'));
29 29
   Navigation.registerComponent(Screens.OverlayAlert, () => require('./OverlayAlert'));
30 30
   Navigation.registerComponent(Screens.Pushed, () => require('./PushedScreen'));
31
+  Navigation.registerComponent(Screens.ScrollViewOverlay, () => require('./ScrollViewOverlay'));
32
+  Navigation.registerComponent(Screens.RoundButton, () => require('./RoundedButton'));
33
+  Navigation.registerComponent(Screens.LifecycleButton, () => require('./LifecycleButton'));
31 34
   Navigation.registerComponent(Screens.ReactTitleView, () => require('./CustomTopBar'));
32 35
   Navigation.registerComponent(Screens.RoundButton, () => require('./RoundedButton'));
33 36
   Navigation.registerComponent(Screens.ScrollViewOverlay, () => require('./ScrollViewOverlay'));

+ 2
- 0
playground/src/testIDs.js View File

@@ -128,6 +128,8 @@ module.exports = {
128 128
   PUSH_BOTTOM_TABS_BUTTON: `PUSH_BOTTOM_TABS_BUTTON`,
129 129
   SET_STACK_ROOT_BUTTON: `SET_STACK_ROOT_BUTTON`,
130 130
   SET_ROOT:'SET_ROOT',
131
+  RESET_BUTTONS: 'RESET_BUTTONS',
132
+  SHOW_LIFECYCLE_BTN: 'SHOW_LIFECYCLE_BTN',
131 133
 
132 134
   // Elements
133 135
   SCROLLVIEW_ELEMENT: `SCROLLVIEW_ELEMENT`,