Ensure component view is not created by mergeOptions (wix#6102)

When mergeOptions is called before a view is created or after it's destroyed, don't attempt to create the view. Fixes wix#6097
stachu2k · Apr 8, 2020 · 4663ac2 · 4663ac2
1 parent 75b5526
commit 4663ac2
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 6 deletions.
diff --git a/.../app/src/main/java/com/reactnativenavigation/viewcontrollers/ComponentViewController.java b/.../app/src/main/java/com/reactnativenavigation/viewcontrollers/ComponentViewController.java
@@ -96,7 +96,7 @@ protected ComponentLayout createView() {
     @Override
     public void mergeOptions(Options options) {
         if (options == Options.EMPTY) return;
-        presenter.mergeOptions(getView(), options);
+        if (isViewShown()) presenter.mergeOptions(getView(), options);
         super.mergeOptions(options);
     }
 

diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ViewController.java b/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ViewController.java
@@ -326,8 +326,8 @@ void runOnPreDraw(Func1<T> task) {
 
     public boolean isViewShown() {
         return !isDestroyed &&
-               getView().isShown() &&
                view != null &&
+               view.isShown() &&
                isRendered();
     }
 

diff --git a/.../src/test/java/com/reactnativenavigation/viewcontrollers/ComponentViewControllerTest.java b/.../src/test/java/com/reactnativenavigation/viewcontrollers/ComponentViewControllerTest.java
@@ -21,6 +21,7 @@
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyZeroInteractions;
 import static org.mockito.Mockito.when;
 
 public class ComponentViewControllerTest extends BaseTest {
@@ -40,12 +41,12 @@ public void beforeEach() {
         parent = TestUtils.newStackController(activity).build();
         Presenter presenter = new Presenter(activity, new Options());
         this.presenter = spy(new ComponentPresenter(Options.EMPTY));
-        uut = new ComponentViewController(activity, new ChildControllersRegistry(), "componentId1", "componentName", (activity1, componentId, componentName) -> view, new Options(), presenter, this.presenter) {
+        uut = spy(new ComponentViewController(activity, new ChildControllersRegistry(), "componentId1", "componentName", (activity1, componentId, componentName) -> view, new Options(), presenter, this.presenter) {
             @Override
             public Options resolveCurrentOptions(Options defaultOptions) {
                 return resolvedOptions;
             }
-        };
+        });
         uut.setParentController(parent);
         parent.ensureViewIsCreated();
     }
@@ -117,8 +118,13 @@ public void mergeOptions_emptyOptionsAreIgnored() {
     }
 
     @Test
-    public void mergeOptions_delegatesToPresenter() {
+    public void mergeOptions_delegatesToPresenterIfViewIsNotShown() {
         Options options = new Options();
+        assertThat(uut.isViewShown()).isFalse();
+        uut.mergeOptions(options);
+        verifyZeroInteractions(presenter);
+
+        when(uut.isViewShown()).thenReturn(true);
         uut.mergeOptions(options);
         verify(presenter).mergeOptions(uut.getView(), options);
     }

diff --git a/...droid/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java b/...droid/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java
@@ -1,7 +1,6 @@
 package com.reactnativenavigation.viewcontrollers;
 
 import android.app.Activity;
-import androidx.coordinatorlayout.widget.CoordinatorLayout;
 import android.view.View;
 import android.view.ViewGroup;
 import android.view.ViewParent;
@@ -27,6 +26,8 @@
 
 import java.lang.reflect.Field;
 
+import androidx.coordinatorlayout.widget.CoordinatorLayout;
+
 import static org.assertj.core.api.Java6Assertions.assertThat;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
@@ -183,6 +184,12 @@ public void onAppear_CalledAtMostOnce() {
         verify(spy, times(1)).onViewAppeared();
     }
 
+    @Test
+    public void isViewShown_doesNotCreateView() {
+        assertThat(uut.isViewShown()).isFalse();
+        assertThat(uut.view).isNull();
+    }
+
     @Test
     public void onDisappear_WhenNotShown_AfterOnAppearWasCalled() {
         ViewController spy = spy(uut);