From cc46225dbecd2af7268c5bfeb31e5a44832e51ce 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 21a2b4d239..c7c1df285d 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 e855925148..0640240069 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 34d1250bf2..f7ab010093 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 d9f2272c5e..1220369e26 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);