Skip to content

Commit 15edce8

Browse files
committed
messageActionSheet: Refactor to not rely on BackgroundData
This will allow us to use the shared messageActionSheet code in the React Native parts of the codebase, rather than only the webview parts of the codebase. It's also generally a cleaner and more type-safe approach. The test coverage for this seems not great, but I manually tested clicking on most of the buttons on both styles of action sheet in the app.
1 parent 2f5fe9c commit 15edce8

File tree

3 files changed

+138
-125
lines changed

3 files changed

+138
-125
lines changed

src/message/__tests__/messageActionSheet-test.js

Lines changed: 25 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -5,116 +5,80 @@ import { HOME_NARROW } from '../../utils/narrow';
55
import * as eg from '../../__tests__/lib/exampleData';
66
import { constructMessageActionButtons, constructHeaderActionButtons } from '../messageActionSheet';
77

8-
const baseBackgroundData = deepFreeze({
9-
alertWords: [],
10-
allImageEmojiById: eg.action.realm_init.data.realm_emoji,
11-
auth: eg.selfAuth,
12-
debug: eg.baseReduxState.session.debug,
13-
flags: eg.baseReduxState.flags,
14-
mute: [],
15-
ownUser: eg.selfUser,
16-
subscriptions: [],
17-
theme: 'default',
18-
twentyFourHourTime: false,
19-
});
20-
218
describe('constructActionButtons', () => {
229
const narrow = deepFreeze(HOME_NARROW);
2310

2411
test('show star message option if message is not starred', () => {
2512
const message = eg.streamMessage();
26-
const flags = { ...baseBackgroundData.flags, starred: {} };
27-
const buttons = constructMessageActionButtons({
28-
backgroundData: { ...baseBackgroundData, flags },
29-
message,
30-
narrow,
31-
});
13+
const flags = { ...eg.baseReduxState.flags, starred: {} };
14+
const buttons = constructMessageActionButtons(eg.selfUser, flags, message, narrow);
3215
expect(buttons).toContain('starMessage');
3316
});
3417

3518
test('show unstar message option if message is starred', () => {
3619
const message = eg.streamMessage();
37-
const flags = { ...baseBackgroundData.flags, starred: { [message.id]: true } };
38-
const buttons = constructMessageActionButtons({
39-
backgroundData: { ...baseBackgroundData, flags },
40-
message,
41-
narrow,
42-
});
20+
const flags = { ...eg.baseReduxState.flags, starred: { [message.id]: true } };
21+
const buttons = constructMessageActionButtons(eg.selfUser, flags, message, narrow);
4322
expect(buttons).toContain('unstarMessage');
4423
});
4524

4625
test('show reactions option if message is has at least one reaction', () => {
47-
const buttons = constructMessageActionButtons({
48-
backgroundData: baseBackgroundData,
49-
message: eg.streamMessage({ reactions: [eg.unicodeEmojiReaction] }),
26+
const buttons = constructMessageActionButtons(
27+
eg.selfUser,
28+
eg.baseReduxState.flags,
29+
eg.streamMessage({ reactions: [eg.unicodeEmojiReaction] }),
5030
narrow,
51-
});
31+
);
5232
expect(buttons).toContain('showReactions');
5333
});
5434
});
5535

