Fix drawer router logic to close drawer (#4394)

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

@@ -6,6 +6,13 @@ import withDefaultValue from '../utils/withDefaultValue';
 
 import DrawerActions from './DrawerActions';
 
+const getActiveRouteKey = route => {
+  if (route.routes && route.routes[route.index]) {
+    return getActiveRouteKey(route.routes[route.index]);
+  }
+  return route.key;
+};
+
 export default (routeConfigs, config = {}) => {
   config = { ...config };
   config = withDefaultValue(config, 'resetOnBlur', false);
@@ -69,16 +76,19 @@ export default (routeConfigs, config = {}) => {
         return null;
       }
 
+      // Has the switch router changed the state?
       if (switchedState !== state) {
-        if (switchedState.index !== state.index) {
-          // If the tabs have changed, make sure to close the drawer
+        if (getActiveRouteKey(switchedState) !== getActiveRouteKey(state)) {
+          // If any navigation has happened, 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
+
+        // At this point, return the state as defined by the switch router.
+        // The active route key hasn't changed, so this most likely means that a child router has returned
+        // a new state like a param change, but the same key is still active and the drawer will remain open
         return switchedState;
       }
 

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

@@ -5,8 +5,10 @@ import React from 'react';
 import StackRouter from '../StackRouter';
 import TabRouter from '../TabRouter';
 import SwitchRouter from '../SwitchRouter';
+import DrawerRouter from '../DrawerRouter';
 
 import NavigationActions from '../../NavigationActions';
+import DrawerActions from '../DrawerActions';
 import { _TESTING_ONLY_normalize_keys } from '../KeyGenerator';
 
 beforeEach(() => {
@@ -16,6 +18,8 @@ beforeEach(() => {
 const ROUTERS = {
   TabRouter,
   StackRouter,
+  DrawerRouter,
+  SwitchRouter,
 };
 
 const dummyEventSubscriber = (name, handler) => ({
@@ -26,7 +30,7 @@ Object.keys(ROUTERS).forEach(routerName => {
   const Router = ROUTERS[routerName];
 
   describe(`General router features - ${routerName}`, () => {
-    test('title is configurable using navigationOptions and getScreenOptions', () => {
+    test(`title is configurable using navigationOptions and getScreenOptions - ${routerName}`, () => {
       class FooView extends React.Component {
         render() {
           return <div />;
@@ -87,6 +91,31 @@ Object.keys(ROUTERS).forEach(routerName => {
         ).title
       ).toEqual('Baz-123');
     });
+
+    test(`set params works in ${routerName}`, () => {
+      class FooView extends React.Component {
+        render() {
+          return <div />;
+        }
+      }
+      const router = Router({
+        Foo: { screen: FooView },
+        Bar: { screen: FooView },
+      });
+
+      const initState = router.getStateForAction(NavigationActions.init());
+      const initRoute = initState.routes[initState.index];
+      expect(initRoute.params).toEqual(undefined);
+
+      const state0 = router.getStateForAction(
+        NavigationActions.setParams({
+          params: { foo: 42 },
+          key: initRoute.key,
+        }),
+        initState
+      );
+      expect(state0.routes[state0.index].params.foo).toEqual(42);
+    });
   });
 });
 
@@ -442,3 +471,53 @@ test('Inner actions are only unpacked if the current tab matches', () => {
     innerState && comparable(innerState)
   );
 });
+
+test('DrawerRouter will close drawer on child navigaton, not on child param changes', () => {
+  class FooView extends React.Component {
+    render() {
+      return <div />;
+    }
+  }
+  const BarRouter = SwitchRouter({
+    Qux: FooView,
+    Quo: FooView,
+  });
+  class BarView extends React.Component {
+    static router = BarRouter;
+    render() {
+      return <div />;
+    }
+  }
+  const router = DrawerRouter({
+    Bar: BarView,
+    Foo: FooView,
+  });
+
+  const emptyState = router.getStateForAction(NavigationActions.init());
+  const initState = router.getStateForAction(
+    DrawerActions.openDrawer(),
+    emptyState
+  );
+  expect(initState.isDrawerOpen).toBe(true);
+
+  const state0 = router.getStateForAction(
+    NavigationActions.navigate({ routeName: 'Quo' }),
+    initState
+  );
+  expect(state0.isDrawerOpen).toBe(false);
+
+  const initSwitchState = initState.routes[initState.index];
+  const initQuxState = initSwitchState.routes[initSwitchState.index];
+
+  const state1 = router.getStateForAction(
+    NavigationActions.setParams({
+      key: initQuxState.key,
+      params: { foo: 'bar' },
+    }),
+    initState
+  );
+  expect(state1.isDrawerOpen).toBe(true);
+  const state1switchState = state1.routes[state1.index];
+  const state1quxState = state1switchState.routes[state1switchState.index];
+  expect(state1quxState.params.foo).toEqual('bar');
+});