Skip to content

Commit 963c978

Browse files
committed
ActionSheets: Have constructHeaderActionButtons stop taking message arg
This changes the constructHeaderActionButtons function to take a stream and topic, instead of a message, since that is conceptually closer to what it's doing. Since the webview code previously called this path even for PMs, which don't have a "topic", I've changed the webview code show a toast when the user long-presses on a PM header. This way, they'll still be able to see the list of all recipients, but we'll avoid the somewhat silly action sheet with only a "Cancel" action. I haven't routed this through to showHeaderActionButton yet, since all of the button callbacks currently still require a Message, but that's the goal in the future. I've manually tested long-pressing a PM header, which correctly shows a toast displaying the users in the PM.
1 parent 99efbd4 commit 963c978

File tree

3 files changed

+50
-50
lines changed

3 files changed

+50
-50
lines changed

src/message/__tests__/messageActionSheet-test.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// @flow strict-local
22
import deepFreeze from 'deep-freeze';
33
import { HOME_NARROW } from '../../utils/narrow';
4+
import { streamNameOfStreamMessage } from '../../utils/recipient';
45

56
import * as eg from '../../__tests__/lib/exampleData';
67
import { constructMessageActionButtons, constructHeaderActionButtons } from '../messageActionSheet';
@@ -48,15 +49,12 @@ describe('constructActionButtons', () => {
4849
describe('constructHeaderActionButtons', () => {
4950
test('show Unmute topic option if topic is muted', () => {
5051
const mute = deepFreeze([['electron issues', 'issue #556']]);
51-
const message = eg.streamMessage({
52-
display_recipient: 'electron issues',
53-
subject: 'issue #556',
54-
});
5552
const buttons = constructHeaderActionButtons({
5653
mute,
5754
subscriptions: [],
5855
ownUser: eg.selfUser,
59-
message,
56+
stream: 'electron issues',
57+
topic: 'issue #556',
6058
});
6159
expect(buttonTitles(buttons)).toContain('Unmute topic');
6260
});
@@ -66,7 +64,8 @@ describe('constructHeaderActionButtons', () => {
6664
mute: [],
6765
subscriptions: [],
6866
ownUser: eg.selfUser,
69-
message: eg.streamMessage(),
67+
stream: streamNameOfStreamMessage(eg.streamMessage()),
68+
topic: eg.streamMessage().subject,
7069
});
7170
expect(buttonTitles(buttons)).toContain('Mute topic');
7271
});
@@ -77,7 +76,8 @@ describe('constructHeaderActionButtons', () => {
7776
mute: [],
7877
subscriptions,
7978
ownUser: eg.selfUser,
80-
message: eg.streamMessage(),
79+
stream: streamNameOfStreamMessage(eg.streamMessage()),
80+
topic: eg.streamMessage().subject,
8181
});
8282
expect(buttonTitles(buttons)).toContain('Unmute stream');
8383
});
@@ -88,7 +88,8 @@ describe('constructHeaderActionButtons', () => {
8888
mute: [],
8989
subscriptions,
9090
ownUser: eg.selfUser,
91-
message: eg.streamMessage(),
91+
stream: streamNameOfStreamMessage(eg.streamMessage()),
92+
topic: eg.streamMessage().subject,
9293
});
9394
expect(buttonTitles(buttons)).toContain('Mute stream');
9495
});
@@ -99,7 +100,8 @@ describe('constructHeaderActionButtons', () => {
99100
mute: [],
100101
subscriptions: [],
101102
ownUser,
102-
message: eg.streamMessage(),
103+
stream: streamNameOfStreamMessage(eg.streamMessage()),
104+
topic: eg.streamMessage().subject,
103105
});
104106
expect(buttonTitles(buttons)).toContain('Delete topic');
105107
});
@@ -109,7 +111,8 @@ describe('constructHeaderActionButtons', () => {
109111
mute: [],
110112
subscriptions: [],
111113
ownUser: eg.selfUser,
112-
message: eg.streamMessage(),
114+
stream: streamNameOfStreamMessage(eg.streamMessage()),
115+
topic: eg.streamMessage().subject,
113116
});
114117
expect(buttonTitles(buttons)).not.toContain('Delete topic');
115118
});

src/message/messageActionSheet.js

Lines changed: 22 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import * as api from '../api';
2727
import { showToast } from '../utils/info';
2828
import { doNarrow, deleteOutboxMessage, navigateToEmojiPicker } from '../actions';
2929
import { navigateToMessageReactionScreen } from '../nav/navActions';
30-
import { pmUiRecipientsFromMessage, streamNameOfStreamMessage } from '../utils/recipient';
30+
import { streamNameOfStreamMessage } from '../utils/recipient';
3131
import { deleteMessagesForTopic } from '../topics/topicActions';
3232
import * as logging from '../utils/logging';
3333

