diff --git a/src/ZulipMobile.js b/src/ZulipMobile.js index 3e305a32c5b..5bb53d63d35 100644 --- a/src/ZulipMobile.js +++ b/src/ZulipMobile.js @@ -10,7 +10,9 @@ import CompatibilityChecker from './boot/CompatibilityChecker'; import AppEventHandlers from './boot/AppEventHandlers'; import AppDataFetcher from './boot/AppDataFetcher'; import BackNavigationHandler from './nav/BackNavigationHandler'; +import InitialNavigationDispatcher from './nav/InitialNavigationDispatcher'; import AppWithNavigation from './nav/AppWithNavigation'; +import NavigationService from './nav/NavigationService'; import './i18n/locale'; import { initializeSentry } from './sentry'; @@ -27,9 +29,11 @@ export default (): React$Node => ( - - - + + + + + diff --git a/src/account/AccountPickScreen.js b/src/account/AccountPickScreen.js index e620be5f937..1f2a4525f85 100644 --- a/src/account/AccountPickScreen.js +++ b/src/account/AccountPickScreen.js @@ -32,7 +32,7 @@ class AccountPickScreen extends PureComponent { dispatch(accountSwitch(index)); }); } else { - dispatch(navigateToRealmScreen(realm)); + dispatch(navigateToRealmScreen({ realm })); } }; diff --git a/src/account/accountActions.js b/src/account/accountActions.js index 056e00269d8..52fbcf057d6 100644 --- a/src/account/accountActions.js +++ b/src/account/accountActions.js @@ -1,5 +1,5 @@ /* @flow strict-local */ -import type { Action } from '../types'; +import type { Action, Dispatch, GetState } from '../types'; import { ACCOUNT_SWITCH, REALM_ADD, @@ -7,13 +7,19 @@ import { LOGIN_SUCCESS, LOGOUT, } from '../actionConstants'; +import { resetToAccountPicker, resetToLoading } from '../nav/navActions'; import type { ZulipVersion } from '../utils/zulipVersion'; -export const accountSwitch = (index: number): Action => ({ +const accountSwitchPlain = (index: number): Action => ({ type: ACCOUNT_SWITCH, index, }); +export const accountSwitch = (index: number) => (dispatch: Dispatch, getState: GetState) => { + dispatch(resetToLoading()); + dispatch(accountSwitchPlain(index)); +}; + export const realmAdd = ( realm: URL, zulipFeatureLevel: number, @@ -30,13 +36,26 @@ export const removeAccount = (index: number): Action => ({ index, }); -export const loginSuccess = (realm: URL, email: string, apiKey: string): Action => ({ +const loginSuccessPlain = (realm: URL, email: string, apiKey: string): Action => ({ type: LOGIN_SUCCESS, realm, email, apiKey, }); -export const logout = (): Action => ({ +export const loginSuccess = (realm: URL, email: string, apiKey: string) => ( + dispatch: Dispatch, + getState: GetState, +) => { + dispatch(resetToLoading()); + dispatch(loginSuccessPlain(realm, email, apiKey)); +}; + +const logoutPlain = (): Action => ({ type: LOGOUT, }); + +export const logout = () => async (dispatch: Dispatch, getState: GetState) => { + dispatch(resetToAccountPicker()); + dispatch(logoutPlain()); +}; diff --git a/src/boot/store.js b/src/boot/store.js index e3193d753ef..91d04f1e8ac 100644 --- a/src/boot/store.js +++ b/src/boot/store.js @@ -17,7 +17,6 @@ import { REHYDRATE } from '../actionConstants'; import rootReducer from './reducers'; import ZulipAsyncStorage from './ZulipAsyncStorage'; import createMigration from '../redux-persist-migrate/index'; -import { getNav } from '../nav/navSelectors'; // AsyncStorage.clear(); // use to reset storage during development @@ -212,7 +211,7 @@ function listMiddleware() { const result = [ // Allow us to cause navigation by dispatching Redux actions. // See docs: https://github.com/react-navigation/redux-helpers - createReactNavigationReduxMiddleware(getNav, 'root'), + createReactNavigationReduxMiddleware((state: GlobalState) => state.nav, 'root'), // Delay ("buffer") actions until a REHYDRATE action comes through. // After dispatching the latter, this will go back and dispatch diff --git a/src/events/doEventActionSideEffects.js b/src/events/doEventActionSideEffects.js index 9301d109dc8..58beaaecf66 100644 --- a/src/events/doEventActionSideEffects.js +++ b/src/events/doEventActionSideEffects.js @@ -28,7 +28,9 @@ const messageEvent = (state: GlobalState, message: Message): void => { } const activeAccount = getActiveAccount(state); - const { narrow } = getChatScreenParams(state); + // Assume (unchecked) that `narrow` is `Narrow` if present + // $FlowFixMe + const narrow: Narrow | void = getChatScreenParams().narrow; const isUserInSameNarrow = activeAccount && narrow !== undefined // chat screen is not at top diff --git a/src/message/__tests__/fetchActions-test.js b/src/message/__tests__/fetchActions-test.js index ec9113b97fc..071986d7a86 100644 --- a/src/message/__tests__/fetchActions-test.js +++ b/src/message/__tests__/fetchActions-test.js @@ -16,7 +16,6 @@ import { FIRST_UNREAD_ANCHOR } from '../../anchor'; import type { Message } from '../../api/modelTypes'; import type { ServerMessage } from '../../api/messages/getMessages'; import { streamNarrow, HOME_NARROW, HOME_NARROW_STR } from '../../utils/narrow'; -import { navStateWithNarrow } from '../../utils/testHelpers'; import * as eg from '../../__tests__/lib/exampleData'; const mockStore = configureStore([thunk]); @@ -41,7 +40,6 @@ describe('fetchActions', () => { older: true, }, }, - ...navStateWithNarrow(HOME_NARROW), narrows: { [streamNarrowStr]: [], }, @@ -64,7 +62,6 @@ describe('fetchActions', () => { older: false, }, }, - ...navStateWithNarrow(HOME_NARROW), narrows: { [streamNarrowStr]: [1], }, @@ -116,7 +113,6 @@ describe('fetchActions', () => { const message1 = eg.streamMessage({ id: 1 }); const baseState = eg.reduxState({ - ...navStateWithNarrow(HOME_NARROW), accounts: [eg.makeAccount()], narrows: { [streamNarrowStr]: [message1.id], @@ -295,7 +291,6 @@ describe('fetchActions', () => { test('when messages to be fetched both before and after anchor, numBefore and numAfter are greater than zero', async () => { const store = mockStore( eg.reduxState({ - ...navStateWithNarrow(HOME_NARROW), accounts: [eg.selfAccount], narrows: { [streamNarrowStr]: [1], @@ -322,7 +317,6 @@ describe('fetchActions', () => { test('when no messages to be fetched before the anchor, numBefore is not greater than zero', async () => { const store = mockStore( eg.reduxState({ - ...navStateWithNarrow(HOME_NARROW), accounts: [eg.selfAccount], narrows: { [streamNarrowStr]: [1], @@ -348,7 +342,6 @@ describe('fetchActions', () => { test('when no messages to be fetched after the anchor, numAfter is not greater than zero', async () => { const store = mockStore( eg.reduxState({ - ...navStateWithNarrow(HOME_NARROW), accounts: [eg.selfAccount], narrows: { [streamNarrowStr]: [1], @@ -377,7 +370,6 @@ describe('fetchActions', () => { const message2 = eg.streamMessage({ id: 2 }); const baseState = eg.reduxState({ - ...navStateWithNarrow(HOME_NARROW), accounts: [eg.selfAccount], narrows: { ...eg.baseReduxState.narrows, @@ -471,7 +463,6 @@ describe('fetchActions', () => { const message2 = eg.streamMessage({ id: 2 }); const baseState = eg.reduxState({ - ...navStateWithNarrow(HOME_NARROW), accounts: [eg.selfAccount], narrows: { ...eg.baseReduxState.narrows, diff --git a/src/message/__tests__/messageActions-test.js b/src/message/__tests__/messageActions-test.js index 213968e188d..406109aa2e2 100644 --- a/src/message/__tests__/messageActions-test.js +++ b/src/message/__tests__/messageActions-test.js @@ -3,8 +3,7 @@ import configureStore from 'redux-mock-store'; import thunk from 'redux-thunk'; import { doNarrow } from '../messagesActions'; -import { streamNarrow, HOME_NARROW } from '../../utils/narrow'; -import { navStateWithNarrow } from '../../utils/testHelpers'; +import { streamNarrow } from '../../utils/narrow'; import * as eg from '../../__tests__/lib/exampleData'; import type { GlobalState } from '../../reduxTypes'; import type { Action } from '../../actionTypes'; @@ -20,7 +19,6 @@ describe('messageActions', () => { eg.reduxState({ accounts: [eg.selfAccount], session: { ...eg.baseReduxState.session, isHydrated: true }, - ...navStateWithNarrow(HOME_NARROW), streams: [eg.makeStream({ name: 'some stream' })], }), ); diff --git a/src/message/fetchActions.js b/src/message/fetchActions.js index 07d2d4c355d..90b17f919ae 100644 --- a/src/message/fetchActions.js +++ b/src/message/fetchActions.js @@ -10,6 +10,7 @@ import type { } from '../types'; import type { InitialData } from '../api/initialDataTypes'; import * as api from '../api'; +import { resetToMainTabs } from '../actions'; import { isClientError } from '../api/apiErrors'; import { getAuth, @@ -18,6 +19,7 @@ import { getLastMessageId, getCaughtUpForNarrow, getFetchingForNarrow, + getNavigationRoutes, } from '../selectors'; import config from '../config'; import { @@ -156,10 +158,26 @@ const initialFetchStart = (): Action => ({ type: INITIAL_FETCH_START, }); -const initialFetchComplete = (): Action => ({ +const initialFetchCompletePlain = (): Action => ({ type: INITIAL_FETCH_COMPLETE, }); +export const initialFetchComplete = () => async (dispatch: Dispatch, getState: GetState) => { + if (!getNavigationRoutes().some(navigationRoute => navigationRoute.routeName === 'main')) { + // If we're anywhere in the normal UI of the app, then remain + // where we are. Only reset the nav state if we're elsewhere, + // and in that case, go to the main screen. + // + // TODO: "elsewhere" is probably just a way of saying "on the + // loading screen", but we're not sure. We could adjust the + // conditional accordingly, if we found out we're not depending on + // the more general condition; see + // https://github.com/zulip/zulip-mobile/pull/4274#discussion_r505941875 + dispatch(resetToMainTabs()); + } + dispatch(initialFetchCompletePlain()); +}; + /** Private; exported only for tests. */ export const isFetchNeededAtAnchor = ( state: GlobalState, diff --git a/src/nav/AppNavigator.js b/src/nav/AppNavigator.js index 1dc1b47c0f5..6a8c164c837 100644 --- a/src/nav/AppNavigator.js +++ b/src/nav/AppNavigator.js @@ -94,7 +94,7 @@ export default createStackNavigator( ios: TransitionPresets.DefaultTransition, }), }, - initialRouteName: 'main', + initialRouteName: 'loading', headerMode: 'none', }, ); diff --git a/src/nav/AppWithNavigation.js b/src/nav/AppWithNavigation.js index 45bd8b86ff9..abf650f5e9e 100644 --- a/src/nav/AppWithNavigation.js +++ b/src/nav/AppWithNavigation.js @@ -2,10 +2,15 @@ import { createReduxContainer } from 'react-navigation-redux-helpers'; -import { connect } from '../react-redux'; -import { getNav } from '../selectors'; +import { connect } from 'react-redux'; import AppNavigator from './AppNavigator'; -export default connect(state => ({ - state: getNav(state), -}))(createReduxContainer(AppNavigator, 'root')); +// $FlowFixMe - should use a type-checked `connect` +export default connect( + state => ({ + state: state.nav, + }), + null, + null, + { forwardRef: true }, +)((createReduxContainer(AppNavigator, 'root'): React$ComponentType<{||}>)); diff --git a/src/nav/BackNavigationHandler.js b/src/nav/BackNavigationHandler.js index 12965f31a86..e098d81255b 100644 --- a/src/nav/BackNavigationHandler.js +++ b/src/nav/BackNavigationHandler.js @@ -6,7 +6,6 @@ import { BackHandler } from 'react-native'; import type { Dispatch } from '../types'; import { connect } from '../react-redux'; -import { getCanGoBack } from '../selectors'; import { navigateBack } from '../actions'; type Props = $ReadOnly<{| @@ -38,5 +37,5 @@ class BackNavigationHandler extends PureComponent { } export default connect(state => ({ - canGoBack: getCanGoBack(state), + canGoBack: state.nav.index > 0, }))(BackNavigationHandler); diff --git a/src/nav/InitialNavigationDispatcher.js b/src/nav/InitialNavigationDispatcher.js new file mode 100644 index 00000000000..dde72678b28 --- /dev/null +++ b/src/nav/InitialNavigationDispatcher.js @@ -0,0 +1,95 @@ +/* @flow strict-local */ +import type { Node as React$Node } from 'react'; +import { PureComponent } from 'react'; + +import type { Dispatch, Account } from '../types'; +import { resetToAccountPicker, resetToRealmScreen, resetToMainTabs } from '../actions'; +import { connect } from '../react-redux'; +import { getIsHydrated, hasAuth as getHasAuth, getHaveServerData } from '../selectors'; + +type SelectorProps = $ReadOnly<{| + isHydrated: boolean, + hasAuth: boolean, + accounts: Account[], + haveServerData: boolean, +|}>; + +type Props = $ReadOnly<{| + children: React$Node, + + dispatch: Dispatch, + ...SelectorProps, +|}>; + +class InitialNavigationDispatcher extends PureComponent { + componentDidMount() { + if (this.props.isHydrated) { + this.doInitialNavigation(); + } + } + + componentDidUpdate(prevProps) { + if (!prevProps.isHydrated && this.props.isHydrated) { + this.doInitialNavigation(); + } + } + + /** + * Data has been loaded, so open the app to the right screen. + * + * Not to be called before the REHYDRATE action, and not to be + * called more than once. + */ + doInitialNavigation = () => { + const { hasAuth, accounts, haveServerData, dispatch } = this.props; + + // If the active account is not logged in, bring the user as close + // as we can to AuthScreen, the place where they can log in. + if (!hasAuth) { + if (accounts.length > 1) { + // We can't guess which account, of multiple, the user wants + // to use. Let them pick one. + dispatch(resetToAccountPicker()); + return; + } else if (accounts.length === 1) { + // We already know the realm, so give that to the realm + // screen. If that screen finds that the realm is valid, it'll + // send the user along to AuthScreen for that realm right + // away. If this means you're on the AuthScreen when you don't + // want to be (i.e., you want to choose a different realm), + // you can always go back to RealmScreen. + dispatch(resetToRealmScreen({ initial: true, realm: accounts[0].realm })); + return; + } else { + // Just go to the realm screen and have the user type out the + // realm. + dispatch(resetToRealmScreen({ initial: true })); + return; + } + } + + // If there's an active, logged-in account but no server data, then behave + // like `ACCOUNT_SWITCH`: show loading screen. Crucially, `sessionReducer` + // will have set `needsInitialFetch`, too, so we really will be loading. + if (!haveServerData) { + // We're already on the loading screen -- see `initialRouteName` + // in `AppNavigator`. + return; + } + + // Great: we have an active, logged-in account, and server data for it. + // Show the main UI. + dispatch(resetToMainTabs()); + }; + + render() { + return this.props.children; + } +} + +export default connect(state => ({ + hasAuth: getHasAuth(state), + accounts: state.accounts, + haveServerData: getHaveServerData(state), + isHydrated: getIsHydrated(state), +}))(InitialNavigationDispatcher); diff --git a/src/nav/NavigationService.js b/src/nav/NavigationService.js new file mode 100644 index 00000000000..bc20cf6a2b2 --- /dev/null +++ b/src/nav/NavigationService.js @@ -0,0 +1,54 @@ +/* @flow strict-local */ +import React from 'react'; +import type { + NavigationAction, + NavigationNavigatorProps, + NavigationState, + NavigationDispatch, + SupportedThemes, +} from 'react-navigation'; + +type ReduxContainerProps = { + ...$Diff, { navigation: mixed }>, + state: NavigationState, + dispatch: NavigationDispatch, + theme: SupportedThemes | 'no-preference', +}; + +// Should mimic return type of react-navigation-redux-helpers's +// `createReduxContainer`. +type ReduxContainer = React$Component; + +// TODO: This will eventually be a ref to the component instance +// created by React Navigation's `createAppContainer`, not +// react-navigation-redux-helpers's `createReduxContainer`. +const reduxContainerRef = React.createRef(); + +const getState = (): NavigationState => { + if (reduxContainerRef.current === null) { + throw new Error('Tried to use NavigationService before reduxContainerRef was set.'); + } + return ( + reduxContainerRef.current + // $FlowFixMe - how to tell Flow about this method? + .getCurrentNavigation().state + ); +}; + +const dispatch = (navigationAction: NavigationAction): boolean => { + if (reduxContainerRef.current === null) { + throw new Error('Tried to use NavigationService before reduxContainerRef was set.'); + } + return ( + reduxContainerRef.current + // $FlowFixMe - how to tell Flow about this method? + .getCurrentNavigation() + .dispatch(navigationAction) + ); +}; + +export default { + getState, + dispatch, + reduxContainerRef, +}; diff --git a/src/nav/__tests__/navReducer-test.js b/src/nav/__tests__/navReducer-test.js deleted file mode 100644 index add69acfc58..00000000000 --- a/src/nav/__tests__/navReducer-test.js +++ /dev/null @@ -1,182 +0,0 @@ -import deepFreeze from 'deep-freeze'; - -import { LOGIN_SUCCESS, INITIAL_FETCH_COMPLETE, REHYDRATE } from '../../actionConstants'; -import navReducer, { - getStateForRoute, - initialState as initialNavigationState, -} from '../navReducer'; - -describe('navReducer', () => { - const initialState = deepFreeze(initialNavigationState); - - describe('LOGIN_SUCCESS', () => { - test('replaces the existing route stack with "loading" on sign in', () => { - const prevState = deepFreeze({ - index: 2, - routes: [{ key: 'one' }, { key: 'two' }, { key: 'password' }], - }); - - const action = deepFreeze({ - type: LOGIN_SUCCESS, - }); - - const expectedState = { - index: 0, - routes: [{ routeName: 'loading' }], - }; - - const newState = navReducer(prevState, action); - - expect(newState.index).toEqual(expectedState.index); - expect(newState.routes[0].routeName).toEqual(expectedState.routes[0].routeName); - }); - }); - - describe('INITIAL_FETCH_COMPLETE', () => { - test('do not mutate navigation state if already at the same route', () => { - const prevState = getStateForRoute('main'); - - const action = deepFreeze({ - type: INITIAL_FETCH_COMPLETE, - }); - - const newState = navReducer(prevState, action); - - expect(newState).toBe(prevState); - }); - }); - - describe('REHYDRATE', () => { - test('when no previous navigation is given do not throw but return some result', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [{ apiKey: '123' }], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - }); - - test('if logged in and users is empty, go to loading', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [{ apiKey: '123' }], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('loading'); - }); - - test('if logged in and users is not empty, go to main', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [{ apiKey: '123' }], - users: [{ user_id: 123 }], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('main'); - }); - - test('if not logged in, and no previous accounts, show login screen', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('realm'); - }); - - test('if more than one account and no active account, display account list', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [{ apiKey: '' }, { apiKey: '' }], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('account'); - }); - - test('when only a single account and no other properties, redirect to login screen', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [{ apiKey: '', realm: new URL('https://example.com') }], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('realm'); - }); - - test('when multiple accounts and default one has realm and email, show account list', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [ - { apiKey: '', realm: new URL('https://example.com'), email: 'johndoe@example.com' }, - { apiKey: '', realm: new URL('https://example.com'), email: 'janedoe@example.com' }, - ], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('account'); - }); - - test('when default account has server and email set, redirect to login screen', () => { - const action = deepFreeze({ - type: REHYDRATE, - payload: { - accounts: [ - { apiKey: '', realm: new URL('https://example.com'), email: 'johndoe@example.com' }, - ], - users: [], - realm: {}, - }, - }); - - const nav = navReducer(initialState, action); - - expect(nav.routes).toHaveLength(1); - expect(nav.routes[0].routeName).toEqual('realm'); - }); - }); -}); diff --git a/src/nav/__tests__/navSelectors-test.js b/src/nav/__tests__/navSelectors-test.js index 47d70d80711..5bb98d93158 100644 --- a/src/nav/__tests__/navSelectors-test.js +++ b/src/nav/__tests__/navSelectors-test.js @@ -4,24 +4,25 @@ import { getCurrentRouteName, getCurrentRouteParams, getChatScreenParams, - getCanGoBack, getSameRoutesCount, } from '../navSelectors'; +import NavigationService from '../NavigationService'; describe('getCurrentRouteName', () => { test('return name of the current route', () => { - const state = deepFreeze({ - nav: { + NavigationService.getState = jest.fn().mockReturnValue( + deepFreeze({ index: 1, routes: [ { routeName: 'first', params: { email: 'a@a.com' } }, { routeName: 'second', params: { email: 'b@a.com' } }, ], - }, - }); + }), + ); + const expectedResult = 'second'; - const actualResult = getCurrentRouteName(state); + const actualResult = getCurrentRouteName(); expect(actualResult).toEqual(expectedResult); }); @@ -29,18 +30,18 @@ describe('getCurrentRouteName', () => { describe('getCurrentRouteParams', () => { test('return params of the current route', () => { - const state = deepFreeze({ - nav: { + NavigationService.getState = jest.fn().mockReturnValue( + deepFreeze({ index: 1, routes: [ { routeName: 'first', params: { email: 'a@a.com' } }, { routeName: 'second', params: { email: 'b@a.com' } }, ], - }, - }); + }), + ); const expectedResult = { email: 'b@a.com' }; - const actualResult = getCurrentRouteParams(state); + const actualResult = getCurrentRouteParams(); expect(actualResult).toEqual(expectedResult); }); @@ -48,67 +49,47 @@ describe('getCurrentRouteParams', () => { describe('getChatScreenParams', () => { test('when no params are passed do not return "undefined"', () => { - const state = deepFreeze({ - nav: { + NavigationService.getState = jest.fn().mockReturnValue( + deepFreeze({ index: 0, routes: [{ routeName: 'chat' }], - }, - }); + }), + ); - const actualResult = getChatScreenParams(state); + const actualResult = getChatScreenParams(); expect(actualResult).toBeDefined(); }); }); -describe('getCanGoBack', () => { - test('return true if current route in the stack is not the only route', () => { - const state = deepFreeze({ - nav: { - index: 1, - }, - }); - expect(getCanGoBack(state)).toBe(true); - }); - - test('return false if current route in the stack is the only route', () => { - const state = deepFreeze({ - nav: { - index: 0, - }, - }); - expect(getCanGoBack(state)).toBe(false); - }); -}); - describe('getSameRoutesCount', () => { test('if no routes the count of same routes is 0', () => { - const state = deepFreeze({ - nav: { + NavigationService.getState = jest.fn().mockReturnValue( + deepFreeze({ routes: [], - }, - }); + }), + ); - const count = getSameRoutesCount(state); + const count = getSameRoutesCount(); expect(count).toEqual(0); }); test('if last route differs from routes the count of same routes is 0', () => { - const state = deepFreeze({ - nav: { + NavigationService.getState = jest.fn().mockReturnValue( + deepFreeze({ routes: [{ routeName: 'main' }, { routeName: 'chat' }], - }, - }); + }), + ); - const count = getSameRoutesCount(state); + const count = getSameRoutesCount(); expect(count).toEqual(1); }); test('if several of the routes are the same ignore the params and return their count', () => { - const state = deepFreeze({ - nav: { + NavigationService.getState = jest.fn().mockReturnValue( + deepFreeze({ routes: [ { routeName: 'login' }, { routeName: 'main' }, @@ -116,10 +97,10 @@ describe('getSameRoutesCount', () => { { routeName: 'chat', params: { key: 'another value' } }, { routeName: 'chat', params: { anotherKey: 'some value' } }, ], - }, - }); + }), + ); - const count = getSameRoutesCount(state); + const count = getSameRoutesCount(); expect(count).toEqual(3); }); diff --git a/src/nav/navActions.js b/src/nav/navActions.js index 5e9234b7215..6f360cb4e79 100644 --- a/src/nav/navActions.js +++ b/src/nav/navActions.js @@ -1,10 +1,8 @@ /* @flow strict-local */ -import { StackActions } from 'react-navigation'; +import { StackActions, NavigationActions } from 'react-navigation'; import type { - Dispatch, NavigationAction, - GetState, Message, Narrow, UserOrBot, @@ -13,14 +11,37 @@ import type { } from '../types'; import { getSameRoutesCount } from '../selectors'; -export const navigateBack = () => (dispatch: Dispatch, getState: GetState): NavigationAction => - dispatch(StackActions.pop({ n: getSameRoutesCount(getState()) })); +export const navigateBack = (): NavigationAction => StackActions.pop({ n: getSameRoutesCount() }); -// Other stack routes reached through `navReducer`: -// StackActions.push({ routeName: 'loading' }); -// StackActions.push({ routeName: 'realm' }); -// StackActions.push({ routeName: 'account' }); -// StackActions.push({ routeName: 'main' }); +/* + * "Reset" actions, to explicitly prohibit back navigation. + */ + +export const resetToLoading = (): NavigationAction => + StackActions.reset({ index: 0, actions: [NavigationActions.navigate({ routeName: 'loading' })] }); + +export const resetToRealmScreen = ( + args: { realm?: URL, initial?: boolean } = {}, +): NavigationAction => + StackActions.reset({ + index: 0, + actions: [ + NavigationActions.navigate({ + routeName: 'realm', + params: { realm: args.realm, initial: args.initial }, + }), + ], + }); + +export const resetToAccountPicker = (): NavigationAction => + StackActions.reset({ index: 0, actions: [NavigationActions.navigate({ routeName: 'account' })] }); + +export const resetToMainTabs = (): NavigationAction => + StackActions.reset({ index: 0, actions: [NavigationActions.navigate({ routeName: 'main' })] }); + +/* + * Ordinary "push" actions that will push to the stack. + */ /** Only call this via `doNarrow`. See there for details. */ export const navigateToChat = (narrow: Narrow): NavigationAction => @@ -51,8 +72,10 @@ export const navigateToAccountDetails = (userId: number): NavigationAction => export const navigateToGroupDetails = (recipients: UserOrBot[]): NavigationAction => StackActions.push({ routeName: 'group-details', params: { recipients } }); -export const navigateToRealmScreen = (realm?: URL): NavigationAction => - StackActions.push({ routeName: 'realm', params: { realm } }); +export const navigateToRealmScreen = ( + args: { realm?: URL, initial?: boolean } = {}, +): NavigationAction => + StackActions.push({ routeName: 'realm', params: { realm: args.realm, initial: args.initial } }); export const navigateToLightbox = (src: string, message: Message): NavigationAction => StackActions.push({ routeName: 'lightbox', params: { src, message } }); diff --git a/src/nav/navReducer.js b/src/nav/navReducer.js index 69b94bd6129..07eee0d51b4 100644 --- a/src/nav/navReducer.js +++ b/src/nav/navReducer.js @@ -3,14 +3,6 @@ import type { NavigationAction } from 'react-navigation'; import type { NavigationState, Action } from '../types'; import AppNavigator from './AppNavigator'; -import { - REHYDRATE, - INITIAL_FETCH_COMPLETE, - ACCOUNT_SWITCH, - LOGIN_SUCCESS, - LOGOUT, -} from '../actionConstants'; -import { hasAuth } from '../account/accountsSelectors'; /** * Get the initial state for the given route. @@ -18,17 +10,7 @@ import { hasAuth } from '../account/accountsSelectors'; * Private; exported only for tests. */ export const getStateForRoute = (route: string): NavigationState => { - // TODO: this is kind of a hack! Refactor to a better way. - // * Perhaps pass `initial: true` unconditionally in this initialization - // code, to all routes? Then that'd just be part of the interface of - // making a screen work as an initial screen. - // * Alternatively, we could replace the whole system of `ModalNavBar`, - // `canGoBack`, etc., with our own "navigation view" that would be more - // directly integrated into the navigation framework: - // https://reactnavigation.org/docs/en/2.x/navigation-views.html - const params = route === 'realm' ? { initial: true } : undefined; - - const action = AppNavigator.router.getActionForPathAndParams(route, params); + const action = AppNavigator.router.getActionForPathAndParams(route); if (!action) { // The argument should be a constant string that is a genuine nav route; // so this condition can only happen if we've gotten that wrong. @@ -41,53 +23,10 @@ export const getStateForRoute = (route: string): NavigationState => { return state; }; -const rehydrate = (state, action) => { - // If there's no data to rehydrate, or no account data, show login screen. - if (!action.payload || !action.payload.accounts) { - return getStateForRoute('realm'); - } - - // If there are accounts but the active account is not logged in, - // show account screen. - const rehydratedState = action.payload; - if (!hasAuth(rehydratedState)) { - const { accounts } = rehydratedState; - return getStateForRoute(accounts && accounts.length > 1 ? 'account' : 'realm'); - } - - // If there's an active, logged-in account but no server data, then behave - // like `ACCOUNT_SWITCH`: show loading screen. Crucially, `sessionReducer` - // will have set `needsInitialFetch`, too, so we really will be loading. - // - // (Valid server data must have a user: the self user, at a minimum. - // Compare getHaveServerData; this has an extra check because `users` - // may be missing entirely.) - if (rehydratedState.users === undefined || rehydratedState.users.length === 0) { - return getStateForRoute('loading'); - } - - // Great: we have an active, logged-in account, and server data for it. - // Show the main UI. - return getStateForRoute('main'); -}; - export const initialState = getStateForRoute('loading'); export default (state: NavigationState = initialState, action: Action): NavigationState => { switch (action.type) { - case REHYDRATE: - return rehydrate(state, action); - - case ACCOUNT_SWITCH: - case LOGIN_SUCCESS: - return getStateForRoute('loading'); - - case INITIAL_FETCH_COMPLETE: - return state.routes[0].routeName === 'main' ? state : getStateForRoute('main'); - - case LOGOUT: - return getStateForRoute('account'); - default: { // The `react-navigation` libdef says this only takes a NavigationAction, // but docs say pass arbitrary action. $FlowFixMe diff --git a/src/nav/navSelectors.js b/src/nav/navSelectors.js index c4d5f5d2ae1..dce36f6bc65 100644 --- a/src/nav/navSelectors.js +++ b/src/nav/navSelectors.js @@ -1,35 +1,30 @@ /* @flow strict-local */ -import type { GlobalState, NavigationRouteState, NavigationState } from '../types'; -import type { Narrow } from '../api/apiTypes'; +import type { NavigationState, NavigationRoute } from 'react-navigation'; -export const getNav = (state: GlobalState): NavigationState => state.nav; +import NavigationService from './NavigationService'; -const getNavigationRoutes = (state: GlobalState): NavigationRouteState[] => state.nav.routes; +export const getNavState = (): NavigationState => NavigationService.getState(); -const getNavigationIndex = (state: GlobalState): number => getNav(state).index; +export const getNavigationRoutes = () => getNavState().routes; -const getCurrentRoute = (state: GlobalState): void | NavigationRouteState => - getNavigationRoutes(state)[getNavigationIndex(state)]; +const getNavigationIndex = () => getNavState().index; -export const getCurrentRouteName = (state: GlobalState): void | string => - getCurrentRoute(state)?.routeName; +const getCurrentRoute = (): void | NavigationRoute => getNavigationRoutes()[getNavigationIndex()]; -export const getCurrentRouteParams = (state: GlobalState): void | { narrow?: Narrow } => - getCurrentRoute(state)?.params; +export const getCurrentRouteName = () => getCurrentRoute()?.routeName; -export const getChatScreenParams = (state: GlobalState): { narrow?: Narrow } => - getCurrentRouteParams(state) ?? { narrow: undefined }; +export const getCurrentRouteParams = () => getCurrentRoute()?.params; -export const getCanGoBack = (state: GlobalState) => state.nav.index > 0; +export const getChatScreenParams = () => getCurrentRouteParams() ?? { narrow: undefined }; -export const getSameRoutesCount = (state: GlobalState): number => { - const nav = getNav(state); - let i = nav.routes.length - 1; +export const getSameRoutesCount = () => { + const routes = getNavigationRoutes(); + let i = routes.length - 1; while (i >= 0) { - if (nav.routes[i].routeName !== nav.routes[nav.routes.length - 1].routeName) { + if (routes[i].routeName !== routes[routes.length - 1].routeName) { break; } i--; } - return nav.routes.length - i - 1; + return routes.length - i - 1; }; diff --git a/src/session/sessionReducer.js b/src/session/sessionReducer.js index f8ae8cefaf4..32a735211ae 100644 --- a/src/session/sessionReducer.js +++ b/src/session/sessionReducer.js @@ -91,7 +91,8 @@ const rehydrate = (state, action) => { // On rehydration, do an initial fetch if we have access to an account // (indicated by the presence of an api key). Otherwise, the initial fetch // will be initiated on loginSuccess. - // NB navReducer's rehydrate logic depends intimately on this behavior. + // NB `InitialNavigationDispatcher`'s `doInitialNavigation` + // depends intimately on this behavior. needsInitialFetch: haveApiKey, }; }; diff --git a/src/start/RealmScreen.js b/src/start/RealmScreen.js index 7b5fe3ab157..254f0eafe3e 100644 --- a/src/start/RealmScreen.js +++ b/src/start/RealmScreen.js @@ -12,7 +12,7 @@ import * as api from '../api'; import { realmAdd, navigateToAuth } from '../actions'; type SelectorProps = {| - +initialRealm: string, + +initialRealmInputValue: string, |}; type Props = $ReadOnly<{| @@ -24,9 +24,6 @@ type Props = $ReadOnly<{| ...NavigationStateRoute, params?: {| realm: URL | void, - // Currently passed as `true` in a hack in `navReducer.js`; see - // https://github.com/zulip/zulip-mobile/pull/4273#discussion_r499114689. - // TODO: Stop using that hack. initial?: boolean, |}, |}>, @@ -44,7 +41,7 @@ type State = {| class RealmScreen extends PureComponent { state = { progress: false, - realmInputValue: this.props.initialRealm, + realmInputValue: this.props.initialRealmInputValue, error: null, }; @@ -92,14 +89,14 @@ class RealmScreen extends PureComponent { handleRealmChange = (value: string) => this.setState({ realmInputValue: value }); componentDidMount() { - const { initialRealm } = this.props; - if (initialRealm && initialRealm.length > 0) { + const { initialRealmInputValue } = this.props; + if (initialRealmInputValue && initialRealmInputValue.length > 0) { this.tryRealm(); } } render() { - const { initialRealm, navigation } = this.props; + const { initialRealmInputValue, navigation } = this.props; const { progress, error, realmInputValue } = this.state; const styles = { @@ -124,7 +121,7 @@ class RealmScreen extends PureComponent { defaultProtocol="https://" defaultOrganization="your-org" defaultDomain="zulipchat.com" - defaultValue={initialRealm} + defaultValue={initialRealmInputValue} onChangeText={this.handleRealmChange} onSubmitEditing={this.tryRealm} enablesReturnKeyAutomatically @@ -147,5 +144,5 @@ class RealmScreen extends PureComponent { } export default connect((state, props) => ({ - initialRealm: props.navigation.state.params?.realm?.toString() ?? '', + initialRealmInputValue: props.navigation.state.params?.realm?.toString() ?? '', }))(RealmScreen); diff --git a/src/title/__tests__/titleSelectors-test.js b/src/title/__tests__/titleSelectors-test.js index 7b964df99bd..da3eaca83ed 100644 --- a/src/title/__tests__/titleSelectors-test.js +++ b/src/title/__tests__/titleSelectors-test.js @@ -2,14 +2,12 @@ import deepFreeze from 'deep-freeze'; import { DEFAULT_TITLE_BACKGROUND_COLOR, getTitleBackgroundColor } from '../titleSelectors'; import { groupNarrow, streamNarrow, privateNarrow } from '../../utils/narrow'; -import { defaultNav, otherNav } from '../../utils/testHelpers'; const subscriptions = [{ name: 'all', color: '#fff' }, { name: 'announce', color: '#000' }]; describe('getTitleBackgroundColor', () => { test('return default for screens other than chat, i.e narrow is undefined', () => { const state = deepFreeze({ - nav: otherNav, subscriptions, }); @@ -18,7 +16,6 @@ describe('getTitleBackgroundColor', () => { test('return stream color for stream and topic narrow', () => { const state = deepFreeze({ - nav: defaultNav, subscriptions, }); @@ -27,7 +24,6 @@ describe('getTitleBackgroundColor', () => { test('return null stream color for invalid stream or unknown subscriptions', () => { const state = deepFreeze({ - nav: defaultNav, subscriptions, }); @@ -36,7 +32,6 @@ describe('getTitleBackgroundColor', () => { test('return default for non topic/stream narrow', () => { const state = deepFreeze({ - nav: defaultNav, subscriptions, }); diff --git a/src/utils/testHelpers.js b/src/utils/testHelpers.js deleted file mode 100644 index e913b89283a..00000000000 --- a/src/utils/testHelpers.js +++ /dev/null @@ -1,35 +0,0 @@ -/* @flow strict-local */ -import type { Narrow } from '../types'; -import type { NavigationState } from '../reduxTypes'; - -export const navStateWithNarrow = (narrow: Narrow): {| nav: NavigationState |} => ({ - nav: { - key: 'StackRouterRoot', - index: 1, - isTransitioning: false, - routes: [ - { - key: 'id-1592948746166-1', - params: {}, - routeName: 'main', - }, - { - key: 'id-1592948746166-2', - routeName: 'chat', - params: { - narrow, - }, - }, - ], - }, -}); - -export const defaultNav = { - index: 0, - routes: [{ routeName: 'chat' }], -}; - -export const otherNav = { - index: 1, - routes: [{ routeName: 'main' }, { routeName: 'account' }], -};