Fixes to drawer router handling of child routers (#4131)

Addresses #4129, and also lets child routers swallow actions with null. Tests added

packages/react-navigation/src/routers/DrawerRouter.js

@@ -33,44 +33,52 @@ export default (routeConfigs, config = {}) => {
 
       const isRouterTargeted = action.key == null || action.key === state.key;
 
-      if (
-        isRouterTargeted &&
-        action.type === DrawerActions.CLOSE_DRAWER &&
-        state.isDrawerOpen
-      ) {
-        return {
-          ...state,
-          isDrawerOpen: false,
-        };
+      if (isRouterTargeted) {
+        // Only handle actions that are meant for this drawer, as specified by action.key.
+
+        if (action.type === DrawerActions.CLOSE_DRAWER && state.isDrawerOpen) {
+          return {
+            ...state,
+            isDrawerOpen: false,
+          };
+        }
+
+        if (action.type === DrawerActions.OPEN_DRAWER && !state.isDrawerOpen) {
+          return {
+            ...state,
+            isDrawerOpen: true,
+          };
+        }
+
+        if (action.type === DrawerActions.TOGGLE_DRAWER) {
+          return {
+            ...state,
+            isDrawerOpen: !state.isDrawerOpen,
+          };
+        }
       }
 
-      if (
-        isRouterTargeted &&
-        action.type === DrawerActions.OPEN_DRAWER &&
-        !state.isDrawerOpen
-      ) {
-        return {
-          ...state,
-          isDrawerOpen: true,
-        };
+      // Fall back on switch router for screen switching logic, and handling of child routers
+      const switchedState = switchRouter.getStateForAction(action, state);
+
+      if (switchedState === null) {
+        // The switch router or a child router is attempting to swallow this action. We return null to allow this.
+        return null;
       }
 
-      if (isRouterTargeted && action.type === DrawerActions.TOGGLE_DRAWER) {
-        return {
-          ...state,
-          isDrawerOpen: !state.isDrawerOpen,
-        };
+      if (switchedState !== state) {
+        if (switchedState.index !== state.index) {
+          // If the tabs have changed, make sure to close the drawer
+          return {
+            ...switchedState,
+            isDrawerOpen: false,
+          };
+        }
+        // Return the state new state, as returned by the switch router.
+        // The index hasn't changed, so this most likely means that a child router has returned a new state
+        return switchedState;
       }
 
-      // Fall back on tab router for screen switching logic
-      const childState = switchRouter.getStateForAction(action, state);
-      if (childState !== null && childState !== state) {
-        // If the tabs have changed, make sure to close the drawer
-        return {
-          ...childState,
-          isDrawerOpen: false,
-        };
-      }
       return state;
     },
   };

packages/react-navigation/src/routers/__tests__/DrawerRouter-test.js

@@ -91,3 +91,49 @@ describe('DrawerRouter', () => {
     expect(state3.isDrawerOpen).toEqual(true);
   });
 });
+
+test('Nested routers bubble up blocked actions', () => {
+  const ScreenA = () => <div />;
+  ScreenA.router = {
+    getStateForAction(action, lastState) {
+      if (action.type === 'CHILD_ACTION') return null;
+      return lastState;
+    },
+  };
+  const ScreenB = () => <div />;
+  const router = DrawerRouter({
+    Foo: { screen: ScreenA },
+    Bar: { screen: ScreenB },
+  });
+  const state = router.getStateForAction(INIT_ACTION);
+
+  const state2 = router.getStateForAction({ type: 'CHILD_ACTION' }, state);
+  expect(state2).toEqual(null);
+});
+
+test('Drawer stays open when child routers return new state', () => {
+  const ScreenA = () => <div />;
+  ScreenA.router = {
+    getStateForAction(action, lastState = { changed: false }) {
+      if (action.type === 'CHILD_ACTION')
+        return { ...lastState, changed: true };
+      return lastState;
+    },
+  };
+  const router = DrawerRouter({
+    Foo: { screen: ScreenA },
+  });
+
+  const state = router.getStateForAction(INIT_ACTION);
+  expect(state.isDrawerOpen).toEqual(false);
+
+  const state2 = router.getStateForAction(
+    { type: DrawerActions.OPEN_DRAWER, key: state.key },
+    state
+  );
+  expect(state2.isDrawerOpen).toEqual(true);
+
+  const state3 = router.getStateForAction({ type: 'CHILD_ACTION' }, state2);
+  expect(state3.isDrawerOpen).toEqual(true);
+  expect(state3.routes[0].changed).toEqual(true);
+});