From 111df5a3ba51ba6762cffd7119071bb4f71d18f7 Mon Sep 17 00:00:00 2001 From: Guy Carmeli Date: Sun, 5 Apr 2020 16:50:33 +0300 Subject: [PATCH] Ensure component view is not created by mergeOptions (#6102) When mergeOptions is called before a view is created or after it's destroyed, don't attempt to create the view. Fixes #6097 --- .../viewcontrollers/ComponentViewController.java | 2 +- .../viewcontrollers/ViewController.java | 2 +- .../viewcontrollers/ComponentViewControllerTest.java | 12 +++++++++--- .../viewcontrollers/ViewControllerTest.java | 9 ++++++++- 4 files changed, 19 insertions(+), 6 deletions(-) diff --git a/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ComponentViewController.java b/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ComponentViewController.java index 6661bf01748..630304aeac6 100644 --- a/lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/ComponentViewController.java +++ b/lib/android/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 index 015de0355cb..ed70c15bd7e 100644 --- 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 task) { public boolean isViewShown() { return !isDestroyed && - getView().isShown() && view != null && + view.isShown() && isRendered(); } diff --git a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ComponentViewControllerTest.java b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ComponentViewControllerTest.java index f3dfc86da6e..b01c85a6f6e 100644 --- a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ComponentViewControllerTest.java +++ b/lib/android/app/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/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java b/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java index 9e50fe72fa7..12598156707 100644 --- a/lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/ViewControllerTest.java +++ b/lib/android/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);