From 858a0d7a5354fcdafaf76d3a179ce070ed66e468 Mon Sep 17 00:00:00 2001 From: Eric Vicenti Date: Thu, 8 Feb 2018 09:02:47 -0800 Subject: [PATCH] StackRouter block actions while transitioning (#3469) The most straightforward fix for two issues is to block all navigation actions while mid-transition of a stack navigator. This will fix: The double-navigate on double tap issue, because the first navigation will start the transition and the second action will be ignored. Will fix the buggy header experience that you can see when going back and forward to a different route quickly. This happens because the next navigate action happens before the completion action. After the fix, the navigate action will be ignored, the user will tap again, and will see a good transition --- src/NavigationActions.js | 6 ++++++ src/__tests__/NavigationContainer-test.js | 14 ++++++------ src/routers/StackRouter.js | 26 +++++++++++++---------- src/routers/__tests__/StackRouter-test.js | 23 +++++++++++++++++--- 4 files changed, 48 insertions(+), 21 deletions(-) diff --git a/src/NavigationActions.js b/src/NavigationActions.js index 21a2b4d..c7c1df2 100644 --- a/src/NavigationActions.js +++ b/src/NavigationActions.js @@ -45,6 +45,9 @@ const navigate = createAction(NAVIGATE, payload => { if (payload.key) { action.key = payload.key; } + if (payload.immediate) { + action.immediate = payload.immediate; + } return action; }); @@ -70,6 +73,9 @@ const push = createAction(PUSH, payload => { if (payload.action) { action.action = payload.action; } + if (payload.immediate) { + action.immediate = payload.immediate; + } return action; }); diff --git a/src/__tests__/NavigationContainer-test.js b/src/__tests__/NavigationContainer-test.js index e855925..0640240 100644 --- a/src/__tests__/NavigationContainer-test.js +++ b/src/__tests__/NavigationContainer-test.js @@ -109,7 +109,7 @@ describe('NavigationContainer', () => { // First dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'bar' }) + NavigationActions.navigate({ routeName: 'bar', immediate: true }) ) ).toEqual(true); @@ -119,7 +119,7 @@ describe('NavigationContainer', () => { // Second dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'baz' }) + NavigationActions.navigate({ routeName: 'baz', immediate: true }) ) ).toEqual(true); @@ -147,7 +147,7 @@ describe('NavigationContainer', () => { // First dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'bar' }) + NavigationActions.navigate({ routeName: 'bar', immediate: true }) ) ).toEqual(true); @@ -157,28 +157,28 @@ describe('NavigationContainer', () => { // Second dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'baz' }) + NavigationActions.navigate({ routeName: 'baz', immediate: true }) ) ).toEqual(true); // Third dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'car' }) + NavigationActions.navigate({ routeName: 'car', immediate: true }) ) ).toEqual(true); // Fourth dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'dog' }) + NavigationActions.navigate({ routeName: 'dog', immediate: true }) ) ).toEqual(true); // Fifth dispatch expect( navigationContainer.dispatch( - NavigationActions.navigate({ routeName: 'elk' }) + NavigationActions.navigate({ routeName: 'elk', immediate: true }) ) ).toEqual(true); diff --git a/src/routers/StackRouter.js b/src/routers/StackRouter.js index 34d1250..f7ab010 100644 --- a/src/routers/StackRouter.js +++ b/src/routers/StackRouter.js @@ -148,6 +148,17 @@ export default (routeConfigs, stackConfig = {}) => { }; } + if ( + action.type === NavigationActions.COMPLETE_TRANSITION && + (action.key == null || action.key === state.key) && + state.isTransitioning + ) { + return { + ...state, + isTransitioning: false, + }; + } + // Check if a child scene wants to handle the action as long as it is not a reset to the root stack if (action.type !== NavigationActions.RESET || action.key !== null) { const keyIndex = action.key @@ -218,6 +229,10 @@ export default (routeConfigs, stackConfig = {}) => { behavesLikePushAction(action) && childRouters[action.routeName] !== undefined ) { + if (state.isTransitioning) { + return { ...state }; + } + const childRouter = childRouters[action.routeName]; let route; @@ -290,17 +305,6 @@ export default (routeConfigs, stackConfig = {}) => { }; } - if ( - action.type === NavigationActions.COMPLETE_TRANSITION && - (action.key == null || action.key === state.key) && - state.isTransitioning - ) { - return { - ...state, - isTransitioning: false, - }; - } - // Handle navigation to other child routers that are not yet pushed if (behavesLikePushAction(action)) { const childRouterNames = Object.keys(childRouters); diff --git a/src/routers/__tests__/StackRouter-test.js b/src/routers/__tests__/StackRouter-test.js index d9f2272..1220369 100644 --- a/src/routers/__tests__/StackRouter-test.js +++ b/src/routers/__tests__/StackRouter-test.js @@ -493,6 +493,23 @@ describe('StackRouter', () => { expect(poppedImmediatelyState.isTransitioning).toBe(false); }); + test('Navigate does not happen while transitioning', () => { + const TestRouter = StackRouter({ + foo: { screen: () =>
}, + bar: { screen: () =>
}, + }); + const initState = { + ...TestRouter.getStateForAction(NavigationActions.init()), + isTransitioning: true, + }; + const pushedState = TestRouter.getStateForAction( + NavigationActions.navigate({ routeName: 'bar' }), + initState + ); + expect(pushedState.index).toEqual(0); + expect(pushedState.routes.length).toEqual(1); + }); + test('Navigate Pushes duplicate routeName', () => { const TestRouter = StackRouter({ foo: { screen: () =>
}, @@ -500,13 +517,13 @@ describe('StackRouter', () => { }); const initState = TestRouter.getStateForAction(NavigationActions.init()); const pushedState = TestRouter.getStateForAction( - NavigationActions.navigate({ routeName: 'bar' }), + NavigationActions.navigate({ routeName: 'bar', immediate: true }), initState ); expect(pushedState.index).toEqual(1); expect(pushedState.routes[1].routeName).toEqual('bar'); const pushedTwiceState = TestRouter.getStateForAction( - NavigationActions.navigate({ routeName: 'bar' }), + NavigationActions.navigate({ routeName: 'bar', immediate: true }), pushedState ); expect(pushedTwiceState.index).toEqual(2); @@ -540,7 +557,7 @@ describe('StackRouter', () => { }); const initState = TestRouter.getStateForAction(NavigationActions.init()); const pushedState = TestRouter.getStateForAction( - NavigationActions.push({ routeName: 'bar' }), + NavigationActions.push({ routeName: 'bar', immediate: true }), initState ); expect(pushedState.index).toEqual(1);