From 8af1f90a11630d9ce50df2487f55b4ac5c595a5c Mon Sep 17 00:00:00 2001
From: Brent Vatne <brentvatne@gmail.com>
Date: Fri, 22 Jun 2018 10:17:36 -0700
Subject: [PATCH] Single source of truth for drawer open state: the view

---
 packages/drawer/example/src/SimpleDrawer.js   | 19 +++++-
 packages/drawer/src/routers/DrawerActions.js  |  4 ++
 packages/drawer/src/routers/DrawerRouter.js   | 27 ++++++--
 .../routers/__tests__/DrawerRouter-test.js    | 45 +++++++------
 packages/drawer/src/views/DrawerView.js       | 67 ++++++++-----------
 5 files changed, 95 insertions(+), 67 deletions(-)

diff --git a/packages/drawer/example/src/SimpleDrawer.js b/packages/drawer/example/src/SimpleDrawer.js
index 59eaa17a..f20b102f 100644
--- a/packages/drawer/example/src/SimpleDrawer.js
+++ b/packages/drawer/example/src/SimpleDrawer.js
@@ -1,16 +1,29 @@
 import React from 'react';
-import { Button, ScrollView, StatusBar, Text } from 'react-native';
+import { Button, ScrollView, StatusBar, Text, View } from 'react-native';
 import { createStackNavigator, SafeAreaView } from 'react-navigation';
 import MaterialIcons from 'react-native-vector-icons/MaterialIcons';
 import { createDrawerNavigator } from 'react-navigation-drawer';
