Remove support from popping specific screens in a stack

While arguable being convenient, this is behaviour goes against the Stack api.

Some commands, such as push and pop, can be applied on stacks by passing the
stack's componentId.
This commit also fixes those stack commands which were always applied on parent stacks.

e2e/ScreenStack.test.js

@@ -23,17 +23,6 @@ describe('screen stack', () => {
     await expect(elementById(testIDs.WELCOME_SCREEN_HEADER)).toBeVisible();
   });
 
-  test('pop screen deep in the stack', async () => {
-    await elementById(testIDs.PUSH_BUTTON).tap();
-    await expect(elementByLabel('Stack Position: 1')).toBeVisible();
-    await elementById(testIDs.PUSH_BUTTON).tap();
-    await expect(elementByLabel('Stack Position: 2')).toBeVisible();
-    await elementById(testIDs.POP_PREVIOUS_BUTTON).tap();
-    await expect(elementByLabel('Stack Position: 2')).toBeVisible();
-    await elementById(testIDs.POP_BUTTON).tap();
-    await expect(elementById(testIDs.WELCOME_SCREEN_HEADER)).toBeVisible();
-  });
-
   test('pop to specific id', async () => {
     await elementById(testIDs.PUSH_BUTTON).tap();
     await elementById(testIDs.PUSH_BUTTON).tap();

lib/android/app/src/main/java/com/reactnativenavigation/react/NavigationModule.java

@@ -98,8 +98,8 @@ public class NavigationModule extends ReactContextBaseJavaModule {
     }
 
 	@ReactMethod
-	public void pop(String commandId, String onComponentId, ReadableMap options, Promise promise) {
-		handle(() -> navigator().popSpecific(onComponentId, new NativeCommandListener(commandId, promise, eventEmitter, now)));
+	public void pop(String commandId, String componentId, ReadableMap options, Promise promise) {
+		handle(() -> navigator().pop(componentId, new NativeCommandListener(commandId, promise, eventEmitter, now)));
 	}
 
 	@ReactMethod

lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/Navigator.java

@@ -15,6 +15,7 @@ import com.reactnativenavigation.presentation.OverlayManager;
 import com.reactnativenavigation.react.EventEmitter;
 import com.reactnativenavigation.utils.CommandListener;
 import com.reactnativenavigation.utils.CompatUtils;
+import com.reactnativenavigation.utils.Task;
 import com.reactnativenavigation.viewcontrollers.modal.ModalStack;
 import com.reactnativenavigation.viewcontrollers.stack.StackController;
 import com.reactnativenavigation.views.element.ElementTransitionManager;
@@ -147,54 +148,28 @@ public class Navigator extends ParentController {
         }
     }
 
-    public void push(final String fromId, final ViewController viewController, CommandListener listener) {
-        ViewController from = findControllerById(fromId);
-        if (from != null) {
-            from.performOnParentStack(
-                    stack -> ((StackController) stack).push(viewController, listener),
-                    () -> rejectPush(fromId, viewController, listener)
-            );
-        } else {
-            rejectPush(fromId, viewController, listener);
-        }
+    public void push(final String id, final ViewController viewController, CommandListener listener) {
+        applyOnStack(id, listener, stack -> stack.push(viewController, listener));
     }
 
-    public void setStackRoot(String fromId, ViewController viewController, CommandListener listener) {
-        ViewController from = findControllerById(fromId);
-        if (from != null) {
-            from.performOnParentStack(stack -> ((StackController) stack).setRoot(viewController, listener));
-        }
-    }
-
-    public void pop(final String fromId, CommandListener listener) {
-        ViewController from = findControllerById(fromId);
-        if (from != null) {
-            from.performOnParentStack(stack -> ((StackController) stack).pop(listener));
-        }
+    public void setStackRoot(String id, ViewController viewController, CommandListener listener) {
+        applyOnStack(id, listener, stack -> stack.setRoot(viewController, listener));
     }
 
-    public void popSpecific(final String id, CommandListener listener) {
-        ViewController from = findControllerById(id);
-        if (from != null) {
-            from.performOnParentStack(stack -> ((StackController) stack).popSpecific(from, listener), () -> listener.onError("Nothing to pop"));
-        } else {
-            listener.onError("Nothing to pop");
-        }
+    public void pop(String id, CommandListener listener) {
+        applyOnStack(id, listener, stack -> stack.pop(listener));
     }
 
     public void popToRoot(final String id, CommandListener listener) {
-        ViewController from = findControllerById(id);
-        if (from != null) {
-            from.performOnParentStack(stack -> ((StackController) stack).popToRoot(listener));
-        }
+        applyOnStack(id, listener, stack -> stack.popToRoot(listener));
     }
 
-    public void popTo(final String componentId, CommandListener listener) {
-        ViewController target = findControllerById(componentId);
+    public void popTo(final String id, CommandListener listener) {
+        ViewController target = findControllerById(id);
         if (target != null) {
-            target.performOnParentStack(stack -> ((StackController) stack).popTo(target, listener), () -> listener.onError("Nothing to pop"));
+            target.performOnParentStack(stack -> ((StackController) stack).popTo(target, listener));
         } else {
-            listener.onError("Nothing to pop");
+            listener.onError("Failed to execute stack command. Stack by " + id + " not found.");
         }
     }
 
@@ -229,12 +204,17 @@ public class Navigator extends ParentController {
         return controllerById != null ? controllerById : modalStack.findControllerById(id);
     }
 
-    private void rejectPush(String fromId, ViewController viewController, CommandListener listener) {
-        listener.onError("Could not push component: " +
-                         viewController.getId() +
-                         ". Stack with id " +
-                         fromId +
-                         " was not found.");
+    private void applyOnStack(String fromId, CommandListener listener, Task<StackController> task) {
+        ViewController from = findControllerById(fromId);
+        if (from != null) {
+            if (from instanceof StackController) {
+                task.run((StackController) from);
+            } else {
+                from.performOnParentStack(stack -> task.run((StackController) stack) );
+            }
+        } else {
+            listener.onError("Failed to execute stack command. Stack " + fromId + " not found.");
+        }
     }
 
     private boolean isRootNotCreated() {

lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackController.java

@@ -19,7 +19,6 @@ import com.reactnativenavigation.utils.CommandListenerAdapter;
 import com.reactnativenavigation.viewcontrollers.ChildControllersRegistry;
 import com.reactnativenavigation.viewcontrollers.IdStack;
 import com.reactnativenavigation.viewcontrollers.ParentController;
-import com.reactnativenavigation.viewcontrollers.ReactViewCreator;
 import com.reactnativenavigation.viewcontrollers.ViewController;
 import com.reactnativenavigation.viewcontrollers.topbar.TopBarBackgroundViewController;
 import com.reactnativenavigation.viewcontrollers.topbar.TopBarController;
@@ -38,16 +37,14 @@ public class StackController extends ParentController<StackLayout> {
 
     private final IdStack<ViewController> stack = new IdStack<>();
     private final NavigationAnimator animator;
-    private final ReactViewCreator topBarButtonCreator;
     private TopBarBackgroundViewController topBarBackgroundViewController;
     private TopBarController topBarController;
     private BackButtonHelper backButtonHelper;
     private final StackOptionsPresenter presenter;
 
-    public StackController(Activity activity, List<ViewController> children, ChildControllersRegistry childRegistry, ReactViewCreator topBarButtonCreator, TopBarBackgroundViewController topBarBackgroundViewController, TopBarController topBarController, NavigationAnimator animator, String id, Options initialOptions, BackButtonHelper backButtonHelper, StackOptionsPresenter stackPresenter, OptionsPresenter presenter) {
+    public StackController(Activity activity, List<ViewController> children, ChildControllersRegistry childRegistry, TopBarBackgroundViewController topBarBackgroundViewController, TopBarController topBarController, NavigationAnimator animator, String id, Options initialOptions, BackButtonHelper backButtonHelper, StackOptionsPresenter stackPresenter, OptionsPresenter presenter) {
         super(activity, childRegistry, id, presenter, initialOptions);
         this.topBarController = topBarController;
-        this.topBarButtonCreator = topBarButtonCreator;
         this.topBarBackgroundViewController = topBarBackgroundViewController;
         this.animator = animator;
         this.backButtonHelper = backButtonHelper;
@@ -224,15 +221,6 @@ public class StackController extends ParentController<StackLayout> {
         listener.onSuccess(disappearing.getId());
     }
 
-    public void popSpecific(ViewController childController, CommandListener listener) {
-        if (stack.isTop(childController.getId())) {
-            pop(listener);
-        } else {
-            removeAndDestroyController(childController);
-            listener.onSuccess(childController.getId());
-        }
-    }
-
     public void popTo(final ViewController viewController, CommandListener listener) {
         if (!stack.containsId(viewController.getId())) {
             listener.onError("Nothing to pop");

lib/android/app/src/main/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerBuilder.java

@@ -95,7 +95,6 @@ public class StackControllerBuilder {
         return new StackController(activity,
                 children,
                 childRegistry,
-                topBarButtonCreator,
                 topBarBackgroundViewController,
                 topBarController,
                 animator,

lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/NavigatorTest.java

@@ -24,6 +24,7 @@ import com.reactnativenavigation.utils.CommandListenerAdapter;
 import com.reactnativenavigation.utils.CompatUtils;
 import com.reactnativenavigation.utils.ImageLoader;
 import com.reactnativenavigation.utils.OptionHelper;
+import com.reactnativenavigation.utils.ViewUtils;
 import com.reactnativenavigation.viewcontrollers.bottomtabs.BottomTabsController;
 import com.reactnativenavigation.viewcontrollers.modal.ModalStack;
 import com.reactnativenavigation.viewcontrollers.stack.StackController;
@@ -217,20 +218,16 @@ public class NavigatorTest extends BaseTest {
     }
 
     @Test
-    public void popSpecific() {
-        disablePushAnimation(child1, child2, child3, child4);
-        StackController stack1 = newStack(); stack1.ensureViewIsCreated();
-        StackController stack2 = newStack(); stack2.ensureViewIsCreated();
-        stack1.push(child1, new CommandListenerAdapter());
-        stack2.push(child2, new CommandListenerAdapter());
-        stack2.push(child3, new CommandListenerAdapter());
-        stack2.push(child4, new CommandListenerAdapter());
-        BottomTabsController bottomTabsController = newTabs(Arrays.asList(stack1, stack2));
-        uut.setRoot(bottomTabsController, new CommandListenerAdapter());
-
-        uut.popSpecific(child2.getId(), new CommandListenerAdapter());
+    public void pop_byStackId() {
+        disablePushAnimation(child1, child2);
+        disablePopAnimation(child2, child1);
+        StackController stack = newStack(); stack.ensureViewIsCreated();
+        uut.setRoot(stack, new CommandListenerAdapter());
+        stack.push(child1, new CommandListenerAdapter());
+        stack.push(child2, new CommandListenerAdapter());
 
-        assertThat(stack2.getChildControllers()).containsOnly(child4, child3);
+        uut.pop(stack.getId(), new CommandListenerAdapter());
+        assertThat(stack.getChildControllers()).containsOnly(child1);
     }
 
     @Test
@@ -434,8 +431,8 @@ public class NavigatorTest extends BaseTest {
                 assertThat(spy.getChildControllers().size()).isEqualTo(1);
             }
         };
-        uut.popSpecific("child2", listener);
-        verify(spy, times(1)).popSpecific(child2, listener);
+        uut.pop("child2", listener);
+        verify(spy, times(1)).pop(listener);
     }
 
     @Test
@@ -474,6 +471,18 @@ public class NavigatorTest extends BaseTest {
         verify(parentVisibilityListener, times(2)).onViewAppeared(parentController.getView());
     }
 
+    @Test
+    public void dismissModal_reattachedToRoot() {
+        disableModalAnimations(child1);
+
+        uut.setRoot(parentController, new CommandListenerAdapter());
+        assertThat(ViewUtils.isChildOf(uut.getRootLayout(), parentController.getView()));
+        uut.showModal(child1, new CommandListenerAdapter());
+
+        uut.dismissModal(child1.getId(), new CommandListenerAdapter());
+        assertThat(ViewUtils.isChildOf(uut.getRootLayout(), parentController.getView()));
+    }
+
     @Test
     public void dismissModal_rejectIfRootIsNotSetAndSingleModalIsDisplayed() {
         disableModalAnimations(child1, child2);

lib/android/app/src/test/java/com/reactnativenavigation/viewcontrollers/stack/StackControllerTest.java

@@ -424,23 +424,6 @@ public class StackControllerTest extends BaseTest {
         });
     }
 
-    @Test
-    public void pop_specificWhenTopIsRegularPop() {
-        uut.push(child1, new CommandListenerAdapter());
-        uut.push(child2, new CommandListenerAdapter() {
-            @Override
-            public void onSuccess(String childId) {
-                uut.popSpecific(child2, new CommandListenerAdapter() {
-                    @Override
-                    public void onSuccess(String childId) {
-                        assertContainsOnlyId(child1.getId());
-                        assertIsChild(uut.getView(), child1.getView());
-                    }
-                });
-            }
-        });
-    }
-
     @Test
     public void pop_appearingChildHasCorrectLayoutParams() {
         child2.options.animations.pop.enable = new Bool(false);
@@ -460,16 +443,6 @@ public class StackControllerTest extends BaseTest {
                 .getHeight());
     }
 
-    @Test
-    public void popSpecific_deepInStack() {
-        uut.push(child1, new CommandListenerAdapter());
-        uut.push(child2, new CommandListenerAdapter());
-        assertIsChild(uut.getView(), child2.getView());
-        uut.popSpecific(child1, new CommandListenerAdapter());
-        assertContainsOnlyId(child2.getId());
-        assertIsChild(uut.getView(), child2.getView());
-    }
-
     @Test
     public void popTo_PopsTopUntilControllerIsNewTop() {
         uut.push(child1, new CommandListenerAdapter());
@@ -687,20 +660,6 @@ public class StackControllerTest extends BaseTest {
         });
     }
 
-    @Test
-    public void popSpecific_CallsDestroyOnPoppedChild() {
-        child1 = spy(child1);
-        child2 = spy(child2);
-        child3 = spy(child3);
-        uut.push(child1, new CommandListenerAdapter());
-        uut.push(child2, new CommandListenerAdapter());
-        uut.push(child3, new CommandListenerAdapter());
-
-        verify(child2, times(0)).destroy();
-        uut.popSpecific(child2, new CommandListenerAdapter());
-        verify(child2, times(1)).destroy();
-    }
-
     @Test
     public void popTo_CallsDestroyOnPoppedChild() {
         child1 = spy(child1);