Skip to content

Commit 2a0364d

Browse files
committed
action-sheets: Plumb navigation down to button args (PmArgs, etc.)
This way, we can stop using NavigationService in these action sheets, and instead use `navigation.push` calls. React Navigation upstream recommends avoiding the NavigationService approach where possible, and this goes in that direction; see more in zulip#5408.
1 parent 4838382 commit 2a0364d

File tree

9 files changed

+46
-12
lines changed

9 files changed

+46
-12
lines changed

src/action-sheets/index.js

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ import { getStreamTopicUrl, getStreamUrl } from '../utils/internalLinks';
5353
import { reactionTypeFromEmojiType } from '../emoji/data';
5454
import { Role, type RoleT } from '../api/permissionsTypes';
5555
import { roleIsAtLeast } from '../permissionSelectors';
56+
import type { AppNavigationMethods } from '../nav/AppNavigator';
5657

5758
// TODO really this belongs in a libdef.
5859
export type ShowActionSheetWithOptions = (
@@ -66,6 +67,7 @@ type StreamArgs = {
6667
subscriptions: Map<number, Subscription>,
6768
streams: Map<number, Stream>,
6869
dispatch: Dispatch,
70+
navigation: AppNavigationMethods,
6971
_: GetText,
7072
...
7173
};
@@ -78,12 +80,14 @@ type TopicArgs = {
7880
streams: Map<number, Stream>,
7981
zulipFeatureLevel: number,
8082
dispatch: Dispatch,
83+
navigation: AppNavigationMethods,
8184
_: GetText,
8285
...
8386
};
8487

8588
type PmArgs = {
8689
pmKeyRecipients: PmKeyRecipients,
90+
navigation: AppNavigationMethods,
8791
_: GetText,
8892
...
8993
};
@@ -93,8 +97,9 @@ type MessageArgs = {
9397
ownUser: User,
9498
message: Message | Outbox,
9599
dispatch: Dispatch,
96-
_: GetText,
97100
startEditMessage: (editMessage: EditMessage) => void,
101+
navigation: AppNavigationMethods,
102+
_: GetText,
98103
...
99104
};
100105

@@ -661,6 +666,7 @@ export const showMessageActionSheet = (args: {|
661666
callbacks: {|
662667
dispatch: Dispatch,
663668
startEditMessage: (editMessage: EditMessage) => void,
669+
navigation: AppNavigationMethods,
664670
_: GetText,
665671
|},
666672
backgroundData: $ReadOnly<{
@@ -685,6 +691,7 @@ export const showTopicActionSheet = (args: {|
685691
showActionSheetWithOptions: ShowActionSheetWithOptions,
686692
callbacks: {|
687693
dispatch: Dispatch,
694+
navigation: AppNavigationMethods,
688695
_: GetText,
689696
|},
690697
backgroundData: $ReadOnly<{
@@ -716,6 +723,7 @@ export const showStreamActionSheet = (args: {|
716723
showActionSheetWithOptions: ShowActionSheetWithOptions,
717724
callbacks: {|
718725
dispatch: Dispatch,
726+
navigation: AppNavigationMethods,
719727
_: GetText,
720728
|},
721729
backgroundData: $ReadOnly<{
@@ -741,6 +749,7 @@ export const showStreamActionSheet = (args: {|
741749
export const showPmConversationActionSheet = (args: {|
742750
showActionSheetWithOptions: ShowActionSheetWithOptions,
743751
callbacks: {|
752+
navigation: AppNavigationMethods,
744753
_: GetText,
745754
|},
746755
backgroundData: $ReadOnly<{ ownUser: User, allUsersById: Map<UserId, UserOrBot>, ... }>,

src/chat/ChatScreen.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ export default function ChatScreen(props: Props): Node {
221221
}
222222
showMessagePlaceholders={showMessagePlaceholders}
223223
startEditMessage={setEditMessage}
224+
navigation={navigation}
224225
/>
225226
);
226227
}

src/search/SearchMessagesCard.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { createStyleSheet } from '../styles';
99
import LoadingIndicator from '../common/LoadingIndicator';
1010
import SearchEmptyState from '../common/SearchEmptyState';
1111
import MessageList from '../webview/MessageList';
12+
import { useNavigation } from '../react-navigation';
1213

1314
const styles = createStyleSheet({
1415
results: {
@@ -24,6 +25,7 @@ type Props = $ReadOnly<{|
2425

2526
export default function SearchMessagesCard(props: Props): Node {
2627
const { isFetching, messages } = props;
28+
const navigation = useNavigation();
2729

2830
if (isFetching) {
2931
// Display loading indicator only if there are no messages to
@@ -55,6 +57,7 @@ export default function SearchMessagesCard(props: Props): Node {
5557
// TODO: handle editing a message from the search results,
5658
// or make this prop optional
5759
startEditMessage={() => undefined}
60+
navigation={navigation}
5861
/>
5962
</View>
6063
);

src/streams/StreamItem.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import UnreadCount from '../common/UnreadCount';
3333
import { foregroundColorFromBackground } from '../utils/color';
3434
import { IconPlus, IconDone } from '../common/Icons';
3535
import StreamIcon from './StreamIcon';
36+
import { useNavigation } from '../react-navigation';
3637

3738
const componentStyles = createStyleSheet({
3839
description: {
@@ -114,6 +115,7 @@ export default function StreamItem(props: Props): Node {
114115
const showActionSheetWithOptions: ShowActionSheetWithOptions =
115116
useActionSheet().showActionSheetWithOptions;
116117
const _ = useContext(TranslationContext);
118+
const navigation = useNavigation();
117119
const dispatch = useDispatch();
118120
const backgroundData = useSelector(state => ({
119121
auth: getAuth(state),
@@ -144,7 +146,7 @@ export default function StreamItem(props: Props): Node {
144146
onLongPress={() => {
145147
showStreamActionSheet({
146148
showActionSheetWithOptions,
147-
callbacks: { dispatch, _ },
149+
callbacks: { dispatch, navigation, _ },
148150
backgroundData,
149151
streamId,
150152
});

src/streams/TopicItem.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
import { getMute } from '../mute/muteModel';
2626
import { getUnread } from '../unread/unreadModel';
2727
import { getOwnUserRole } from '../permissionSelectors';
28+
import { useNavigation } from '../react-navigation';
2829

2930
const componentStyles = createStyleSheet({
3031
selectedRow: {
@@ -69,6 +70,7 @@ export default function TopicItem(props: Props): Node {
6970
const showActionSheetWithOptions: ShowActionSheetWithOptions =
7071
useActionSheet().showActionSheetWithOptions;
7172
const _ = useContext(TranslationContext);
73+
const navigation = useNavigation();
7274
const dispatch = useDispatch();
7375
const backgroundData = useSelector(state => ({
7476
auth: getAuth(state),
@@ -88,7 +90,7 @@ export default function TopicItem(props: Props): Node {
8890
onLongPress={() => {
8991
showTopicActionSheet({
9092
showActionSheetWithOptions,
91-
callbacks: { dispatch, _ },
93+
callbacks: { dispatch, navigation, _ },
9294
backgroundData,
9395
streamId,
9496
topic: name,

src/title/TitleStream.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { showStreamActionSheet, showTopicActionSheet } from '../action-sheets';
2727
import type { ShowActionSheetWithOptions } from '../action-sheets';
2828
import { getUnread } from '../unread/unreadModel';
2929
import { getOwnUserRole } from '../permissionSelectors';
30+
import { useNavigation } from '../react-navigation';
3031

3132
type Props = $ReadOnly<{|
3233
narrow: Narrow,
@@ -50,6 +51,7 @@ const componentStyles = createStyleSheet({
5051
export default function TitleStream(props: Props): Node {
5152
const { narrow, color } = props;
5253
const dispatch = useDispatch();
54+
const navigation = useNavigation();
5355
const stream = useSelector(state => getStreamInNarrow(state, narrow));
5456
const backgroundData = useSelector(state => ({
5557
auth: getAuth(state),
@@ -75,7 +77,7 @@ export default function TitleStream(props: Props): Node {
7577
? () => {
7678
showTopicActionSheet({
7779
showActionSheetWithOptions,
78-
callbacks: { dispatch, _ },
80+
callbacks: { dispatch, navigation, _ },
7981
backgroundData,
8082
streamId: stream.stream_id,
8183
topic: topicOfNarrow(narrow),
@@ -84,7 +86,7 @@ export default function TitleStream(props: Props): Node {
8486
: () => {
8587
showStreamActionSheet({
8688
showActionSheetWithOptions,
87-
callbacks: { dispatch, _ },
89+
callbacks: { dispatch, navigation, _ },
8890
backgroundData,
8991
streamId: stream.stream_id,
9092
});

src/webview/MessageList.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,13 +38,17 @@ import { tryParseUrl } from '../utils/url';
3838
import { caseNarrow, isConversationNarrow } from '../utils/narrow';
3939
import { type BackgroundData, getBackgroundData } from './backgroundData';
4040
import { ensureUnreachable } from '../generics';
41+
import type { AppNavigationMethods } from '../nav/AppNavigator';
4142

4243
type OuterProps = $ReadOnly<{|
4344
narrow: Narrow,
4445
messages: $ReadOnlyArray<Message | Outbox>,
4546
initialScrollMessageId: number | null,
4647
showMessagePlaceholders: boolean,
4748
startEditMessage: (editMessage: EditMessage) => void,
49+
50+
// TODO: Could instead use useNavigation once this is a function component
51+
navigation: AppNavigationMethods,
4852
|}>;
4953

5054
type SelectorProps = {|
@@ -128,7 +132,7 @@ class MessageListInner extends Component<Props> {
128132
this.unsentInboundEvents = [];
129133
} else {
130134
const { _ } = this.props;
131-
handleWebViewOutboundEvent(this.props, _, eventData);
135+
handleWebViewOutboundEvent(this.props, this.props.navigation, _, eventData);
132136
}
133137
};
134138

src/webview/__tests__/generateInboundEvents-test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ describe('generateInboundEvents', () => {
3333
...baseSelectorProps,
3434
showActionSheetWithOptions: jest.fn(),
3535

36+
// $FlowFixMe[incompatible-type] - A stub; fill in where needed
37+
// $FlowFixMe[prop-missing]
38+
navigation: {
39+
push: jest.fn(),
40+
// etc., incomplete
41+
},
3642
_: jest.fn(),
3743
});
3844

src/webview/handleOutboundEvents.js

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
} from '../action-sheets';
3131
import { ensureUnreachable } from '../types';
3232
import { base64Utf8Decode } from '../utils/encoding';
33+
import type { AppNavigationMethods } from '../nav/AppNavigator';
3334

3435
type WebViewOutboundEventReady = {|
3536
type: 'ready',
@@ -206,12 +207,14 @@ const handleImage = (props: Props, src: string, messageId: number) => {
206207

207208
const handleLongPress = (args: {|
208209
props: Props,
209-
_: GetText,
210210
target: 'message' | 'header' | 'link',
211211
messageId: number,
212212
href: string | null,
213+
214+
navigation: AppNavigationMethods,
215+
_: GetText,
213216
|}) => {
214-
const { props, _, target, messageId, href } = args;
217+
const { props, target, messageId, href, navigation, _ } = args;
215218

216219
if (href !== null) {
217220
const url = new URL(href, props.backgroundData.auth.realm).toString();
@@ -229,23 +232,23 @@ const handleLongPress = (args: {|
229232
if (message.type === 'stream') {
230233
showTopicActionSheet({
231234
showActionSheetWithOptions,
232-
callbacks: { dispatch, _ },
235+
callbacks: { dispatch, navigation, _ },
233236
backgroundData,
234237
streamId: message.stream_id,
235238
topic: message.subject,
236239
});
237240
} else if (message.type === 'private') {
238241
showPmConversationActionSheet({
239242
showActionSheetWithOptions,
240-
callbacks: { _ },
243+
callbacks: { navigation, _ },
241244
backgroundData,
242245
pmKeyRecipients: pmKeyRecipientsFromMessage(message, backgroundData.ownUser.user_id),
243246
});
244247
}
245248
} else if (target === 'message') {
246249
showMessageActionSheet({
247250
showActionSheetWithOptions,
248-
callbacks: { dispatch, startEditMessage, _ },
251+
callbacks: { dispatch, startEditMessage, navigation, _ },
249252
backgroundData,
250253
message,
251254
narrow,
@@ -255,6 +258,7 @@ const handleLongPress = (args: {|
255258

256259
export const handleWebViewOutboundEvent = (
257260
props: Props,
261+
navigation: AppNavigationMethods,
258262
_: GetText,
259263
event: WebViewOutboundEvent,
260264
) => {
@@ -284,10 +288,11 @@ export const handleWebViewOutboundEvent = (
284288
case 'longPress':
285289
handleLongPress({
286290
props,
287-
_,
288291
target: event.target,
289292
messageId: event.messageId,
290293
href: event.href,
294+
navigation,
295+
_,
291296
});
292297
break;
293298

0 commit comments

Comments
 (0)