Skip to content

Commit

Permalink
nav: Switch from NavigationService to props.navigation
Browse files Browse the repository at this point in the history
React Navigation upstream has since the 5.x release been
discouraging the use of the strategy we call NavigationService.
See discussion at 140c28c and:
  zulip#4417
Instead, the recommendation is to use the `navigation` prop where
available, and failing that the `useNavigation` hook.

In this commit, we make that change for all sites where we were
using NavigationService in a function component that is a screen
in a navigator, and gets its own navigation prop.
  • Loading branch information
gnprice committed Jun 10, 2022
1 parent 5144d33 commit 59bdb7c
Show file tree
Hide file tree
Showing 10 changed files with 36 additions and 33 deletions.
11 changes: 7 additions & 4 deletions src/chat/PmConversationDetailsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { FlatList } from 'react-native';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import { useSelector } from '../react-redux';
import type { UserOrBot } from '../types';
import { pmUiRecipientsFromKeyRecipients, type PmKeyRecipients } from '../utils/recipient';
Expand All @@ -20,12 +19,16 @@ type Props = $ReadOnly<{|
|}>;

export default function PmConversationDetailsScreen(props: Props): Node {
const { navigation } = props;
const { recipients } = props.route.params;
const ownUserId = useSelector(getOwnUserId);

const handlePress = useCallback((user: UserOrBot) => {
NavigationService.dispatch(navigateToAccountDetails(user.user_id));
}, []);
const handlePress = useCallback(
(user: UserOrBot) => {
navigation.dispatch(navigateToAccountDetails(user.user_id));
},
[navigation],
);

return (
<Screen title="Recipients" scrollEnabled={false}>
Expand Down
4 changes: 2 additions & 2 deletions src/main/HomeScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { View } from 'react-native';

import type { RouteProp } from '../react-navigation';
import type { MainTabsNavigationProp } from './MainTabsScreen';
import * as NavigationService from '../nav/NavigationService';
import { useDispatch } from '../react-redux';
import { HOME_NARROW, MENTIONED_NARROW, STARRED_NARROW } from '../utils/narrow';
import { TopTabButton, TopTabButtonGeneral } from '../nav/TopTabButton';
Expand Down Expand Up @@ -35,6 +34,7 @@ type Props = $ReadOnly<{|
|}>;

export default function HomeScreen(props: Props): Node {
const { navigation } = props;
const dispatch = useDispatch();

return (
Expand Down Expand Up @@ -62,7 +62,7 @@ export default function HomeScreen(props: Props): Node {
<TopTabButton
name="search"
onPress={() => {
NavigationService.dispatch(navigateToSearch());
navigation.dispatch(navigateToSearch());
}}
/>
</View>
Expand Down
6 changes: 3 additions & 3 deletions src/pm-conversations/PmConversationsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { View } from 'react-native';

import type { RouteProp } from '../react-navigation';
import type { MainTabsNavigationProp } from '../main/MainTabsScreen';
import * as NavigationService from '../nav/NavigationService';
import { ThemeContext, createStyleSheet } from '../styles';
import { useSelector } from '../react-redux';
import ZulipTextIntl from '../common/ZulipTextIntl';
Expand Down Expand Up @@ -45,6 +44,7 @@ type Props = $ReadOnly<{|
* The "PMs" page in the main tabs navigation.
* */
export default function PmConversationsScreen(props: Props): Node {
const { navigation } = props;
const conversations = useSelector(getRecentConversations);
const context = useContext(ThemeContext);

Expand All @@ -57,7 +57,7 @@ export default function PmConversationsScreen(props: Props): Node {
style={styles.button}
text="New PM"
onPress={() => {
setTimeout(() => NavigationService.dispatch(navigateToUsersScreen()));
setTimeout(() => navigation.dispatch(navigateToUsersScreen()));
}}
/>
<ZulipButton
Expand All @@ -66,7 +66,7 @@ export default function PmConversationsScreen(props: Props): Node {
style={styles.button}
text="New group PM"
onPress={() => {
setTimeout(() => NavigationService.dispatch(navigateToCreateGroup()));
setTimeout(() => navigation.dispatch(navigateToCreateGroup()));
}}
/>
</View>
Expand Down
6 changes: 3 additions & 3 deletions src/reactions/MessageReactionsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { createMaterialTopTabNavigator } from '@react-navigation/material-top-ta

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import * as logging from '../utils/logging';
import ReactionUserList from './ReactionUserList';
import { useSelector } from '../react-redux';
Expand Down Expand Up @@ -40,6 +39,7 @@ type Props = $ReadOnly<{|
* screen first appears.
*/
export default function MessageReactionsScreen(props: Props): Node {
const { navigation } = props;
const { messageId, reactionName } = props.route.params;
const message = useSelector(state => state.messages.get(messageId));
const ownUserId = useSelector(getOwnUserId);
Expand All @@ -59,9 +59,9 @@ export default function MessageReactionsScreen(props: Props): Node {
if (prevMessage !== undefined && message === undefined) {
// The message was present, but got purged (currently only caused by a
// REGISTER_COMPLETE following a dead event queue), so go back.
NavigationService.dispatch(navigateBack());
navigation.dispatch(navigateBack());
}
}, [prevMessage, message]);
}, [prevMessage, message, navigation]);

const content: Node = (() => {
if (message === undefined) {
Expand Down
10 changes: 5 additions & 5 deletions src/settings/SettingsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import type { Node } from 'react';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import { useGlobalSelector, useDispatch } from '../react-redux';
import { getGlobalSettings } from '../selectors';
import NestedNavRow from '../common/NestedNavRow';
Expand Down Expand Up @@ -38,6 +37,7 @@ export default function SettingsScreen(props: Props): Node {
state => getGlobalSettings(state).doNotMarkMessagesAsRead,
);
const dispatch = useDispatch();
const { navigation } = props;

const handleThemeChange = useCallback(() => {
dispatch(setGlobalSettings({ theme: theme === 'default' ? 'night' : 'default' }));
Expand All @@ -64,28 +64,28 @@ export default function SettingsScreen(props: Props): Node {
Icon={IconNotifications}
label="Notifications"
onPress={() => {
NavigationService.dispatch(navigateToNotifications());
navigation.dispatch(navigateToNotifications());
}}
/>
<NestedNavRow
Icon={IconLanguage}
label="Language"
onPress={() => {
NavigationService.dispatch(navigateToLanguage());
navigation.dispatch(navigateToLanguage());
}}
/>
<NestedNavRow
Icon={IconDiagnostics}
label="Diagnostics"
onPress={() => {
NavigationService.dispatch(navigateToDiagnostics());
navigation.dispatch(navigateToDiagnostics());
}}
/>
<NestedNavRow
Icon={IconMoreHorizontal}
label="Legal"
onPress={() => {
NavigationService.dispatch(navigateToLegal());
navigation.dispatch(navigateToLegal());
}}
/>
</Screen>
Expand Down
6 changes: 3 additions & 3 deletions src/sharing/SharingScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {

import type { RouteParamsOf, RouteProp } from '../react-navigation';
import type { AppNavigationMethods, AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import type { SharedData } from './types';
import { createStyleSheet } from '../styles';
import { materialTopTabNavigatorConfig } from '../styles/tabs';
Expand Down Expand Up @@ -51,16 +50,17 @@ const styles = createStyleSheet({

export default function SharingScreen(props: Props): Node {
const { params } = props.route;
const { navigation } = props;
const hasAuth = useGlobalSelector(getHasAuth);

useEffect(() => {
if (!hasAuth) {
// If there is no active logged-in account, abandon the sharing attempt,
// and present the account picker screen to the user.
// TODO(?): Offer to come back and finish the share after auth
NavigationService.dispatch(resetToAccountPicker());
navigation.dispatch(resetToAccountPicker());
}
}, [hasAuth]);
}, [hasAuth, navigation]);

return (
<Screen canGoBack={false} title="Share on Zulip" shouldShowLoadingBanner={false}>
Expand Down
6 changes: 3 additions & 3 deletions src/streams/InviteUsersScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { Node } from 'react';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import type { UserOrBot } from '../types';
import { useSelector } from '../react-redux';
import Screen from '../common/Screen';
Expand All @@ -19,6 +18,7 @@ type Props = $ReadOnly<{|
|}>;

export default function InviteUsersScreen(props: Props): Node {
const { navigation } = props;
const auth = useSelector(getAuth);
const stream = useSelector(state => getStreamForId(state, props.route.params.streamId));

Expand All @@ -29,9 +29,9 @@ export default function InviteUsersScreen(props: Props): Node {
const recipients = selected.map(user => user.email);
// This still uses a stream name (#3918) because the API method does; see there.
api.subscriptionAdd(auth, [{ name: stream.name }], recipients);
NavigationService.dispatch(navigateBack());
navigation.dispatch(navigateBack());
},
[auth, stream],
[auth, navigation, stream.name],
);

return (
Expand Down
10 changes: 5 additions & 5 deletions src/streams/StreamSettingsScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import { View } from 'react-native';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import { useSelector } from '../react-redux';
import { delay } from '../utils/async';
import SwitchRow from '../common/SwitchRow';
Expand All @@ -29,6 +28,7 @@ type Props = $ReadOnly<{|
|}>;

export default function StreamSettingsScreen(props: Props): Node {
const { navigation } = props;
const auth = useSelector(getAuth);
const isAtLeastAdmin = useSelector(state => roleIsAtLeast(getOwnUserRole(state), Role.Admin));
const stream = useSelector(state => getStreamForId(state, props.route.params.streamId));
Expand All @@ -52,12 +52,12 @@ export default function StreamSettingsScreen(props: Props): Node {
);

const handlePressEdit = useCallback(() => {
NavigationService.dispatch(navigateToEditStream(stream.stream_id));
}, [stream]);
navigation.dispatch(navigateToEditStream(stream.stream_id));
}, [navigation, stream.stream_id]);

const handlePressEditSubscribers = useCallback(() => {
NavigationService.dispatch(navigateToStreamSubscribers(stream.stream_id));
}, [stream]);
navigation.dispatch(navigateToStreamSubscribers(stream.stream_id));
}, [navigation, stream.stream_id]);

const handlePressSubscribe = useCallback(() => {
// This still uses a stream name (#3918) because the API method does; see there.
Expand Down
4 changes: 2 additions & 2 deletions src/subscriptions/StreamListCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import { View, FlatList } from 'react-native';

import type { RouteProp } from '../react-navigation';
import type { StreamTabsNavigationProp } from '../main/StreamTabsScreen';
import * as NavigationService from '../nav/NavigationService';
import { createStyleSheet } from '../styles';
import { useDispatch, useSelector } from '../react-redux';
import ZulipButton from '../common/ZulipButton';
Expand Down Expand Up @@ -40,6 +39,7 @@ type Props = $ReadOnly<{|
|}>;

export default function StreamListCard(props: Props): Node {
const { navigation } = props;
const dispatch = useDispatch();
const auth = useSelector(getAuth);
const canCreateStreams = useSelector(getCanCreateStreams);
Expand Down Expand Up @@ -79,7 +79,7 @@ export default function StreamListCard(props: Props): Node {
text="Create new stream"
onPress={() =>
delay(() => {
NavigationService.dispatch(navigateToCreateStream());
navigation.dispatch(navigateToCreateStream());
})
}
/>
Expand Down
6 changes: 3 additions & 3 deletions src/user-groups/CreateGroupScreen.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import type { Node } from 'react';

import type { RouteProp } from '../react-navigation';
import type { AppNavigationProp } from '../nav/AppNavigator';
import * as NavigationService from '../nav/NavigationService';
import type { UserOrBot } from '../types';
import { useSelector, useDispatch } from '../react-redux';
import Screen from '../common/Screen';
Expand All @@ -20,17 +19,18 @@ type Props = $ReadOnly<{|
|}>;

export default function CreateGroupScreen(props: Props): Node {
const { navigation } = props;
const dispatch = useDispatch();
const ownUserId = useSelector(getOwnUserId);

const [filter, setFilter] = useState<string>('');

const handleCreateGroup = useCallback(
(selected: $ReadOnlyArray<UserOrBot>) => {
NavigationService.dispatch(navigateBack());
navigation.dispatch(navigateBack());
dispatch(doNarrow(pmNarrowFromRecipients(pmKeyRecipientsFromUsers(selected, ownUserId))));
},
[dispatch, ownUserId],
[dispatch, navigation, ownUserId],
);

return (
Expand Down

0 comments on commit 59bdb7c

Please sign in to comment.