Browse Source

Add YellowBoxed back to ReactRootViews before unmount

Apparently, UiImplementation doesn't like it when views disappear from hierarchy unexpectedly.
As yellow boxes are added to all ReactRootViews, we remove them from TopBar components.
When these components got unmounted, UiImplementation tried to remove the yellow box and since it
couldn't find it - it crashed.

This commit simply adds the yellow box before views are unmounted.
Guy Carmeli 6 years ago
parent
commit
97b83e8124

+ 8
- 0
e2e/ScreenStyle.test.js View File

@@ -1,4 +1,5 @@
1 1
 const Utils = require('./Utils');
2
+const Android = require('./AndroidUtils');
2 3
 const testIDs = require('../playground/src/testIDs');
3 4
 
4 5
 const { elementById, elementByLabel } = Utils;
@@ -175,4 +176,11 @@ describe('screen style', () => {
175 176
     await expect(elementById(testIDs.TOP_BAR_ELEMENT)).toBeVisible();
176 177
     await expect(elementById(testIDs.TOP_BAR_BUTTON)).toBeVisible();
177 178
   });
179
+
180
+  test(':android: Popping screen with yellow box in button, title and background components should not crash', async () => {
181
+    await elementById(testIDs.PUSH_OPTIONS_BUTTON).tap();
182
+    await elementById(testIDs.SHOW_YELLOW_BOX).tap();
183
+    Android.pressBack();
184
+    await expect(elementByLabel('React Native Navigation!')).toBeVisible();    
185
+  });
178 186
 });

+ 1
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ChildController.java View File

@@ -17,7 +17,7 @@ public abstract class ChildController<T extends ViewGroup> extends ViewControlle
17 17
     }
18 18
 
19 19
     public ChildController(Activity activity, ChildControllersRegistry childRegistry, String id, OptionsPresenter presenter, Options initialOptions) {
20
-        super(activity, id, initialOptions);
20
+        super(activity, id, new NoOpYellowBoxDelegate(), initialOptions);
21 21
         this.presenter = presenter;
22 22
         this.childRegistry = childRegistry;
23 23
     }

+ 10
- 0
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/NoOpYellowBoxDelegate.java View File

@@ -0,0 +1,10 @@
1
+package com.reactnativenavigation.viewcontrollers;
2
+
3
+import android.view.View;
4
+
5
+public class NoOpYellowBoxDelegate extends YellowBoxDelegate {
6
+    @Override
7
+    public void onChildViewAdded(View parent, View child) {
8
+
9
+    }
10
+}

+ 1
- 8
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/TitleBarButtonController.java View File

@@ -9,8 +9,6 @@ import android.support.annotation.RestrictTo;
9 9
 import android.support.v7.widget.ActionMenuView;
10 10
 import android.support.v7.widget.Toolbar;
11 11
 import android.view.MenuItem;
12
-import android.view.View;
13
-import android.view.ViewManager;
14 12
 import android.widget.ImageButton;
15 13
 import android.widget.TextView;
16 14
 
@@ -58,7 +56,7 @@ public class TitleBarButtonController extends ViewController<TitleBarReactButton
58 56
                                     Button button,
59 57
                                     ReactViewCreator viewCreator,
60 58
                                     OnClickListener onClickListener) {
61
-        super(activity, button.id, new Options());
59
+        super(activity, button.id, new YellowBoxDelegate(), new Options());
62 60
         this.navigationIconResolver = navigationIconResolver;
63 61
         this.imageLoader = imageLoader;
64 62
         this.optionsPresenter = optionsPresenter;
@@ -97,11 +95,6 @@ public class TitleBarButtonController extends ViewController<TitleBarReactButton
97 95
         return true;
98 96
     }
99 97
 