@@ -228,30 +228,29 @@ export const constructHeaderActionButtons = ({
228228
mute,
229229
subscriptions,
230230
ownUser,
231-
message,
231+
stream,
232+
topic,
232233
}: {|
233234
mute: MuteState,
234235
subscriptions: Subscription[],
235236
ownUser: User,
236-
message: Message | Outbox,
237+
stream: string,
238+
topic: string,
237239
|}): HeaderButton[] => {
238240
const buttons = [];
239-
if (message.type === 'stream') {
240-
if (ownUser.is_admin) {
241-
buttons.push(deleteTopic);
242-
}
243-
const streamName = streamNameOfStreamMessage(message);
244-
if (isTopicMuted(streamName, message.subject, mute)) {
245-
buttons.push(unmuteTopic);
246-
} else {
247-
buttons.push(muteTopic);
248-
}
249-
const sub = subscriptions.find(x => x.name === streamName);
250-
if (sub && !sub.in_home_view) {
251-
buttons.push(unmuteStream);
252-
} else {
253-
buttons.push(muteStream);
254-
}
241+
if (ownUser.is_admin) {
242+
buttons.push(deleteTopic);
243+
}
244+
if (isTopicMuted(stream, topic, mute)) {
245+
buttons.push(unmuteTopic);
246+
} else {
247+
buttons.push(muteTopic);
248+
}
249+
const sub = subscriptions.find(x => x.name === stream);
250+
if (sub && !sub.in_home_view) {
251+
buttons.push(unmuteStream);
252+
} else {
253+
buttons.push(muteStream);
255254
}
256255
buttons.push(cancel);
257256
return buttons;
@@ -331,19 +330,6 @@ export const constructNonHeaderActionButtons = ({
331330
}
332331
};
333332

334-
/** Returns the title for the action sheet. */
335-
const getActionSheetTitle = (message: Message | Outbox, ownUser: User): string => {
336-
if (message.type === 'private') {
337-
const recipients = pmUiRecipientsFromMessage(message, ownUser.user_id);
338-
return recipients
339-
.map(r => r.full_name)
340-
.sort()
341-
.join(', ');
342-
} else {
343-
return `#${streamNameOfStreamMessage(message)} > ${message.subject}`;
344-
}
345-
};
346-
347333
function makeButtonCallback<ButtonType: HeaderButton | MessageButton>({
348334
buttonList,
349335
auth,
@@ -439,15 +425,17 @@ export const showHeaderActionSheet = (
439425
}>,
440426
message: Message | Outbox,
441427
): void => {
428+
invariant(message.type === 'stream', 'showHeaderActionSheet: got PM');
442429
const buttonList = constructHeaderActionButtons({
443430
mute: params.mute,
444431
subscriptions: params.subscriptions,
445432
ownUser: params.ownUser,
446-
message,
433+
stream: streamNameOfStreamMessage(message),
434+
topic: message.subject,
447435
});
448436
showActionSheetWithOptions(
449437
{
450-
title: getActionSheetTitle(message, params.ownUser),
438+
title: `#${streamNameOfStreamMessage(message)} > ${message.subject}`,
451439
options: buttonList.map(button => callbacks._(button.title)),
452440
cancelButtonIndex: buttonList.length - 1,
453441
},

src/webview/handleOutboundEvents.js

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import type { BackgroundData } from './MessageList';
99
import type { ShowActionSheetWithOptions } from '../message/messageActionSheet';
1010
import type { JSONableDict } from '../utils/jsonable';
1111
import { showToast } from '../utils/info';
12+
import { pmUiRecipientsFromMessage } from '../utils/recipient';
1213
import { isUrlAnImage } from '../utils/url';
1314
import * as logging from '../utils/logging';
1415
import { filterUnreadMessagesInRange } from '../utils/unread';
@@ -212,12 +213,20 @@ const handleLongPress = (
212213
}
213214
const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props;
214215
if (target === 'header') {
215-
showHeaderActionSheet(
216-
showActionSheetWithOptions,
217-
{ dispatch, startEditMessage, _ },
218-
backgroundData,
219-
message,
220-
);
216+
if (message.type === 'stream') {
217+
showHeaderActionSheet(
218+
showActionSheetWithOptions,
219+
{ dispatch, _ },
220+
backgroundData,
221+
message,
222+
);
223+
} else if (message.type === 'private') {
224+
const label = pmUiRecipientsFromMessage(message, backgroundData.ownUser.user_id)
225+
.map(r => r.full_name)
226+
.sort()
227+
.join(', ');
228+
showToast(label);
229+
}
221230
} else if (target === 'message') {
222231
showMessageActionSheet(
223232
showActionSheetWithOptions,

0 commit comments

Comments
 (0)