Skip to content

Commit ffb03e5

Browse files
committed
ActionSheets: Remove message arg from constructHeaderActionButtons
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. 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.
1 parent acb61f8 commit ffb03e5

File tree

2 files changed

+38
-44
lines changed

2 files changed

+38
-44
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';
@@ -58,21 +59,19 @@ describe('constructActionButtons', () => {
5859
describe('constructHeaderActionButtons', () => {
5960
test('show Unmute topic option if topic is muted', () => {
6061
const mute = deepFreeze([['electron issues', 'issue #556']]);
61-
const message = eg.streamMessage({
62-
display_recipient: 'electron issues',
63-
subject: 'issue #556',
64-
});
6562
const buttons = constructHeaderActionButtons({
6663
backgroundData: { ...baseBackgroundData, mute },
67-
message,
64+
stream: 'electron issues',
65+
topic: 'issue #556',
6866
});
6967
expect(buttonTitles(buttons)).toContain('Unmute topic');
7068
});
7169

7270
test('show mute topic option if topic is not muted', () => {
7371
const buttons = constructHeaderActionButtons({
7472
backgroundData: { ...baseBackgroundData, mute: [] },
75-
message: eg.streamMessage(),
73+
stream: streamNameOfStreamMessage(eg.streamMessage()),
74+
topic: eg.streamMessage().subject,
7675
});
7776
expect(buttonTitles(buttons)).toContain('Mute topic');
7877
});
@@ -81,7 +80,8 @@ describe('constructHeaderActionButtons', () => {
8180
const subscriptions = [{ ...eg.subscription, in_home_view: false }];
8281
const buttons = constructHeaderActionButtons({
8382
backgroundData: { ...baseBackgroundData, subscriptions },
84-
message: eg.streamMessage(),
83+
stream: streamNameOfStreamMessage(eg.streamMessage()),
84+
topic: eg.streamMessage().subject,
8585
});
8686
expect(buttonTitles(buttons)).toContain('Unmute stream');
8787
});
@@ -90,7 +90,8 @@ describe('constructHeaderActionButtons', () => {
9090
const subscriptions = [{ ...eg.subscription, in_home_view: true }];
9191
const buttons = constructHeaderActionButtons({
9292
backgroundData: { ...baseBackgroundData, subscriptions },
93-
message: eg.streamMessage(),
93+
stream: streamNameOfStreamMessage(eg.streamMessage()),
94+
topic: eg.streamMessage().subject,
9495
});
9596
expect(buttonTitles(buttons)).toContain('Mute stream');
9697
});
@@ -99,15 +100,17 @@ describe('constructHeaderActionButtons', () => {
99100
const ownUser = { ...eg.selfUser, is_admin: true };
100101
const buttons = constructHeaderActionButtons({
101102
backgroundData: { ...baseBackgroundData, ownUser },
102-
message: eg.streamMessage(),
103+
stream: streamNameOfStreamMessage(eg.streamMessage()),
104+
topic: eg.streamMessage().subject,
103105
});
104106
expect(buttonTitles(buttons)).toContain('Delete topic');
105107
});
106108

107109
test('do not show delete topic option if current user is not an admin', () => {
108110
const buttons = constructHeaderActionButtons({
109111
backgroundData: baseBackgroundData,
110-
message: eg.streamMessage(),
112+
stream: streamNameOfStreamMessage(eg.streamMessage()),
113+
topic: eg.streamMessage().subject,
111114
});
112115
expect(buttonTitles(buttons)).not.toContain('Delete topic');
113116
});

src/message/messageActionSheet.js

Lines changed: 25 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

@@ -220,33 +220,32 @@ cancel.errorMessage = 'Failed to hide menu';
220220

221221
export const constructHeaderActionButtons = ({
222222
backgroundData: { mute, subscriptions, ownUser },
223-
message,
223+
stream,
224+
topic,
224225
}: {|
225226
backgroundData: $ReadOnly<{
226227
mute: MuteState,
227228
subscriptions: Subscription[],
228229
ownUser: User,
229230
...
230231
}>,
231-
message: Message | Outbox,
232+
stream: string,
233+
topic: string,
232234
|}): Button<HeaderArgs>[] => {
233235
const buttons = [];
234-
if (message.type === 'stream') {
235-
if (ownUser.is_admin) {
236-
buttons.push(deleteTopic);
237-
}
238-
const streamName = streamNameOfStreamMessage(message);
239-
if (isTopicMuted(streamName, message.subject, mute)) {
240-
buttons.push(unmuteTopic);
241-
} else {
242-
buttons.push(muteTopic);
243-
}
244-
const sub = subscriptions.find(x => x.name === streamName);
245-
if (sub && !sub.in_home_view) {
246-
buttons.push(unmuteStream);
247-
} else {
248-
buttons.push(muteStream);
249-
}
236+
if (ownUser.is_admin) {
237+
buttons.push(deleteTopic);
238+
}
239+
if (isTopicMuted(stream, topic, mute)) {
240+
buttons.push(unmuteTopic);
241+
} else {
242+
buttons.push(muteTopic);
243+
}
244+
const sub = subscriptions.find(x => x.name === stream);
245+
if (sub && !sub.in_home_view) {
246+
buttons.push(unmuteStream);
247+
} else {
248+
buttons.push(muteStream);
250249
}
251250
buttons.push(cancel);
252251
return buttons;
@@ -329,19 +328,6 @@ export const constructNonHeaderActionButtons = ({
329328
}
330329
};
331330

332-
/** Returns the title for the action sheet. */
333-
const getActionSheetTitle = (message: Message | Outbox, ownUser: User): string => {
334-
if (message.type === 'private') {
335-
const recipients = pmUiRecipientsFromMessage(message, ownUser.user_id);
336-
return recipients
337-
.map(r => r.full_name)
338-
.sort()
339-
.join(', ');
340-
} else {
341-
return `#${streamNameOfStreamMessage(message)} > ${message.subject}`;
342-
}
343-
};
344-
345331
export const showMessageActionSheet = ({
346332
showActionSheetWithOptions,
347333
callbacks,
@@ -410,7 +396,12 @@ export const showHeaderActionSheet = ({
410396
}>,
411397
message: Message | Outbox,
412398
|}): void => {
413-
const buttonList = constructHeaderActionButtons({ backgroundData, message });
399+
invariant(message.type === 'stream', 'showHeaderActionSheet: got PM');
400+
const buttonList = constructHeaderActionButtons({
401+
backgroundData,
402+
stream: streamNameOfStreamMessage(message),
403+
topic: message.subject,
404+
});
414405
const callback = buttonIndex => {
415406
(async () => {
416407
const pressedButton: Button<HeaderArgs> = buttonList[buttonIndex];
@@ -427,7 +418,7 @@ export const showHeaderActionSheet = ({
427418
};
428419
showActionSheetWithOptions(
429420
{
430-
title: getActionSheetTitle(message, backgroundData.ownUser),
421+
title: `#${streamNameOfStreamMessage(message)} > ${message.subject}`,
431422
options: buttonList.map(button => callbacks._(button.title)),
432423
cancelButtonIndex: buttonList.length - 1,
433424
},

0 commit comments

Comments
 (0)