100
-    @Override
101
-    protected void onYellowBoxAdded(View yellowBox) {
102
-        ((ViewManager) yellowBox.getParent()).removeView(yellowBox);
103
-    }
104
-
105 98
     public void applyNavigationIcon(Toolbar toolbar) {
106 99
         navigationIconResolver.resolve(button, icon -> {
107 100
             setIconColor(icon);

+ 1
- 8
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/TitleBarReactViewController.java View File

@@ -1,8 +1,6 @@
1 1
 package com.reactnativenavigation.viewcontrollers;
2 2
 
3 3
 import android.app.Activity;
4
-import android.view.View;
5
-import android.view.ViewManager;
6 4
 
7 5
 import com.reactnativenavigation.parse.Component;
8 6
 import com.reactnativenavigation.parse.Options;
@@ -16,7 +14,7 @@ public class TitleBarReactViewController extends ViewController<TitleBarReactVie
16 14
     private Component component;
17 15
 
18 16
     public TitleBarReactViewController(Activity activity, TitleBarReactViewCreator reactViewCreator) {
19
-        super(activity, CompatUtils.generateViewId() + "", new Options());
17
+        super(activity, CompatUtils.generateViewId() + "", new YellowBoxDelegate(), new Options());
20 18
         this.reactViewCreator = reactViewCreator;
21 19
     }
22 20
 
@@ -43,11 +41,6 @@ public class TitleBarReactViewController extends ViewController<TitleBarReactVie
43 41
 
44 42
     }
45 43
 
46
-    @Override
47
-    protected void onYellowBoxAdded(View yellowBox) {
48
-        ((ViewManager) yellowBox.getParent()).removeView(yellowBox);
49
-    }
50
-
51 44
     public void setComponent(Component component) {
52 45
         this.component = component;
53 46
     }

+ 5
- 10
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ViewController.java View File

@@ -50,6 +50,7 @@ public abstract class ViewController<T extends ViewGroup> implements ViewTreeObs
50 50
 
51 51
     private final Activity activity;
52 52
     private final String id;
53
+    private YellowBoxDelegate yellowBoxDelegate;
53 54
     protected T view;
54 55
     @Nullable private ParentController<T> parentController;
55 56
     private boolean isShown;
@@ -57,9 +58,10 @@ public abstract class ViewController<T extends ViewGroup> implements ViewTreeObs
57 58
     private ViewVisibilityListener viewVisibilityListener = new ViewVisibilityListenerAdapter();
58 59
     protected FabOptionsPresenter fabOptionsPresenter;
59 60
 
60
-    public ViewController(Activity activity, String id, Options initialOptions) {
61
+    public ViewController(Activity activity, String id, YellowBoxDelegate yellowBoxDelegate, Options initialOptions) {
61 62
         this.activity = activity;
62 63
         this.id = id;
64
+        this.yellowBoxDelegate = yellowBoxDelegate;
63 65
         fabOptionsPresenter = new FabOptionsPresenter();
64 66
         this.initialOptions = initialOptions;
65 67
         options = initialOptions.copy();
@@ -222,6 +224,7 @@ public abstract class ViewController<T extends ViewGroup> implements ViewTreeObs
222 224
             isShown = false;
223 225
             onViewDisappear();
224 226
         }
227
+        yellowBoxDelegate.destroy();
225 228
         if (view instanceof Destroyable) {
226 229
             ((Destroyable) view).destroy();
227 230
         }
@@ -257,11 +260,7 @@ public abstract class ViewController<T extends ViewGroup> implements ViewTreeObs
257 260
 
258 261
     @Override
259 262
     public void onChildViewAdded(View parent, View child) {
260
-        if (parent instanceof ViewGroup &&
261
-            child instanceof ViewGroup &&
262
-            YellowBoxHelper.isYellowBox((ViewGroup) parent, (ViewGroup) child)) {
263
-            onYellowBoxAdded(child);
264
-        }
263
+        yellowBoxDelegate.onChildViewAdded(parent, child);
265 264
     }
266 265
 
267 266
     @Override
@@ -269,10 +268,6 @@ public abstract class ViewController<T extends ViewGroup> implements ViewTreeObs
269 268
 
270 269
     }
271 270
 
272
-    protected void onYellowBoxAdded(View yellowBox) {
273
-
274
-    }
275
-
276 271
     void runOnPreDraw(Task<T> task) {
277 272
         UiUtils.runOnPreDrawOnce(getView(), () -> task.run(getView()));
278 273
     }

+ 50
- 0
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/YellowBoxDelegate.java View File

@@ -0,0 +1,50 @@
1
+package com.reactnativenavigation.viewcontrollers;
2
+
3
+import android.support.annotation.RestrictTo;
4
+import android.view.View;
5
+import android.view.ViewGroup;
6
+
7
+public class YellowBoxDelegate {
8
+    private ViewGroup parent;
9
+    private View yellowBox;
10
+    private YellowBoxHelper yellowBoxHelper;
11
+    private boolean isDestroyed;
12
+
13
+    public YellowBoxDelegate() {
14
+        this.yellowBoxHelper = new YellowBoxHelper();
15
+    }
16
+
17
+    YellowBoxDelegate(YellowBoxHelper yellowBoxHelper) {
18
+        this.yellowBoxHelper = yellowBoxHelper;
19
+    }
20
+
21
+    @RestrictTo(RestrictTo.Scope.TESTS)
22
+    public ViewGroup getParent() {
23
+        return parent;
24
+    }
25
+
26
+    @RestrictTo(RestrictTo.Scope.TESTS)
27
+    public View getYellowBox() {
28
+        return yellowBox;
29
+    }
30
+
31
+    public void onChildViewAdded(View parent, View child) {
32
+        if (yellowBoxHelper.isYellowBox(parent, child)) {
33
+            onYellowBoxAdded(parent, child);
34
+        }
35
+    }
36
+
37
+    protected void onYellowBoxAdded(View parent, View yellowBox) {
38
+        if (isDestroyed) return;
39
+        this.yellowBox = yellowBox;
40
+        this.parent = (ViewGroup) parent;
41
+        this.parent.removeView(yellowBox);
42
+    }
43
+
44
+    public void destroy() {
45
+        isDestroyed = true;
46
+        if (yellowBox != null) {
47
+            parent.addView(yellowBox);
48
+        }
49
+    }
50
+}

+ 5
- 3
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/YellowBoxHelper.java View File

@@ -10,9 +10,11 @@ import com.reactnativenavigation.utils.ViewUtils;
10 10
 public class YellowBoxHelper {
11 11
     private final static int YELLOW_BOX_COLOR = -218449360;
12 12
 
13
-    public static boolean isYellowBox(ViewGroup parent, ViewGroup child) {
14
-        return parent.getChildCount() > 1 &&
15
-               !ViewUtils.findChildrenByClassRecursive(child, View.class, YellowBackgroundMather()).isEmpty();
13
+    public boolean isYellowBox(View parent, View child) {
14
+        return parent instanceof ViewGroup &&
15
+               child instanceof ViewGroup &&
16
+                ((ViewGroup) parent).getChildCount() > 1 &&
17
+               !ViewUtils.findChildrenByClassRecursive(((ViewGroup) child), View.class, YellowBackgroundMather()).isEmpty();
16 18
     }
17 19
 
18 20
     @NonNull

+ 2
- 1
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/externalcomponent/ExternalComponentViewController.java View File

@@ -6,6 +6,7 @@ import android.support.v4.app.FragmentActivity;
6 6
 import com.facebook.react.ReactInstanceManager;
7 7
 import com.reactnativenavigation.parse.ExternalComponent;
8 8
 import com.reactnativenavigation.parse.Options;
9
+import com.reactnativenavigation.viewcontrollers.NoOpYellowBoxDelegate;
9 10
 import com.reactnativenavigation.viewcontrollers.ViewController;
10 11
 import com.reactnativenavigation.views.ExternalComponentLayout;
11 12
 
@@ -15,7 +16,7 @@ public class ExternalComponentViewController extends ViewController<ExternalComp
15 16
     private ReactInstanceManager reactInstanceManager;
16 17
 
17 18
     public ExternalComponentViewController(Activity activity, String id, ExternalComponent externalComponent, ExternalComponentCreator componentCreator, ReactInstanceManager reactInstanceManager, Options initialOptions) {
18
-        super(activity, id, initialOptions);
19
+        super(activity, id, new NoOpYellowBoxDelegate(), initialOptions);
19 20
         this.externalComponent = externalComponent;
20 21
         this.componentCreator = componentCreator;
21 22
         this.reactInstanceManager = reactInstanceManager;

+ 3
- 9
lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/topbar/TopBarBackgroundViewController.java View File

@@ -1,13 +1,12 @@
1 1
 package com.reactnativenavigation.viewcontrollers.topbar;
2 2
 
3 3
 import android.app.Activity;
4
-import android.view.View;
5
-import android.view.ViewManager;
6 4
 
7 5
 import com.reactnativenavigation.parse.Component;
8 6
 import com.reactnativenavigation.parse.Options;
9 7
 import com.reactnativenavigation.utils.CompatUtils;
10 8
 import com.reactnativenavigation.viewcontrollers.ViewController;
9
+import com.reactnativenavigation.viewcontrollers.YellowBoxDelegate;
11 10
 import com.reactnativenavigation.views.topbar.TopBarBackgroundView;
12 11
 import com.reactnativenavigation.views.topbar.TopBarBackgroundViewCreator;
13 12
 
@@ -17,12 +16,12 @@ public class TopBarBackgroundViewController extends ViewController<TopBarBackgro
17 16
     private Component component;
18 17
 
19 18
     public TopBarBackgroundViewController(Activity activity, TopBarBackgroundViewCreator viewCreator) {
20
-        super(activity, CompatUtils.generateViewId() + "", new Options());
19
+        super(activity, CompatUtils.generateViewId() + "", new YellowBoxDelegate(), new Options());
21 20
         this.viewCreator = viewCreator;
22 21
     }
23 22
 
24 23
     public TopBarBackgroundViewController(TopBarBackgroundViewController controller) {
25
-        super(controller.getActivity(), controller.getId(), controller.options);
24
+        super(controller.getActivity(), controller.getId(), new YellowBoxDelegate(), controller.options);
26 25
         this.viewCreator = controller.viewCreator;
27 26
     }
28 27
 
@@ -43,11 +42,6 @@ public class TopBarBackgroundViewController extends ViewController<TopBarBackgro
43 42
         super.onViewDisappear();
44 43
     }
45 44
 
46
-    @Override
47
-    protected void onYellowBoxAdded(View yellowBox) {
48
-        ((ViewManager) yellowBox.getParent()).removeView(yellowBox);
49
-    }
50
-
51 45
     @Override
52 46
     public void sendOnNavigationButtonPressed(String buttonId) {
53 47
 

+ 26
- 4
lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java View File

@@ -36,14 +36,16 @@ public class ViewControllerTest extends BaseTest {
36 36
     private ViewController uut;
37 37
     private Activity activity;
38 38
     private ChildControllersRegistry childRegistry;
39
+    private YellowBoxDelegate yellowBoxDelegate;
39 40
 
40 41
     @Override
41 42
     public void beforeEach() {
42 43
         super.beforeEach();
44
+        yellowBoxDelegate = Mockito.mock(YellowBoxDelegate.class);
43 45
         activity = newActivity();
44 46
         childRegistry = new ChildControllersRegistry();
45 47
         uut = new SimpleViewController(activity, childRegistry, "uut", new Options());
46
-        uut.setParentController(Mockito.mock(ParentController.class));
48
+        uut.setParentController(mock(ParentController.class));
47 49
     }
48 50
 
49 51
     @Test
@@ -59,7 +61,8 @@ public class ViewControllerTest extends BaseTest {
59 61
     @Test
60 62
     public void canOverrideViewCreation() {
61 63
         final FrameLayout otherView = new FrameLayout(activity);
62
-        ViewController myController = new ViewController(activity, "vc", new Options()) {
64
+        yellowBoxDelegate = spy(new YellowBoxDelegate());
65
+        ViewController myController = new ViewController(activity, "vc", yellowBoxDelegate, new Options()) {
63 66
             @Override
64 67
             protected FrameLayout createView() {
65 68
                 return otherView;
@@ -108,6 +111,25 @@ public class ViewControllerTest extends BaseTest {
108 111
         assertThat(uut.findControllerById("uut")).isEqualTo(uut);
109 112
     }
110 113
 
114
+    @Test
115
+    public void onChildViewAdded_delegatesToYellowBoxDelegate() {
116
+        View child = new View(activity);
117
+        ViewGroup view = new FrameLayout(activity);
118
+        ViewController vc = new ViewController(activity, "", yellowBoxDelegate, new Options()) {
119
+            @Override
120
+            protected ViewGroup createView() {
121
+                return view;
122
+            }
123
+
124
+            @Override
125
+            public void sendOnNavigationButtonPressed(String buttonId) {
126
+
127
+            }
128
+        };
129
+        vc.onChildViewAdded(view, child);
130
+        verify(yellowBoxDelegate).onChildViewAdded(view, child);
131
+    }
132
+
111 133
     @Test
112 134
     public void onAppear_WhenShown() {
113 135
         ViewController spy = spy(uut);
@@ -237,7 +259,7 @@ public class ViewControllerTest extends BaseTest {
237 259
     @Test
238 260
     public void isRendered_delegatesToView() {
239 261
         uut.setWaitForRender(new Bool(true));
240
-        uut.view = Mockito.mock(ViewGroup.class, withSettings().extraInterfaces(Component.class));
262
+        uut.view = mock(ViewGroup.class, withSettings().extraInterfaces(Component.class));
241 263
         uut.isRendered();
242 264
         verify((Component) uut.view).isRendered();
243 265
     }
@@ -245,7 +267,7 @@ public class ViewControllerTest extends BaseTest {
245 267
     @Test
246 268
     public void isRendered_returnsTrueForEveryViewByDefault() {
247 269
         uut.setWaitForRender(new NullBool());
248
-        uut.view = Mockito.mock(ViewGroup.class);
270
+        uut.view = mock(ViewGroup.class);
249 271
         assertThat(uut.isRendered()).isTrue();
250 272
     }
251 273
 }

+ 66
- 0
lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/YellowBoxDelegateTest.java View File

@@ -0,0 +1,66 @@
1
+package com.reactnativenavigation.viewcontrollers;
2
+
3
+import android.app.Activity;
4
+import android.view.View;
5
+import android.view.ViewGroup;
6
+import android.widget.FrameLayout;
7
+
8
+import com.reactnativenavigation.BaseTest;
9
+
10
+import org.junit.Test;
11
+import org.mockito.Mockito;
12
+
13
+import static org.assertj.core.api.Java6Assertions.assertThat;
14
+import static org.mockito.Mockito.verify;
15
+
16
+public class YellowBoxDelegateTest extends BaseTest {
17
+    private YellowBoxDelegate uut;
18
+    private YellowBoxHelper yellowBoxHelper;
19
+    private View yellowBox;
20
+    private ViewGroup parent;
21
+
22
+    @Override
23
+    public void beforeEach() {
24
+        Activity context = newActivity();
25
+        yellowBox = new View(context);
26
+        parent = new FrameLayout(context);
27
+        yellowBoxHelper = Mockito.mock(YellowBoxHelper.class);
28
+        uut = new YellowBoxDelegate(yellowBoxHelper);
29
+        parent.addView(yellowBox);
30
+    }
31
+
32
+    @Test
33
+    public void onYellowBoxAdded_removedFromParent() {
34
+        uut.onYellowBoxAdded(parent, yellowBox);
35
+        assertThat(yellowBox.getParent()).isNull();
36
+    }
37
+
38
+    @Test
39
+    public void onYellowBoxAdded_storesRefToYellowBoxAndParent() {
40
+        uut.onYellowBoxAdded(parent, yellowBox);
41
+        assertThat(uut.getYellowBox()).isEqualTo(yellowBox);
42
+        assertThat(uut.getParent()).isEqualTo(parent);
43
+    }
44
+
45
+    @Test
46
+    public void onReactViewDestroy_yellowBoxIsAddedBackToParent() {
47
+        uut.onYellowBoxAdded(parent, yellowBox);
48
+        uut.destroy();
49
+        assertThat(yellowBox.getParent()).isEqualTo(parent);
50
+    }
51
+
52
+    @Test
53
+    public void onChildViewAdded() {
54
+        uut.onChildViewAdded(parent, yellowBox);
55
+        verify(yellowBoxHelper).isYellowBox(parent, yellowBox);
56
+    }
57
+
58
+    @Test
59
+    public void onYellowBoxAdded_notHandledIfDelegateIsDestroyed() {
60
+        uut.onYellowBoxAdded(parent, yellowBox);
61
+        uut.destroy();
62
+
63
+        uut.onYellowBoxAdded(parent, yellowBox);
64
+        assertThat(yellowBox.getParent()).isEqualTo(parent);
65
+    }
66
+}

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

@@ -137,6 +137,7 @@ class OptionsScreen extends Component {
137 137
         <Button title='Push Default Options Screen' testID={testIDs.PUSH_DEFAULT_OPTIONS_BUTTON} onPress={this.onClickPushDefaultOptionsScreen} />
138 138
         <Button title='Show TopBar react view' testID={testIDs.SHOW_TOPBAR_REACT_VIEW} onPress={this.onShowTopBarReactView} />
139 139
         {Platform.OS === 'android' && <Button title='Push' testID={testIDs.PUSH_BUTTON} onPress={this.onPush} />}
140
+        <Button title='Show Yellow Box' testID={testIDs.SHOW_YELLOW_BOX} onPress={() => console.warn('Yellow Box')} />
140 141
         <Text style={styles.footer}>{`this.props.containerId = ${this.props.containerId}`}</Text>
141 142
       </View>
142 143
     );

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

@@ -7,6 +7,7 @@ module.exports = {
7 7
   PUSH_LIFECYCLE_BUTTON: `PUSH_LIFECYCLE_BUTTON`,
8 8
   PUSH_STATIC_LIFECYCLE_BUTTON: `PUSH_STATIC_LIFECYCLE_BUTTON`,
9 9
   PUSH_BUTTON: `PUSH_BUTTON`,
10
+  SHOW_YELLOW_BOX: `SHOW_YELLOW_BOX`,
10 11
   SIDE_MENU_PUSH_BUTTON: `SIDE_MENU_PUSH_BUTTON`,
11 12
   LEFT_SIDE_MENU_PUSH_BUTTON: `leftSIDE_MENU_PUSH_BUTTON`,
12 13
   RIGHT_SIDE_MENU_PUSH_BUTTON: `rightSIDE_MENU_PUSH_BUTTON`,

+ 1
- 1
scripts/test-e2e.js View File

@@ -21,5 +21,5 @@ function run() {
21 21
     if (!skipBuild) {
22 22
         exec.execSync(`detox build --configuration ${configuration}`);
23 23
     }
24
-    exec.execSync(`detox test --configuration ${configuration} --platform ${platform} ${cleanup} ${headless$} ${!android ? `-w ${workers}` : ``}`); //--loglevel trace
24
+    exec.execSync(`detox test --configuration ${configuration} --platform ${platform} ${cleanup} ${headless$} ${!android ? `-w ${workers}` : ``}`); //-f "ScreenStyle.test.js" --loglevel trace
25 25
 }