5636
describe('constructHeaderActionButtons', () => {
57-
const narrow = deepFreeze(HOME_NARROW);
58-
5937
test('show Unmute topic option if topic is muted', () => {
6038
const mute = deepFreeze([['electron issues', 'issue #556']]);
6139
const message = eg.streamMessage({
6240
display_recipient: 'electron issues',
6341
subject: 'issue #556',
6442
});
65-
const buttons = constructHeaderActionButtons({
66-
backgroundData: { ...baseBackgroundData, mute },
67-
message,
68-
narrow,
69-
});
43+
const buttons = constructHeaderActionButtons(mute, [], eg.selfUser, message);
7044
expect(buttons).toContain('unmuteTopic');
7145
});
7246

7347
test('show mute topic option if topic is not muted', () => {
74-
const buttons = constructHeaderActionButtons({
75-
backgroundData: { ...baseBackgroundData, mute: [] },
76-
message: eg.streamMessage(),
77-
narrow,
78-
});
48+
const buttons = constructHeaderActionButtons([], [], eg.selfUser, eg.streamMessage());
7949
expect(buttons).toContain('muteTopic');
8050
});
8151

8252
test('show Unmute stream option if stream is not in home view', () => {
8353
const subscriptions = [{ ...eg.subscription, in_home_view: false }];
84-
const buttons = constructHeaderActionButtons({
85-
backgroundData: { ...baseBackgroundData, subscriptions },
86-
message: eg.streamMessage(),
87-
narrow,
88-
});
54+
const buttons = constructHeaderActionButtons(
55+
[],
56+
subscriptions,
57+
eg.selfUser,
58+
eg.streamMessage(),
59+
);
8960
expect(buttons).toContain('unmuteStream');
9061
});
9162

9263
test('show mute stream option if stream is in home view', () => {
9364
const subscriptions = [{ ...eg.subscription, in_home_view: true }];
94-
const buttons = constructHeaderActionButtons({
95-
backgroundData: { ...baseBackgroundData, subscriptions },
96-
message: eg.streamMessage(),
97-
narrow,
98-
});
65+
const buttons = constructHeaderActionButtons(
66+
[],
67+
subscriptions,
68+
eg.selfUser,
69+
eg.streamMessage(),
70+
);
9971
expect(buttons).toContain('muteStream');
10072
});
10173

10274
test('show delete topic option if current user is an admin', () => {
10375
const ownUser = { ...eg.selfUser, is_admin: true };
104-
const buttons = constructHeaderActionButtons({
105-
backgroundData: { ...baseBackgroundData, ownUser },
106-
message: eg.streamMessage(),
107-
narrow,
108-
});
76+
const buttons = constructHeaderActionButtons([], [], ownUser, eg.streamMessage());
10977
expect(buttons).toContain('deleteTopic');
11078
});
11179

11280
test('do not show delete topic option if current user is not an admin', () => {
113-
const buttons = constructHeaderActionButtons({
114-
backgroundData: baseBackgroundData,
115-
message: eg.streamMessage(),
116-
narrow,
117-
});
81+
const buttons = constructHeaderActionButtons([], [], eg.selfUser, eg.streamMessage());
11882
expect(buttons).not.toContain('deleteTopic');
11983
});
12084
});

src/message/messageActionSheet.js

