diff --git a/src/message/__tests__/messageActionSheet-test.js b/src/message/__tests__/messageActionSheet-test.js index d6f6b9f463b..4ba396319a9 100644 --- a/src/message/__tests__/messageActionSheet-test.js +++ b/src/message/__tests__/messageActionSheet-test.js @@ -1,6 +1,7 @@ // @flow strict-local import deepFreeze from 'deep-freeze'; import { HOME_NARROW } from '../../utils/narrow'; +import { streamNameOfStreamMessage } from '../../utils/recipient'; import * as eg from '../../__tests__/lib/exampleData'; import { constructMessageActionButtons, constructHeaderActionButtons } from '../messageActionSheet'; @@ -18,6 +19,8 @@ const baseBackgroundData = deepFreeze({ twentyFourHourTime: false, }); +const buttonTitles = buttons => buttons.map(button => button.title); + describe('constructActionButtons', () => { const narrow = deepFreeze(HOME_NARROW); @@ -29,7 +32,7 @@ describe('constructActionButtons', () => { message, narrow, }); - expect(buttons).toContain('starMessage'); + expect(buttonTitles(buttons)).toContain('Star message'); }); test('show unstar message option if message is starred', () => { @@ -40,7 +43,7 @@ describe('constructActionButtons', () => { message, narrow, }); - expect(buttons).toContain('unstarMessage'); + expect(buttonTitles(buttons)).toContain('Unstar message'); }); test('show reactions option if message is has at least one reaction', () => { @@ -49,72 +52,66 @@ describe('constructActionButtons', () => { message: eg.streamMessage({ reactions: [eg.unicodeEmojiReaction] }), narrow, }); - expect(buttons).toContain('showReactions'); + expect(buttonTitles(buttons)).toContain('See who reacted'); }); }); describe('constructHeaderActionButtons', () => { - const narrow = deepFreeze(HOME_NARROW); - test('show Unmute topic option if topic is muted', () => { const mute = deepFreeze([['electron issues', 'issue #556']]); - const message = eg.streamMessage({ - display_recipient: 'electron issues', - subject: 'issue #556', - }); const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, mute }, - message, - narrow, + stream: 'electron issues', + topic: 'issue #556', }); - expect(buttons).toContain('unmuteTopic'); + expect(buttonTitles(buttons)).toContain('Unmute topic'); }); test('show mute topic option if topic is not muted', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, mute: [] }, - message: eg.streamMessage(), - narrow, + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); - expect(buttons).toContain('muteTopic'); + expect(buttonTitles(buttons)).toContain('Mute topic'); }); test('show Unmute stream option if stream is not in home view', () => { const subscriptions = [{ ...eg.subscription, in_home_view: false }]; const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, subscriptions }, - message: eg.streamMessage(), - narrow, + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); - expect(buttons).toContain('unmuteStream'); + expect(buttonTitles(buttons)).toContain('Unmute stream'); }); test('show mute stream option if stream is in home view', () => { const subscriptions = [{ ...eg.subscription, in_home_view: true }]; const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, subscriptions }, - message: eg.streamMessage(), - narrow, + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); - expect(buttons).toContain('muteStream'); + expect(buttonTitles(buttons)).toContain('Mute stream'); }); test('show delete topic option if current user is an admin', () => { const ownUser = { ...eg.selfUser, is_admin: true }; const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, ownUser }, - message: eg.streamMessage(), - narrow, + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); - expect(buttons).toContain('deleteTopic'); + expect(buttonTitles(buttons)).toContain('Delete topic'); }); test('do not show delete topic option if current user is not an admin', () => { const buttons = constructHeaderActionButtons({ backgroundData: baseBackgroundData, - message: eg.streamMessage(), - narrow, + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); - expect(buttons).not.toContain('deleteTopic'); + expect(buttonTitles(buttons)).not.toContain('Delete topic'); }); }); diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index 81d93817fb5..264ab114e37 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -1,20 +1,20 @@ /* @flow strict-local */ -import invariant from 'invariant'; import { Clipboard, Share, Alert } from 'react-native'; import * as NavigationService from '../nav/NavigationService'; import type { Auth, Dispatch, + FlagsState, GetText, Message, + MuteState, Narrow, Outbox, Subscription, User, EditMessage, } from '../types'; -import type { BackgroundData } from '../webview/MessageList'; import { getNarrowForReply, isPmNarrow, @@ -26,7 +26,6 @@ import * as api from '../api'; import { showToast } from '../utils/info'; import { doNarrow, deleteOutboxMessage, navigateToEmojiPicker } from '../actions'; import { navigateToMessageReactionScreen } from '../nav/navActions'; -import { pmUiRecipientsFromMessage, streamNameOfStreamMessage } from '../utils/recipient'; import { deleteMessagesForTopic } from '../topics/topicActions'; import * as logging from '../utils/logging'; @@ -36,27 +35,35 @@ export type ShowActionSheetWithOptions = ( (number) => void, ) => void; -/** Description of a possible option for the action sheet. */ -type ButtonDescription = { - /** The callback. */ - ({ - auth: Auth, - ownUser: User, - message: Message | Outbox, - subscriptions: Subscription[], - dispatch: Dispatch, - _: GetText, - startEditMessage: (editMessage: EditMessage) => void, - ... - }): void | Promise, +type HeaderArgs = { + auth: Auth, + stream: string, + topic: string, + subscriptions: Subscription[], + dispatch: Dispatch, + _: GetText, + ... +}; + +type MessageArgs = { + auth: Auth, + ownUser: User, + message: Message | Outbox, + dispatch: Dispatch, + _: GetText, + startEditMessage: (editMessage: EditMessage) => void, + ... +}; + +type Button = {| + (Args): void | Promise, title: string, /** The title of the alert-box that will be displayed if the callback throws. */ // Required even when the callback can't throw (e.g., "Cancel"), since we can't // otherwise ensure that everything that _can_ throw has one. errorMessage: string, - ... -}; +|}; // // Options for the action sheet go below: ... @@ -104,25 +111,20 @@ const deleteMessage = async ({ auth, message, dispatch }) => { deleteMessage.title = 'Delete message'; deleteMessage.errorMessage = 'Failed to delete message'; -const unmuteTopic = async ({ auth, message }) => { - invariant(message.type === 'stream', 'unmuteTopic: got PM'); - await api.unmuteTopic(auth, streamNameOfStreamMessage(message), message.subject); +const unmuteTopic = async ({ auth, stream, topic }) => { + await api.unmuteTopic(auth, stream, topic); }; unmuteTopic.title = 'Unmute topic'; unmuteTopic.errorMessage = 'Failed to unmute topic'; -const muteTopic = async ({ auth, message }) => { - invariant(message.type === 'stream', 'muteTopic: got PM'); - await api.muteTopic(auth, streamNameOfStreamMessage(message), message.subject); +const muteTopic = async ({ auth, stream, topic }) => { + await api.muteTopic(auth, stream, topic); }; muteTopic.title = 'Mute topic'; muteTopic.errorMessage = 'Failed to mute topic'; -const deleteTopic = async ({ auth, message, dispatch, _ }) => { - invariant(message.type === 'stream', 'deleteTopic: got PM'); - const alertTitle = _('Are you sure you want to delete the topic “{topic}”?', { - topic: message.subject, - }); +const deleteTopic = async ({ auth, stream, topic, dispatch, _ }) => { + const alertTitle = _('Are you sure you want to delete the topic “{topic}”?', { topic }); const AsyncAlert = async (): Promise => new Promise((resolve, reject) => { Alert.alert( @@ -148,15 +150,14 @@ const deleteTopic = async ({ auth, message, dispatch, _ }) => { ); }); if (await AsyncAlert()) { - await dispatch(deleteMessagesForTopic(streamNameOfStreamMessage(message), message.subject)); + await dispatch(deleteMessagesForTopic(stream, topic)); } }; deleteTopic.title = 'Delete topic'; deleteTopic.errorMessage = 'Failed to delete topic'; -const unmuteStream = async ({ auth, message, subscriptions }) => { - invariant(message.type === 'stream', 'unmuteStream: got PM'); - const sub = subscriptions.find(x => x.name === streamNameOfStreamMessage(message)); +const unmuteStream = async ({ auth, stream, subscriptions }) => { + const sub = subscriptions.find(x => x.name === stream); if (sub) { await api.toggleMuteStream(auth, sub.stream_id, false); } @@ -164,9 +165,8 @@ const unmuteStream = async ({ auth, message, subscriptions }) => { unmuteStream.title = 'Unmute stream'; unmuteStream.errorMessage = 'Failed to unmute stream'; -const muteStream = async ({ auth, message, subscriptions }) => { - invariant(message.type === 'stream', 'muteStream: got PM'); - const sub = subscriptions.find(x => x.name === streamNameOfStreamMessage(message)); +const muteStream = async ({ auth, stream, subscriptions }) => { + const sub = subscriptions.find(x => x.name === stream); if (sub) { await api.toggleMuteStream(auth, sub.stream_id, true); } @@ -210,79 +210,45 @@ const cancel = params => {}; cancel.title = 'Cancel'; cancel.errorMessage = 'Failed to hide menu'; -const allButtonsRaw = { - // For messages - addReaction, - reply, - copyToClipboard, - shareMessage, - editMessage, - deleteMessage, - starMessage, - unstarMessage, - showReactions, - - // For headers - unmuteTopic, - muteTopic, - deleteTopic, - muteStream, - unmuteStream, - - // All - cancel, -}; - -// -// ... End of options for the action sheet. -// - -type ButtonCode = $Keys; - -const allButtons: {| [ButtonCode]: ButtonDescription |} = allButtonsRaw; - -type ConstructSheetParams = {| - backgroundData: BackgroundData, - message: MsgType, - narrow: Narrow, -|}; - export const constructHeaderActionButtons = ({ backgroundData: { mute, subscriptions, ownUser }, - message, -}: ConstructSheetParams<>): ButtonCode[] => { - const buttons: ButtonCode[] = []; - if (message.type === 'stream') { - if (ownUser.is_admin) { - buttons.push('deleteTopic'); - } - const streamName = streamNameOfStreamMessage(message); - if (isTopicMuted(streamName, message.subject, mute)) { - buttons.push('unmuteTopic'); - } else { - buttons.push('muteTopic'); - } - const sub = subscriptions.find(x => x.name === streamName); - if (sub && !sub.in_home_view) { - buttons.push('unmuteStream'); - } else { - buttons.push('muteStream'); - } + stream, + topic, +}: {| + backgroundData: $ReadOnly<{ + mute: MuteState, + subscriptions: Subscription[], + ownUser: User, + ... + }>, + stream: string, + topic: string, +|}): Button[] => { + const buttons = []; + if (ownUser.is_admin) { + buttons.push(deleteTopic); + } + if (isTopicMuted(stream, topic, mute)) { + buttons.push(unmuteTopic); + } else { + buttons.push(muteTopic); + } + const sub = subscriptions.find(x => x.name === stream); + if (sub && !sub.in_home_view) { + buttons.push(unmuteStream); + } else { + buttons.push(muteStream); } - buttons.push('cancel'); + buttons.push(cancel); return buttons; }; -export const constructOutboxActionButtons = ({ - backgroundData, - message, - narrow, -}: ConstructSheetParams): ButtonCode[] => { +export const constructOutboxActionButtons = (): Button[] => { const buttons = []; - buttons.push('copyToClipboard'); - buttons.push('shareMessage'); - buttons.push('deleteMessage'); - buttons.push('cancel'); + buttons.push(copyToClipboard); + buttons.push(shareMessage); + buttons.push(deleteMessage); + buttons.push(cancel); return buttons; }; @@ -293,37 +259,45 @@ export const constructMessageActionButtons = ({ backgroundData: { ownUser, flags }, message, narrow, -}: ConstructSheetParams): ButtonCode[] => { +}: { + backgroundData: $ReadOnly<{ + ownUser: User, + flags: FlagsState, + ... + }>, + message: Message, + narrow: Narrow, +}): Button[] => { const buttons = []; if (messageNotDeleted(message)) { - buttons.push('addReaction'); + buttons.push(addReaction); } if (message.reactions.length > 0) { - buttons.push('showReactions'); + buttons.push(showReactions); } if (!isTopicNarrow(narrow) && !isPmNarrow(narrow)) { - buttons.push('reply'); + buttons.push(reply); } if (messageNotDeleted(message)) { - buttons.push('copyToClipboard'); - buttons.push('shareMessage'); + buttons.push(copyToClipboard); + buttons.push(shareMessage); } if ( message.sender_id === ownUser.user_id // Our "edit message" UI only works in certain kinds of narrows. && (isStreamOrTopicNarrow(narrow) || isPmNarrow(narrow)) ) { - buttons.push('editMessage'); + buttons.push(editMessage); } if (message.sender_id === ownUser.user_id && messageNotDeleted(message)) { - buttons.push('deleteMessage'); + buttons.push(deleteMessage); } if (message.id in flags.starred) { - buttons.push('unstarMessage'); + buttons.push(unstarMessage); } else { - buttons.push('starMessage'); + buttons.push(starMessage); } - buttons.push('cancel'); + buttons.push(cancel); return buttons; }; @@ -331,51 +305,107 @@ export const constructNonHeaderActionButtons = ({ backgroundData, message, narrow, -}: ConstructSheetParams<>): ButtonCode[] => { +}: {| + backgroundData: $ReadOnly<{ + ownUser: User, + flags: FlagsState, + ... + }>, + message: Message | Outbox, + narrow: Narrow, +|}): Button[] => { if (message.isOutbox) { - return constructOutboxActionButtons({ backgroundData, message, narrow }); + return constructOutboxActionButtons(); } else { return constructMessageActionButtons({ backgroundData, message, narrow }); } }; -/** Returns the title for the action sheet. */ -const getActionSheetTitle = (message: Message | Outbox, ownUser: User): string => { - if (message.type === 'private') { - const recipients = pmUiRecipientsFromMessage(message, ownUser.user_id); - return recipients - .map(r => r.full_name) - .sort() - .join(', '); - } else { - return `#${streamNameOfStreamMessage(message)} > ${message.subject}`; - } +export const showMessageActionSheet = ({ + showActionSheetWithOptions, + callbacks, + backgroundData, + message, + narrow, +}: {| + showActionSheetWithOptions: ShowActionSheetWithOptions, + callbacks: {| + dispatch: Dispatch, + startEditMessage: (editMessage: EditMessage) => void, + _: GetText, + |}, + backgroundData: $ReadOnly<{ + auth: Auth, + subscriptions: Subscription[], + ownUser: User, + flags: FlagsState, + ... + }>, + message: Message | Outbox, + narrow: Narrow, +|}): void => { + const buttonList = constructNonHeaderActionButtons({ backgroundData, message, narrow }); + const callback = buttonIndex => { + (async () => { + const pressedButton: Button = buttonList[buttonIndex]; + try { + await pressedButton({ + ...backgroundData, + ...callbacks, + message, + narrow, + }); + } catch (err) { + Alert.alert(callbacks._(pressedButton.errorMessage), err.message); + } + })(); + }; + showActionSheetWithOptions( + { + options: buttonList.map(button => callbacks._(button.title)), + cancelButtonIndex: buttonList.length - 1, + }, + callback, + ); }; -/** Invoke the given callback to show an appropriate action sheet. */ -export const showActionSheet = ( - isHeader: boolean, +export const showHeaderActionSheet = ({ + showActionSheetWithOptions, + callbacks, + backgroundData, + topic, + stream, +}: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, - startEditMessage: (editMessage: EditMessage) => void, _: GetText, |}, - params: ConstructSheetParams<>, -): void => { - const optionCodes = isHeader - ? constructHeaderActionButtons(params) - : constructNonHeaderActionButtons(params); + backgroundData: $ReadOnly<{ + auth: Auth, + mute: MuteState, + subscriptions: Subscription[], + ownUser: User, + flags: FlagsState, + ... + }>, + stream: string, + topic: string, +|}): void => { + const buttonList = constructHeaderActionButtons({ + backgroundData, + stream, + topic, + }); const callback = buttonIndex => { (async () => { - const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; + const pressedButton: Button = buttonList[buttonIndex]; try { await pressedButton({ - subscriptions: params.backgroundData.subscriptions, - auth: params.backgroundData.auth, - ownUser: params.backgroundData.ownUser, - ...params, + ...backgroundData, ...callbacks, + stream, + topic, }); } catch (err) { Alert.alert(callbacks._(pressedButton.errorMessage), err.message); @@ -384,13 +414,9 @@ export const showActionSheet = ( }; showActionSheetWithOptions( { - ...(isHeader - ? { - title: getActionSheetTitle(params.message, params.backgroundData.ownUser), - } - : {}), - options: optionCodes.map(code => callbacks._(allButtons[code].title)), - cancelButtonIndex: optionCodes.length - 1, + title: `#${stream} > ${topic}`, + options: buttonList.map(button => callbacks._(button.title)), + cancelButtonIndex: buttonList.length - 1, }, callback, ); diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index e6e0b51a63b..fdbd8b5a9c7 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -9,6 +9,7 @@ import type { BackgroundData } from './MessageList'; import type { ShowActionSheetWithOptions } from '../message/messageActionSheet'; import type { JSONableDict } from '../utils/jsonable'; import { showToast } from '../utils/info'; +import { streamNameOfStreamMessage, pmUiRecipientsFromMessage } from '../utils/recipient'; import { isUrlAnImage } from '../utils/url'; import * as logging from '../utils/logging'; import { filterUnreadMessagesInRange } from '../utils/unread'; @@ -22,7 +23,7 @@ import { navigateToLightbox, messageLinkPress, } from '../actions'; -import { showActionSheet } from '../message/messageActionSheet'; +import { showHeaderActionSheet, showMessageActionSheet } from '../message/messageActionSheet'; import { ensureUnreachable } from '../types'; import { base64Utf8Decode } from '../utils/encoding'; @@ -211,12 +212,31 @@ const handleLongPress = ( return; } const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props; - showActionSheet( - target === 'header', - showActionSheetWithOptions, - { dispatch, startEditMessage, _ }, - { backgroundData, message, narrow }, - ); + if (target === 'header') { + if (message.type === 'stream') { + showHeaderActionSheet({ + showActionSheetWithOptions, + callbacks: { dispatch, _ }, + backgroundData, + stream: streamNameOfStreamMessage(message), + topic: message.subject, + }); + } else if (message.type === 'private') { + const label = pmUiRecipientsFromMessage(message, backgroundData.ownUser.user_id) + .map(r => r.full_name) + .sort() + .join(', '); + showToast(label); + } + } else if (target === 'message') { + showMessageActionSheet({ + showActionSheetWithOptions, + callbacks: { dispatch, startEditMessage, _ }, + backgroundData, + message, + narrow, + }); + } }; export const handleWebViewOutboundEvent = (