+import { KeepAwake } from 'expo';
 
 const SampleText = ({ children }) => <Text>{children}</Text>;
 
 const MyNavScreen = ({ navigation, banner }) => (
   <ScrollView>
     <SafeAreaView forceInset={{ top: 'always' }}>
-      <SampleText>{banner}</SampleText>
+      <View style={{ flex: 1, alignItems: 'center', justifyContent: 'center' }}>
+        <SampleText>{banner}</SampleText>
+      </View>
       <Button onPress={() => navigation.openDrawer()} title="Open drawer" />
+      <Button onPress={() => navigation.toggleDrawer()} title="Toggle drawer" />
+      <Button
+        onPress={() => {
+          navigation.openDrawer();
+          setTimeout(() => {
+            navigation.closeDrawer();
+          }, 500);
+        }}
+        title="Open then close drawer shortly after"
+      />
       <Button
         onPress={() => navigation.navigate('Email')}
         title="Open other screen"
@@ -22,6 +35,7 @@ const MyNavScreen = ({ navigation, banner }) => (
       />
     </SafeAreaView>
     <StatusBar barStyle="default" />
+    <KeepAwake />
   </ScrollView>
 );
 
@@ -84,6 +98,7 @@ const DrawerExample = createDrawerNavigator(
   },
   {
     initialRouteName: 'Drafts',
+    drawerWidth: 210,
     contentOptions: {
       activeTintColor: '#e91e63',
     },
diff --git a/packages/drawer/src/routers/DrawerActions.js b/packages/drawer/src/routers/DrawerActions.js
index 5dae0d31..62e8147c 100644
--- a/packages/drawer/src/routers/DrawerActions.js
+++ b/packages/drawer/src/routers/DrawerActions.js
@@ -1,6 +1,8 @@
 const OPEN_DRAWER = 'Navigation/OPEN_DRAWER';
 const CLOSE_DRAWER = 'Navigation/CLOSE_DRAWER';
 const TOGGLE_DRAWER = 'Navigation/TOGGLE_DRAWER';
+const DRAWER_OPENED = 'Navigation/DRAWER_OPENED';
+const DRAWER_CLOSED = 'Navigation/DRAWER_CLOSED';
 
 const openDrawer = payload => ({
   type: OPEN_DRAWER,
@@ -21,6 +23,8 @@ export default {
   OPEN_DRAWER,
   CLOSE_DRAWER,
   TOGGLE_DRAWER,
+  DRAWER_OPENED,
+  DRAWER_CLOSED,
 
   openDrawer,
   closeDrawer,
diff --git a/packages/drawer/src/routers/DrawerRouter.js b/packages/drawer/src/routers/DrawerRouter.js
index 02fa9171..8e3719e9 100644
--- a/packages/drawer/src/routers/DrawerRouter.js
+++ b/packages/drawer/src/routers/DrawerRouter.js
@@ -42,6 +42,9 @@ export default (routeConfigs, config = {}) => {
         return {
           ...switchRouter.getStateForAction(action, undefined),
           isDrawerOpen: false,
+          openId: 0,
+          closeId: 0,
+          toggleId: 0,
         };
       }
 
@@ -50,24 +53,40 @@ export default (routeConfigs, config = {}) => {
       if (isRouterTargeted) {
         // Only handle actions that are meant for this drawer, as specified by action.key.
 
-        if (action.type === DrawerActions.CLOSE_DRAWER && state.isDrawerOpen) {
+        if (action.type === DrawerActions.DRAWER_CLOSED) {
+          console.log('closed');
           return {
             ...state,
             isDrawerOpen: false,
           };
         }
 
-        if (action.type === DrawerActions.OPEN_DRAWER && !state.isDrawerOpen) {
+        if (action.type === DrawerActions.DRAWER_OPENED) {
+          console.log('opened');
           return {
             ...state,
             isDrawerOpen: true,
           };
         }
 
+        if (action.type === DrawerActions.CLOSE_DRAWER) {
+          return {
+            ...state,
+            closeId: state.closeId + 1,
+          };
+        }
+
+        if (action.type === DrawerActions.OPEN_DRAWER) {
+          return {
+            ...state,
+            openId: state.openId + 1,
+          };
+        }
+
         if (action.type === DrawerActions.TOGGLE_DRAWER) {
           return {
             ...state,
-            isDrawerOpen: !state.isDrawerOpen,
+            toggleId: state.toggleId + 1,
           };
         }
       }
@@ -86,7 +105,7 @@ export default (routeConfigs, config = {}) => {
           // If any navigation has happened, make sure to close the drawer
           return {
             ...switchedState,
-            isDrawerOpen: false,
+            closeId: state.closeId + 1,
           };
         }
 
diff --git a/packages/drawer/src/routers/__tests__/DrawerRouter-test.js b/packages/drawer/src/routers/__tests__/DrawerRouter-test.js
index fdf49e47..8c4dcf5e 100644
--- a/packages/drawer/src/routers/__tests__/DrawerRouter-test.js
+++ b/packages/drawer/src/routers/__tests__/DrawerRouter-test.js
@@ -9,7 +9,7 @@ import DrawerActions from '../../routers/DrawerActions';
 const INIT_ACTION = { type: NavigationActions.INIT };
 
 describe('DrawerRouter', () => {
-  test('Handles basic tab logic', () => {
+  test('Handles basic drawer logic and fires close on switch', () => {
     const ScreenA = () => <div />;
     const ScreenB = () => <div />;
     const router = DrawerRouter({
@@ -25,6 +25,9 @@ describe('DrawerRouter', () => {
         { key: 'Bar', routeName: 'Bar', params: undefined },
       ],
       isDrawerOpen: false,
+      openId: 0,
+      closeId: 0,
+      toggleId: 0,
     };
     expect(state).toEqual(expectedState);
     const state2 = router.getStateForAction(
@@ -39,6 +42,9 @@ describe('DrawerRouter', () => {
         { key: 'Bar', routeName: 'Bar', params: undefined },
       ],
       isDrawerOpen: false,
+      openId: 0,
+      closeId: 1,
+      toggleId: 0,
     };
     expect(state2).toEqual(expectedState2);
     expect(router.getComponentForState(expectedState)).toEqual(ScreenA);
@@ -67,6 +73,9 @@ describe('DrawerRouter', () => {
       index: 0,
       isDrawerOpen: false,
       isTransitioning: false,
+      openId: 0,
+      closeId: 0,
+      toggleId: 0,
       routes: [
         {
           key: 'Foo',
@@ -90,22 +99,22 @@ describe('DrawerRouter', () => {
       Bar: { screen: ScreenB },
     });
     const state = router.getStateForAction(INIT_ACTION);
-    expect(state.isDrawerOpen).toEqual(false);
+    expect(state.toggleId).toEqual(0);
     const state2 = router.getStateForAction(
       { type: DrawerActions.OPEN_DRAWER },
       state
     );
-    expect(state2.isDrawerOpen).toEqual(true);
+    expect(state2.openId).toEqual(1);
     const state3 = router.getStateForAction(
       { type: DrawerActions.CLOSE_DRAWER },
       state2
     );
-    expect(state3.isDrawerOpen).toEqual(false);
+    expect(state3.closeId).toEqual(1);
     const state4 = router.getStateForAction(
       { type: DrawerActions.TOGGLE_DRAWER },
       state3
     );
-    expect(state4.isDrawerOpen).toEqual(true);
+    expect(state4.toggleId).toEqual(1);
   });
 
   test('Drawer opens closes with key targeted', () => {
@@ -120,12 +129,12 @@ describe('DrawerRouter', () => {
       { type: DrawerActions.OPEN_DRAWER, key: 'wrong' },
       state
     );
-    expect(state2.isDrawerOpen).toEqual(false);
+    expect(state2.openId).toEqual(0);
     const state3 = router.getStateForAction(
       { type: DrawerActions.OPEN_DRAWER, key: state.key },
       state2
     );
-    expect(state3.isDrawerOpen).toEqual(true);
+    expect(state3.openId).toEqual(1);
   });
 });
 
@@ -148,7 +157,7 @@ test('Nested routers bubble up blocked actions', () => {
   expect(state2).toEqual(null);
 });
 
-test('Drawer stays open when child routers return new state', () => {
+test('Drawer does not fire close when child routers return new state', () => {
   const ScreenA = () => <div />;
   ScreenA.router = {
     getStateForAction(action, lastState = { changed: false }) {
@@ -162,17 +171,11 @@ test('Drawer stays open when child routers return new state', () => {
   });
 
   const state = router.getStateForAction(INIT_ACTION);
-  expect(state.isDrawerOpen).toEqual(false);
+  expect(state.closeId).toEqual(0);
 
-  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);
+  const state2 = router.getStateForAction({ type: 'CHILD_ACTION' }, state2);
+  expect(state2.closeId).toEqual(0);
+  expect(state2.routes[0].changed).toEqual(true);
 });
 
 test('DrawerRouter will close drawer on child navigaton, not on child param changes', () => {
@@ -201,13 +204,13 @@ test('DrawerRouter will close drawer on child navigaton, not on child param chan
     DrawerActions.openDrawer(),
     emptyState
   );
-  expect(initState.isDrawerOpen).toBe(true);
+  expect(initState.openId).toBe(1);
 
   const state0 = router.getStateForAction(
     NavigationActions.navigate({ routeName: 'Quo' }),
     initState
   );
-  expect(state0.isDrawerOpen).toBe(false);
+  expect(state0.closeId).toBe(1);
 
   const initSwitchState = initState.routes[initState.index];
   const initQuxState = initSwitchState.routes[initSwitchState.index];
@@ -219,8 +222,8 @@ test('DrawerRouter will close drawer on child navigaton, not on child param chan
     }),
     initState
   );
-  expect(state1.isDrawerOpen).toBe(true);
   const state1switchState = state1.routes[state1.index];
   const state1quxState = state1switchState.routes[state1switchState.index];
+  expect(state1.closeId).toBe(0); // don't fire close
   expect(state1quxState.params.foo).toEqual('bar');
 });
diff --git a/packages/drawer/src/views/DrawerView.js b/packages/drawer/src/views/DrawerView.js
index 572d67f2..a897b1cd 100644
--- a/packages/drawer/src/views/DrawerView.js
+++ b/packages/drawer/src/views/DrawerView.js
@@ -22,19 +22,28 @@ export default class DrawerView extends React.PureComponent {
   }
 
   componentDidUpdate(prevProps) {
-    const { isDrawerOpen, key } = this.props.navigation.state;
-    const prevKey = prevProps.navigation.state.key;
-    const wasDrawerOpen = prevProps.navigation.state.isDrawerOpen;
-    const shouldOpen = this._shouldOpen(isDrawerOpen, wasDrawerOpen);
-    const shouldClose =
-      this._shouldClose(isDrawerOpen, wasDrawerOpen) || key !== prevKey;
+    const {
+      openId,
+      closeId,
+      toggleId,
+      drawerIsOpen,
+    } = this.props.navigation.state;
+    const {
+      openId: prevOpenId,
+      closeId: prevCloseId,
+      toggleId: prevToggleId,
+    } = prevProps.navigation.state;
 
-    if (shouldOpen) {
-      this._drawerState = 'opening';
+    if (openId !== prevOpenId) {
       this._drawer.openDrawer();
-    } else if (shouldClose) {
-      this._drawerState = 'closing';
+    } else if (closeId !== prevCloseId) {
       this._drawer.closeDrawer();
+    } else if (toggleId !== prevToggleId) {
+      if (drawerIsOpen) {
+        this._drawer.closeDrawer();
+      } else {
+        this._drawer.openDrawer();
+      }
     }
   }
 
@@ -42,40 +51,18 @@ export default class DrawerView extends React.PureComponent {
     Dimensions.removeEventListener('change', this._updateWidth);
   }
 
-  _drawerState = 'closed';
-
-  _shouldOpen = (isDrawerOpen, wasDrawerOpen) => {
-    return (
-      isDrawerOpen &&
-      !wasDrawerOpen &&
-      (this._drawerState === 'closed' || this._drawerState === 'closing')
-    );
-  };
-
-  _shouldClose = (isDrawerOpen, wasDrawerOpen) => {
-    return (
-      wasDrawerOpen &&
-      !isDrawerOpen &&
-      (this._drawerState === 'open' || this._drawerState === 'opening')
-    );
-  };
-
   _handleDrawerOpen = () => {
-    const { navigation } = this.props;
-    const { isDrawerOpen } = navigation.state;
-    if (!isDrawerOpen && this._drawerState === 'closed') {
-      navigation.dispatch({ type: DrawerActions.OPEN_DRAWER });
-    }
-    this._drawerState = 'open';
+    this.props.navigation.dispatch({
+      type: DrawerActions.DRAWER_OPENED,
+      key: this.props.navigation.state.key,
+    });
   };
 
   _handleDrawerClose = () => {
-    const { navigation } = this.props;
-    const { isDrawerOpen } = navigation.state;
-    if (isDrawerOpen && this._drawerState === 'open') {
-      navigation.dispatch({ type: DrawerActions.CLOSE_DRAWER });
-    }
-    this._drawerState = 'closed';
+    this.props.navigation.dispatch({
+      type: DrawerActions.DRAWER_CLOSED,
+      key: this.props.navigation.state.key,
+    });
   };
 
   _updateWidth = () => {