Lines changed: 89 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,16 @@ import * as NavigationService from '../nav/NavigationService';
66
import type {
77
Auth,
88
Dispatch,
9+
FlagsState,
910
GetText,
1011
Message,
12+
MuteState,
1113
Narrow,
1214
Outbox,
1315
Subscription,
1416
User,
1517
EditMessage,
1618
} from '../types';
17-
import type { BackgroundData } from '../webview/MessageList';
1819
import {
1920
getNarrowForReply,
2021
isPmNarrow,
@@ -241,16 +242,12 @@ type ButtonCode = $Keys<typeof allButtonsRaw>;
241242

242243
const allButtons: {| [ButtonCode]: ButtonDescription |} = allButtonsRaw;
243244

244-
type ConstructSheetParams<MsgType: Message | Outbox = Message | Outbox> = {|
245-
backgroundData: BackgroundData,
246-
message: MsgType,
247-
narrow: Narrow,
248-
|};
249-
250-
export const constructHeaderActionButtons = ({
251-
backgroundData: { mute, subscriptions, ownUser },
252-
message,
253-
}: ConstructSheetParams<>): ButtonCode[] => {
245+
export const constructHeaderActionButtons = (
246+
mute: MuteState,
247+
subscriptions: Subscription[],
248+
ownUser: User,
249+
message: Message | Outbox,
250+
): ButtonCode[] => {
254251
const buttons: ButtonCode[] = [];
255252
if (message.type === 'stream') {
256253
if (ownUser.is_admin) {
@@ -273,11 +270,7 @@ export const constructHeaderActionButtons = ({
273270
return buttons;
274271
};
275272

276-
export const constructOutboxActionButtons = ({
277-
backgroundData,
278-
message,
279-
narrow,
280-
}: ConstructSheetParams<Outbox>): ButtonCode[] => {
273+
export const constructOutboxActionButtons = (message: Outbox, narrow: Narrow): ButtonCode[] => {
281274
const buttons = [];
282275
buttons.push('copyToClipboard');
283276
buttons.push('shareMessage');
@@ -289,11 +282,12 @@ export const constructOutboxActionButtons = ({
289282
const messageNotDeleted = (message: Message | Outbox): boolean =>
290283
message.content !== '<p>(deleted)</p>';
291284

292-
export const constructMessageActionButtons = ({
293-
backgroundData: { ownUser, flags },
294-
message,
295-
narrow,
296-
}: ConstructSheetParams<Message>): ButtonCode[] => {
285+
export const constructMessageActionButtons = (
286+
ownUser: User,
287+
flags: FlagsState,
288+
message: Message,
289+
narrow: Narrow,
290+
): ButtonCode[] => {
297291
const buttons = [];
298292
if (messageNotDeleted(message)) {
299293
buttons.push('addReaction');
@@ -327,15 +321,16 @@ export const constructMessageActionButtons = ({
327321
return buttons;
328322
};
329323

330-
export const constructNonHeaderActionButtons = ({
331-
backgroundData,
332-
message,
333-
narrow,
334-
}: ConstructSheetParams<>): ButtonCode[] => {
324+
export const constructNonHeaderActionButtons = (
325+
ownUser: User,
326+
flags: FlagsState,
327+
message: Message | Outbox,
328+
narrow: Narrow,
329+
): ButtonCode[] => {
335330
if (message.isOutbox) {
336-
return constructOutboxActionButtons({ backgroundData, message, narrow });
331+
return constructOutboxActionButtons(message, narrow);
337332
} else {
338-
return constructMessageActionButtons({ backgroundData, message, narrow });
333+
return constructMessageActionButtons(ownUser, flags, message, narrow);
339334
}
340335
};
341336

@@ -352,46 +347,83 @@ const getActionSheetTitle = (message: Message | Outbox, ownUser: User): string =
352347
}
353348
};
354349

355-
/** Invoke the given callback to show an appropriate action sheet. */
356-
export const showActionSheet = (
357-
isHeader: boolean,
350+
const makeButtonCallback = (
351+
optionCodes: ButtonCode[],
352+
auth: Auth,
353+
subscriptions: Subscription[],
354+
ownUser: User,
355+
flags: FlagsState,
356+
message: Message | Outbox,
357+
callbacks: {|
358+
dispatch: Dispatch,
359+
startEditMessage: (editMessage: EditMessage) => void,
360+
_: GetText,
361+
|},
362+
) => buttonIndex => {
363+
(async () => {
364+
const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]];
365+
try {
366+
await pressedButton({
367+
auth,
368+
subscriptions,
369+
ownUser,
370+
flags,
371+
message,
372+
...callbacks,
373+
});
374+
} catch (err) {
375+
Alert.alert(callbacks._(pressedButton.errorMessage), err.message);
376+
}
377+
})();
378+
};
379+
380+
export const showMessageActionSheet = (
358381
showActionSheetWithOptions: ShowActionSheetWithOptions,
359382
callbacks: {|
360383
dispatch: Dispatch,
361384
startEditMessage: (editMessage: EditMessage) => void,
362385
_: GetText,
363386
|},
364-
params: ConstructSheetParams<>,
387+
auth: Auth,
388+
subscriptions: Subscription[],
389+
ownUser: User,
390+
flags: FlagsState,
391+
message: Message | Outbox,
392+
narrow: Narrow,
393+
): void => {
394+
const optionCodes = constructNonHeaderActionButtons(ownUser, flags, message, narrow);
395+
showActionSheetWithOptions(
396+
{
397+
options: optionCodes.map(code => callbacks._(allButtons[code].title)),
398+
cancelButtonIndex: optionCodes.length - 1,
399+
},
400+
makeButtonCallback(optionCodes, auth, subscriptions, ownUser, flags, message, callbacks),
401+
);
402+
};
403+
404+
export const showHeaderActionSheet = (
405+
showActionSheetWithOptions: ShowActionSheetWithOptions,
406+
callbacks: {|
407+
dispatch: Dispatch,
408+
_: GetText,
409+
|},
410+
auth: Auth,
411+
mute: MuteState,
412+
subscriptions: Subscription[],
413+
ownUser: User,
414+
flags: FlagsState,
415+
message: Message | Outbox,
365416
): void => {
366-
const optionCodes = isHeader
367-
? constructHeaderActionButtons(params)
368-
: constructNonHeaderActionButtons(params);
369-
const callback = buttonIndex => {
370-
(async () => {
371-
const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]];
372-
try {
373-
await pressedButton({
374-
subscriptions: params.backgroundData.subscriptions,
375-
auth: params.backgroundData.auth,
376-
ownUser: params.backgroundData.ownUser,
377-
...params,
378-
...callbacks,
379-
});
380-
} catch (err) {
381-
Alert.alert(callbacks._(pressedButton.errorMessage), err.message);
382-
}
383-
})();
384-
};
417+
const optionCodes = constructHeaderActionButtons(mute, subscriptions, ownUser, message);
385418
showActionSheetWithOptions(
386419
{
387-
...(isHeader
388-
? {
389-
title: getActionSheetTitle(params.message, params.backgroundData.ownUser),
390-
}
391-
: {}),
420+
title: getActionSheetTitle(message, ownUser),
392421
options: optionCodes.map(code => callbacks._(allButtons[code].title)),
393422
cancelButtonIndex: optionCodes.length - 1,
394423
},
395-
callback,
424+
makeButtonCallback(optionCodes, auth, subscriptions, ownUser, flags, message, {
425+
startEditMessage: _ => {},
426+
...callbacks,
427+
}),
396428
);
397429
};

src/webview/handleOutboundEvents.js

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import {
2222
navigateToLightbox,
2323
messageLinkPress,
2424
} from '../actions';
25-
import { showActionSheet } from '../message/messageActionSheet';
25+
import { showMessageActionSheet, showHeaderActionSheet } from '../message/messageActionSheet';
2626
import { ensureUnreachable } from '../types';
2727
import { base64Utf8Decode } from '../utils/encoding';
2828

@@ -211,12 +211,29 @@ const handleLongPress = (
211211
return;
212212
}
213213
const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props;
214-
showActionSheet(
215-
target === 'header',
216-
showActionSheetWithOptions,
217-
{ dispatch, startEditMessage, _ },
218-
{ backgroundData, message, narrow },
219-
);
214+
if (target === 'header') {
215+
showHeaderActionSheet(
216+
showActionSheetWithOptions,
217+
{ dispatch, _ },
218+
backgroundData.auth,
219+
backgroundData.mute,
220+
backgroundData.subscriptions,
221+
backgroundData.ownUser,
222+
backgroundData.flags,
223+
message,
224+
);
225+
} else if (target === 'message') {
226+
showMessageActionSheet(
227+
showActionSheetWithOptions,
228+
{ dispatch, startEditMessage, _ },
229+
backgroundData.auth,
230+
backgroundData.subscriptions,
231+
backgroundData.ownUser,
232+
backgroundData.flags,
233+
message,
234+
narrow,
235+
);
236+
}
220237
};
221238

222239
export const handleWebViewOutboundEvent = (

0 commit comments

Comments
 (0)