From f08a4e00cb5c43216adc9a470fbf52504dedbaae Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Thu, 18 Mar 2021 15:05:40 +0800 Subject: [PATCH 01/11] ActionSheets: Split showActionSheet into multiple functions This splits showActionSheet into a showHeaderActionSheet and a showMessageActionSheet function, since the two of them conceptually require different arguments and do different things. Right now, they take the same set of arguments, but this commit sets us up to change that in the future, and tighten up the typing more. --- src/message/messageActionSheet.js | 49 ++++++++++++++++++++++------- src/webview/handleOutboundEvents.js | 21 ++++++++----- 2 files changed, 52 insertions(+), 18 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index 81d93817fb5..b45079b0fbc 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -352,9 +352,7 @@ const getActionSheetTitle = (message: Message | Outbox, ownUser: User): string = } }; -/** Invoke the given callback to show an appropriate action sheet. */ -export const showActionSheet = ( - isHeader: boolean, +export const showMessageActionSheet = ( showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, @@ -363,9 +361,7 @@ export const showActionSheet = ( |}, params: ConstructSheetParams<>, ): void => { - const optionCodes = isHeader - ? constructHeaderActionButtons(params) - : constructNonHeaderActionButtons(params); + const optionCodes = constructNonHeaderActionButtons(params); const callback = buttonIndex => { (async () => { const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; @@ -384,11 +380,42 @@ export const showActionSheet = ( }; showActionSheetWithOptions( { - ...(isHeader - ? { - title: getActionSheetTitle(params.message, params.backgroundData.ownUser), - } - : {}), + options: optionCodes.map(code => callbacks._(allButtons[code].title)), + cancelButtonIndex: optionCodes.length - 1, + }, + callback, + ); +}; + +export const showHeaderActionSheet = ( + showActionSheetWithOptions: ShowActionSheetWithOptions, + callbacks: {| + dispatch: Dispatch, + startEditMessage: (editMessage: EditMessage) => void, + _: GetText, + |}, + params: ConstructSheetParams<>, +): void => { + const optionCodes = constructHeaderActionButtons(params); + const callback = buttonIndex => { + (async () => { + const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; + try { + await pressedButton({ + subscriptions: params.backgroundData.subscriptions, + auth: params.backgroundData.auth, + ownUser: params.backgroundData.ownUser, + ...params, + ...callbacks, + }); + } catch (err) { + Alert.alert(callbacks._(pressedButton.errorMessage), err.message); + } + })(); + }; + showActionSheetWithOptions( + { + title: getActionSheetTitle(params.message, params.backgroundData.ownUser), options: optionCodes.map(code => callbacks._(allButtons[code].title)), cancelButtonIndex: optionCodes.length - 1, }, diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index e6e0b51a63b..53ebf1dbd37 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -22,7 +22,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 +211,19 @@ const handleLongPress = ( return; } const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props; - showActionSheet( - target === 'header', - showActionSheetWithOptions, - { dispatch, startEditMessage, _ }, - { backgroundData, message, narrow }, - ); + if (target === 'header') { + showHeaderActionSheet( + showActionSheetWithOptions, + { dispatch, startEditMessage, _ }, + { backgroundData, message, narrow }, + ); + } else if (target === 'message') { + showMessageActionSheet( + showActionSheetWithOptions, + { dispatch, startEditMessage, _ }, + { backgroundData, message, narrow }, + ); + } }; export const handleWebViewOutboundEvent = ( From de31a4151d53f843c989026dfe420f1953d1e1cb Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Sat, 20 Mar 2021 00:56:50 +0800 Subject: [PATCH 02/11] ActionSheets: Refactor show/construct function types This changes the: * constructHeaderActionButtons * constructNonHeaderActionButtons * constructMessageActionButtons * constructOutboxActionButtons * showHeaderActionSheet * showMessageActionSheet functions in a few ways: * They no longer explicitly take a `BackgroundData` object, but instead take an inexact object with just the parameters that each function uses. * The show*ActionSheet functions have been converted to use named arguments via an object. --- src/message/messageActionSheet.js | 106 +++++++++++++++++++--------- src/webview/handleOutboundEvents.js | 20 +++--- 2 files changed, 85 insertions(+), 41 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index b45079b0fbc..ebc996e7c81 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -6,15 +6,16 @@ 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, @@ -241,16 +242,20 @@ 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[] => { + narrow, +}: {| + backgroundData: $ReadOnly<{ + mute: MuteState, + subscriptions: Subscription[], + ownUser: User, + ... + }>, + message: Message | Outbox, + narrow: Narrow, +|}): ButtonCode[] => { const buttons: ButtonCode[] = []; if (message.type === 'stream') { if (ownUser.is_admin) { @@ -273,11 +278,7 @@ export const constructHeaderActionButtons = ({ return buttons; }; -export const constructOutboxActionButtons = ({ - backgroundData, - message, - narrow, -}: ConstructSheetParams): ButtonCode[] => { +export const constructOutboxActionButtons = (): ButtonCode[] => { const buttons = []; buttons.push('copyToClipboard'); buttons.push('shareMessage'); @@ -293,7 +294,14 @@ export const constructMessageActionButtons = ({ backgroundData: { ownUser, flags }, message, narrow, -}: ConstructSheetParams): ButtonCode[] => { +}: { + backgroundData: $ReadOnly<{ + ownUser: User, + flags: FlagsState, + }>, + message: Message, + narrow: Narrow, +}): ButtonCode[] => { const buttons = []; if (messageNotDeleted(message)) { buttons.push('addReaction'); @@ -331,9 +339,17 @@ export const constructNonHeaderActionButtons = ({ backgroundData, message, narrow, -}: ConstructSheetParams<>): ButtonCode[] => { +}: {| + backgroundData: $ReadOnly<{ + ownUser: User, + flags: FlagsState, + ... + }>, + message: Message | Outbox, + narrow: Narrow, +|}): ButtonCode[] => { if (message.isOutbox) { - return constructOutboxActionButtons({ backgroundData, message, narrow }); + return constructOutboxActionButtons(); } else { return constructMessageActionButtons({ backgroundData, message, narrow }); } @@ -352,26 +368,38 @@ const getActionSheetTitle = (message: Message | Outbox, ownUser: User): string = } }; -export const showMessageActionSheet = ( +export const showMessageActionSheet = ({ + showActionSheetWithOptions, + callbacks, + backgroundData, + message, + narrow, +}: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, startEditMessage: (editMessage: EditMessage) => void, _: GetText, |}, - params: ConstructSheetParams<>, -): void => { - const optionCodes = constructNonHeaderActionButtons(params); + backgroundData: $ReadOnly<{ + auth: Auth, + subscriptions: Subscription[], + ownUser: User, + flags: FlagsState, + }>, + message: Message | Outbox, + narrow: Narrow, +|}): void => { + const optionCodes = constructNonHeaderActionButtons({ backgroundData, message, narrow }); const callback = buttonIndex => { (async () => { const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; try { await pressedButton({ - subscriptions: params.backgroundData.subscriptions, - auth: params.backgroundData.auth, - ownUser: params.backgroundData.ownUser, - ...params, + ...backgroundData, ...callbacks, + message, + narrow, }); } catch (err) { Alert.alert(callbacks._(pressedButton.errorMessage), err.message); @@ -387,26 +415,38 @@ export const showMessageActionSheet = ( ); }; -export const showHeaderActionSheet = ( +export const showHeaderActionSheet = ({ + showActionSheetWithOptions, + callbacks, + backgroundData, + message, + narrow, +}: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, startEditMessage: (editMessage: EditMessage) => void, _: GetText, |}, - params: ConstructSheetParams<>, -): void => { - const optionCodes = constructHeaderActionButtons(params); + backgroundData: $ReadOnly<{ + auth: Auth, + mute: MuteState, + subscriptions: Subscription[], + ownUser: User, + flags: FlagsState, + }>, + message: Message | Outbox, + narrow: Narrow, +|}): void => { + const optionCodes = constructHeaderActionButtons({ backgroundData, message, narrow }); const callback = buttonIndex => { (async () => { const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; try { await pressedButton({ - subscriptions: params.backgroundData.subscriptions, - auth: params.backgroundData.auth, - ownUser: params.backgroundData.ownUser, - ...params, + ...backgroundData, ...callbacks, + message, }); } catch (err) { Alert.alert(callbacks._(pressedButton.errorMessage), err.message); @@ -415,7 +455,7 @@ export const showHeaderActionSheet = ( }; showActionSheetWithOptions( { - title: getActionSheetTitle(params.message, params.backgroundData.ownUser), + title: getActionSheetTitle(message, backgroundData.ownUser), options: optionCodes.map(code => callbacks._(allButtons[code].title)), cancelButtonIndex: optionCodes.length - 1, }, diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index 53ebf1dbd37..d5f6ad4c29a 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -212,17 +212,21 @@ const handleLongPress = ( } const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props; if (target === 'header') { - showHeaderActionSheet( + showHeaderActionSheet({ showActionSheetWithOptions, - { dispatch, startEditMessage, _ }, - { backgroundData, message, narrow }, - ); + callbacks: { dispatch, startEditMessage, _ }, + backgroundData, + message, + narrow, + }); } else if (target === 'message') { - showMessageActionSheet( + showMessageActionSheet({ showActionSheetWithOptions, - { dispatch, startEditMessage, _ }, - { backgroundData, message, narrow }, - ); + callbacks: { dispatch, startEditMessage, _ }, + backgroundData, + message, + narrow, + }); } }; From 86f3f2ccb0fcfa8e79b7b982a6d76151a0d67fb5 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Sat, 20 Mar 2021 00:57:36 +0800 Subject: [PATCH 03/11] ActionSheets: Remove narrow arg from showHeaderActionSheet --- src/message/__tests__/messageActionSheet-test.js | 8 -------- src/message/messageActionSheet.js | 6 +----- src/webview/handleOutboundEvents.js | 1 - 3 files changed, 1 insertion(+), 14 deletions(-) diff --git a/src/message/__tests__/messageActionSheet-test.js b/src/message/__tests__/messageActionSheet-test.js index d6f6b9f463b..63167838337 100644 --- a/src/message/__tests__/messageActionSheet-test.js +++ b/src/message/__tests__/messageActionSheet-test.js @@ -54,8 +54,6 @@ describe('constructActionButtons', () => { }); 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({ @@ -65,7 +63,6 @@ describe('constructHeaderActionButtons', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, mute }, message, - narrow, }); expect(buttons).toContain('unmuteTopic'); }); @@ -74,7 +71,6 @@ describe('constructHeaderActionButtons', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, mute: [] }, message: eg.streamMessage(), - narrow, }); expect(buttons).toContain('muteTopic'); }); @@ -84,7 +80,6 @@ describe('constructHeaderActionButtons', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, subscriptions }, message: eg.streamMessage(), - narrow, }); expect(buttons).toContain('unmuteStream'); }); @@ -94,7 +89,6 @@ describe('constructHeaderActionButtons', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, subscriptions }, message: eg.streamMessage(), - narrow, }); expect(buttons).toContain('muteStream'); }); @@ -104,7 +98,6 @@ describe('constructHeaderActionButtons', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, ownUser }, message: eg.streamMessage(), - narrow, }); expect(buttons).toContain('deleteTopic'); }); @@ -113,7 +106,6 @@ describe('constructHeaderActionButtons', () => { const buttons = constructHeaderActionButtons({ backgroundData: baseBackgroundData, message: eg.streamMessage(), - narrow, }); expect(buttons).not.toContain('deleteTopic'); }); diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index ebc996e7c81..6760dfd7869 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -245,7 +245,6 @@ const allButtons: {| [ButtonCode]: ButtonDescription |} = allButtonsRaw; export const constructHeaderActionButtons = ({ backgroundData: { mute, subscriptions, ownUser }, message, - narrow, }: {| backgroundData: $ReadOnly<{ mute: MuteState, @@ -254,7 +253,6 @@ export const constructHeaderActionButtons = ({ ... }>, message: Message | Outbox, - narrow: Narrow, |}): ButtonCode[] => { const buttons: ButtonCode[] = []; if (message.type === 'stream') { @@ -420,7 +418,6 @@ export const showHeaderActionSheet = ({ callbacks, backgroundData, message, - narrow, }: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| @@ -436,9 +433,8 @@ export const showHeaderActionSheet = ({ flags: FlagsState, }>, message: Message | Outbox, - narrow: Narrow, |}): void => { - const optionCodes = constructHeaderActionButtons({ backgroundData, message, narrow }); + const optionCodes = constructHeaderActionButtons({ backgroundData, message }); const callback = buttonIndex => { (async () => { const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index d5f6ad4c29a..5ebd851033f 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -217,7 +217,6 @@ const handleLongPress = ( callbacks: { dispatch, startEditMessage, _ }, backgroundData, message, - narrow, }); } else if (target === 'message') { showMessageActionSheet({ From 4e990a005e38c3e2d2629d5b87c7761f91de6b28 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Thu, 18 Mar 2021 17:44:40 +0800 Subject: [PATCH 04/11] ActionSheets: Remove ButtonCode, pass around objects instead This removes the ButtonCode infrastructure, and instead has the code for constructing button lists pass around ButtonDescription objects. This will enable us to split `ButtonDescription` into a `HeaderButton` and `MessageButton` type (or a `Button` type, more likely), and have each of those take only the arguments it needs - for instance, we can avoid passing `startEditMessage` callback to a `Button
`, since we know that'll never be used. This also lets us avoid keeping around a global list of all possible buttons, while still being able to share button code across multiple actionsheets in a typesafe way (for instance, the cancel button). The main downside is that our tests now rely on the `title` of the button to check if the correct buttons have been inserted, since we no longer have a `ButtonCode` to compare. At first glance, this seems bad, but I think it's actually OK: * We apply i18n in the rendering step, so we don't need to worry about the strings being translated - there are no calls to `_` in the codepath that the tests are checking. * We're not really much worse off than we were before - previously, changing the name of a button in the code would necessitate changing the tests, now changing the title will require that. While a title change is probably more common, I don't think we're at any increased risk of "stringly typed" typo bugs - if we change a title, the tests will start failing and we'll need to go through and patch it up, but none of the non-test logic relies on the button titles. --- .../__tests__/messageActionSheet-test.js | 20 ++-- src/message/messageActionSheet.js | 97 +++++++------------ 2 files changed, 44 insertions(+), 73 deletions(-) diff --git a/src/message/__tests__/messageActionSheet-test.js b/src/message/__tests__/messageActionSheet-test.js index 63167838337..1902106f7a4 100644 --- a/src/message/__tests__/messageActionSheet-test.js +++ b/src/message/__tests__/messageActionSheet-test.js @@ -18,6 +18,8 @@ const baseBackgroundData = deepFreeze({ twentyFourHourTime: false, }); +const buttonTitles = buttons => buttons.map(button => button.title); + describe('constructActionButtons', () => { const narrow = deepFreeze(HOME_NARROW); @@ -29,7 +31,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 +42,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,7 +51,7 @@ describe('constructActionButtons', () => { message: eg.streamMessage({ reactions: [eg.unicodeEmojiReaction] }), narrow, }); - expect(buttons).toContain('showReactions'); + expect(buttonTitles(buttons)).toContain('See who reacted'); }); }); @@ -64,7 +66,7 @@ describe('constructHeaderActionButtons', () => { backgroundData: { ...baseBackgroundData, mute }, message, }); - expect(buttons).toContain('unmuteTopic'); + expect(buttonTitles(buttons)).toContain('Unmute topic'); }); test('show mute topic option if topic is not muted', () => { @@ -72,7 +74,7 @@ describe('constructHeaderActionButtons', () => { backgroundData: { ...baseBackgroundData, mute: [] }, message: eg.streamMessage(), }); - expect(buttons).toContain('muteTopic'); + expect(buttonTitles(buttons)).toContain('Mute topic'); }); test('show Unmute stream option if stream is not in home view', () => { @@ -81,7 +83,7 @@ describe('constructHeaderActionButtons', () => { backgroundData: { ...baseBackgroundData, subscriptions }, message: eg.streamMessage(), }); - expect(buttons).toContain('unmuteStream'); + expect(buttonTitles(buttons)).toContain('Unmute stream'); }); test('show mute stream option if stream is in home view', () => { @@ -90,7 +92,7 @@ describe('constructHeaderActionButtons', () => { backgroundData: { ...baseBackgroundData, subscriptions }, message: eg.streamMessage(), }); - expect(buttons).toContain('muteStream'); + expect(buttonTitles(buttons)).toContain('Mute stream'); }); test('show delete topic option if current user is an admin', () => { @@ -99,7 +101,7 @@ describe('constructHeaderActionButtons', () => { backgroundData: { ...baseBackgroundData, ownUser }, message: eg.streamMessage(), }); - expect(buttons).toContain('deleteTopic'); + expect(buttonTitles(buttons)).toContain('Delete topic'); }); test('do not show delete topic option if current user is not an admin', () => { @@ -107,6 +109,6 @@ describe('constructHeaderActionButtons', () => { backgroundData: baseBackgroundData, message: eg.streamMessage(), }); - 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 6760dfd7869..b0ba9e5449b 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -211,37 +211,6 @@ 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; - export const constructHeaderActionButtons = ({ backgroundData: { mute, subscriptions, ownUser }, message, @@ -253,35 +222,35 @@ export const constructHeaderActionButtons = ({ ... }>, message: Message | Outbox, -|}): ButtonCode[] => { - const buttons: ButtonCode[] = []; +|}): ButtonDescription[] => { + const buttons = []; if (message.type === 'stream') { if (ownUser.is_admin) { - buttons.push('deleteTopic'); + buttons.push(deleteTopic); } const streamName = streamNameOfStreamMessage(message); if (isTopicMuted(streamName, message.subject, mute)) { - buttons.push('unmuteTopic'); + buttons.push(unmuteTopic); } else { - buttons.push('muteTopic'); + buttons.push(muteTopic); } const sub = subscriptions.find(x => x.name === streamName); if (sub && !sub.in_home_view) { - buttons.push('unmuteStream'); + buttons.push(unmuteStream); } else { - buttons.push('muteStream'); + buttons.push(muteStream); } } - buttons.push('cancel'); + buttons.push(cancel); return buttons; }; -export const constructOutboxActionButtons = (): ButtonCode[] => { +export const constructOutboxActionButtons = (): ButtonDescription[] => { 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; }; @@ -299,37 +268,37 @@ export const constructMessageActionButtons = ({ }>, message: Message, narrow: Narrow, -}): ButtonCode[] => { +}): ButtonDescription[] => { 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; }; @@ -345,7 +314,7 @@ export const constructNonHeaderActionButtons = ({ }>, message: Message | Outbox, narrow: Narrow, -|}): ButtonCode[] => { +|}): ButtonDescription[] => { if (message.isOutbox) { return constructOutboxActionButtons(); } else { @@ -388,10 +357,10 @@ export const showMessageActionSheet = ({ message: Message | Outbox, narrow: Narrow, |}): void => { - const optionCodes = constructNonHeaderActionButtons({ backgroundData, message, narrow }); + const buttonList = constructNonHeaderActionButtons({ backgroundData, message, narrow }); const callback = buttonIndex => { (async () => { - const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; + const pressedButton: ButtonDescription = buttonList[buttonIndex]; try { await pressedButton({ ...backgroundData, @@ -406,8 +375,8 @@ export const showMessageActionSheet = ({ }; showActionSheetWithOptions( { - options: optionCodes.map(code => callbacks._(allButtons[code].title)), - cancelButtonIndex: optionCodes.length - 1, + options: buttonList.map(button => callbacks._(button.title)), + cancelButtonIndex: buttonList.length - 1, }, callback, ); @@ -434,10 +403,10 @@ export const showHeaderActionSheet = ({ }>, message: Message | Outbox, |}): void => { - const optionCodes = constructHeaderActionButtons({ backgroundData, message }); + const buttonList = constructHeaderActionButtons({ backgroundData, message }); const callback = buttonIndex => { (async () => { - const pressedButton: ButtonDescription = allButtons[optionCodes[buttonIndex]]; + const pressedButton: ButtonDescription = buttonList[buttonIndex]; try { await pressedButton({ ...backgroundData, @@ -452,8 +421,8 @@ export const showHeaderActionSheet = ({ showActionSheetWithOptions( { title: getActionSheetTitle(message, backgroundData.ownUser), - options: optionCodes.map(code => callbacks._(allButtons[code].title)), - cancelButtonIndex: optionCodes.length - 1, + options: buttonList.map(button => callbacks._(button.title)), + cancelButtonIndex: buttonList.length - 1, }, callback, ); From c642bde06a5c36b6b72d528718dd43dbdf4478c8 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Thu, 18 Mar 2021 17:59:18 +0800 Subject: [PATCH 05/11] ActionSheets: Change ButtonDescription type to be more specific This commit replaces the `ButtonDescription` type with a `Button` type, which will allow us to in the future just pass in the required arguments for each type of button (so that, for instance, a `Button` doesn't need to require a `startEditMessage` callback). --- src/message/messageActionSheet.js | 49 ++++++++++++++++++------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index b0ba9e5449b..56b25cea305 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -37,27 +37,34 @@ 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, + message: Message | Outbox, + 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: ... @@ -222,7 +229,7 @@ export const constructHeaderActionButtons = ({ ... }>, message: Message | Outbox, -|}): ButtonDescription[] => { +|}): Button[] => { const buttons = []; if (message.type === 'stream') { if (ownUser.is_admin) { @@ -245,7 +252,7 @@ export const constructHeaderActionButtons = ({ return buttons; }; -export const constructOutboxActionButtons = (): ButtonDescription[] => { +export const constructOutboxActionButtons = (): Button[] => { const buttons = []; buttons.push(copyToClipboard); buttons.push(shareMessage); @@ -268,7 +275,7 @@ export const constructMessageActionButtons = ({ }>, message: Message, narrow: Narrow, -}): ButtonDescription[] => { +}): Button[] => { const buttons = []; if (messageNotDeleted(message)) { buttons.push(addReaction); @@ -314,7 +321,7 @@ export const constructNonHeaderActionButtons = ({ }>, message: Message | Outbox, narrow: Narrow, -|}): ButtonDescription[] => { +|}): Button[] => { if (message.isOutbox) { return constructOutboxActionButtons(); } else { @@ -360,7 +367,7 @@ export const showMessageActionSheet = ({ const buttonList = constructNonHeaderActionButtons({ backgroundData, message, narrow }); const callback = buttonIndex => { (async () => { - const pressedButton: ButtonDescription = buttonList[buttonIndex]; + const pressedButton: Button = buttonList[buttonIndex]; try { await pressedButton({ ...backgroundData, @@ -406,7 +413,7 @@ export const showHeaderActionSheet = ({ const buttonList = constructHeaderActionButtons({ backgroundData, message }); const callback = buttonIndex => { (async () => { - const pressedButton: ButtonDescription = buttonList[buttonIndex]; + const pressedButton: Button = buttonList[buttonIndex]; try { await pressedButton({ ...backgroundData, From a555e2aa8ed887054bdf8f9b49929aad0b365380 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Fri, 19 Mar 2021 22:43:46 +0800 Subject: [PATCH 06/11] webview: Don't show actionsheet for PM header long-press Previously, long-pressing a PM header in the "All Messages" (or similar) view would pop up a actionsheet, which only showed the name of the PM and a "Cancel" button. This was pretty silly, and makes the ActionSheet code more complicated than it needs to be, so we'll just pop up a toast showing the name of the PM instead. This is useful in the case of a PM where the title is long enough that it's truncated in the UI. The code for the toast text is taken from `getActionSheetTitle` in src/message/messageActionSheet.js - it's duplicated in this commit for type safety reasons, but the duplication will be removed in the next commit. --- src/webview/handleOutboundEvents.js | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index 5ebd851033f..dddefbd8132 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 { pmUiRecipientsFromMessage } from '../utils/recipient'; import { isUrlAnImage } from '../utils/url'; import * as logging from '../utils/logging'; import { filterUnreadMessagesInRange } from '../utils/unread'; @@ -212,12 +213,20 @@ const handleLongPress = ( } const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props; if (target === 'header') { - showHeaderActionSheet({ - showActionSheetWithOptions, - callbacks: { dispatch, startEditMessage, _ }, - backgroundData, - message, - }); + if (message.type === 'stream') { + showHeaderActionSheet({ + showActionSheetWithOptions, + callbacks: { dispatch, startEditMessage, _ }, + backgroundData, + message, + }); + } 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, From d7a7e939bc1341540725dbd55e995f95d9d37383 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Fri, 19 Mar 2021 22:48:48 +0800 Subject: [PATCH 07/11] 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. --- .../__tests__/messageActionSheet-test.js | 23 ++++---- src/message/messageActionSheet.js | 59 ++++++++----------- 2 files changed, 38 insertions(+), 44 deletions(-) diff --git a/src/message/__tests__/messageActionSheet-test.js b/src/message/__tests__/messageActionSheet-test.js index 1902106f7a4..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'; @@ -58,13 +59,10 @@ describe('constructActionButtons', () => { describe('constructHeaderActionButtons', () => { 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, + stream: 'electron issues', + topic: 'issue #556', }); expect(buttonTitles(buttons)).toContain('Unmute topic'); }); @@ -72,7 +70,8 @@ describe('constructHeaderActionButtons', () => { test('show mute topic option if topic is not muted', () => { const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, mute: [] }, - message: eg.streamMessage(), + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); expect(buttonTitles(buttons)).toContain('Mute topic'); }); @@ -81,7 +80,8 @@ describe('constructHeaderActionButtons', () => { const subscriptions = [{ ...eg.subscription, in_home_view: false }]; const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, subscriptions }, - message: eg.streamMessage(), + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); expect(buttonTitles(buttons)).toContain('Unmute stream'); }); @@ -90,7 +90,8 @@ describe('constructHeaderActionButtons', () => { const subscriptions = [{ ...eg.subscription, in_home_view: true }]; const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, subscriptions }, - message: eg.streamMessage(), + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); expect(buttonTitles(buttons)).toContain('Mute stream'); }); @@ -99,7 +100,8 @@ describe('constructHeaderActionButtons', () => { const ownUser = { ...eg.selfUser, is_admin: true }; const buttons = constructHeaderActionButtons({ backgroundData: { ...baseBackgroundData, ownUser }, - message: eg.streamMessage(), + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); expect(buttonTitles(buttons)).toContain('Delete topic'); }); @@ -107,7 +109,8 @@ describe('constructHeaderActionButtons', () => { test('do not show delete topic option if current user is not an admin', () => { const buttons = constructHeaderActionButtons({ backgroundData: baseBackgroundData, - message: eg.streamMessage(), + stream: streamNameOfStreamMessage(eg.streamMessage()), + topic: eg.streamMessage().subject, }); expect(buttonTitles(buttons)).not.toContain('Delete topic'); }); diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index 56b25cea305..56b78ae1503 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -27,7 +27,7 @@ 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 { streamNameOfStreamMessage } from '../utils/recipient'; import { deleteMessagesForTopic } from '../topics/topicActions'; import * as logging from '../utils/logging'; @@ -220,7 +220,8 @@ cancel.errorMessage = 'Failed to hide menu'; export const constructHeaderActionButtons = ({ backgroundData: { mute, subscriptions, ownUser }, - message, + stream, + topic, }: {| backgroundData: $ReadOnly<{ mute: MuteState, @@ -228,25 +229,23 @@ export const constructHeaderActionButtons = ({ ownUser: User, ... }>, - message: Message | Outbox, + stream: string, + topic: string, |}): Button[] => { const buttons = []; - 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); - } + 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); return buttons; @@ -329,19 +328,6 @@ export const constructNonHeaderActionButtons = ({ } }; -/** 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, @@ -410,7 +396,12 @@ export const showHeaderActionSheet = ({ }>, message: Message | Outbox, |}): void => { - const buttonList = constructHeaderActionButtons({ backgroundData, message }); + invariant(message.type === 'stream', 'showHeaderActionSheet: got PM'); + const buttonList = constructHeaderActionButtons({ + backgroundData, + stream: streamNameOfStreamMessage(message), + topic: message.subject, + }); const callback = buttonIndex => { (async () => { const pressedButton: Button = buttonList[buttonIndex]; @@ -427,7 +418,7 @@ export const showHeaderActionSheet = ({ }; showActionSheetWithOptions( { - title: getActionSheetTitle(message, backgroundData.ownUser), + title: `#${streamNameOfStreamMessage(message)} > ${message.subject}`, options: buttonList.map(button => callbacks._(button.title)), cancelButtonIndex: buttonList.length - 1, }, From 8a02d90552d61d115df2e49fe00bec0b004e91c8 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Thu, 18 Mar 2021 22:13:57 +0800 Subject: [PATCH 08/11] ActionSheets: Change Button callback to take stream and topic This changes the Button callback to take a stream and topic, rather than a message, since the only thing the callbacks need is the stream and topic. --- src/message/messageActionSheet.js | 35 +++++++++++++------------------ 1 file changed, 15 insertions(+), 20 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index 56b78ae1503..77f0ed0bb60 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -39,7 +39,8 @@ export type ShowActionSheetWithOptions = ( type HeaderArgs = { auth: Auth, - message: Message | Outbox, + stream: string, + topic: string, subscriptions: Subscription[], dispatch: Dispatch, _: GetText, @@ -112,25 +113,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( @@ -156,15 +152,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); } @@ -172,9 +167,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); } @@ -409,7 +403,8 @@ export const showHeaderActionSheet = ({ await pressedButton({ ...backgroundData, ...callbacks, - message, + stream: streamNameOfStreamMessage(message), + topic: message.subject, }); } catch (err) { Alert.alert(callbacks._(pressedButton.errorMessage), err.message); From 687e4146a6525b24cc109e5fd2e4d4f64b622313 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Thu, 18 Mar 2021 22:30:53 +0800 Subject: [PATCH 09/11] ActionSheets: Remove message arg from showHeaderActionSheet showHeaderActionSheet now takes a stream and topic, rather than a message, since we only used the stream and topic anyways. --- src/message/messageActionSheet.js | 19 +++++++++---------- src/webview/handleOutboundEvents.js | 5 +++-- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index 77f0ed0bb60..e967f857bbb 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -1,5 +1,4 @@ /* @flow strict-local */ -import invariant from 'invariant'; import { Clipboard, Share, Alert } from 'react-native'; import * as NavigationService from '../nav/NavigationService'; @@ -27,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 { streamNameOfStreamMessage } from '../utils/recipient'; import { deleteMessagesForTopic } from '../topics/topicActions'; import * as logging from '../utils/logging'; @@ -373,7 +371,8 @@ export const showHeaderActionSheet = ({ showActionSheetWithOptions, callbacks, backgroundData, - message, + topic, + stream, }: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| @@ -388,13 +387,13 @@ export const showHeaderActionSheet = ({ ownUser: User, flags: FlagsState, }>, - message: Message | Outbox, + stream: string, + topic: string, |}): void => { - invariant(message.type === 'stream', 'showHeaderActionSheet: got PM'); const buttonList = constructHeaderActionButtons({ backgroundData, - stream: streamNameOfStreamMessage(message), - topic: message.subject, + stream, + topic, }); const callback = buttonIndex => { (async () => { @@ -403,8 +402,8 @@ export const showHeaderActionSheet = ({ await pressedButton({ ...backgroundData, ...callbacks, - stream: streamNameOfStreamMessage(message), - topic: message.subject, + stream, + topic, }); } catch (err) { Alert.alert(callbacks._(pressedButton.errorMessage), err.message); @@ -413,7 +412,7 @@ export const showHeaderActionSheet = ({ }; showActionSheetWithOptions( { - title: `#${streamNameOfStreamMessage(message)} > ${message.subject}`, + title: `#${stream} > ${topic}`, options: buttonList.map(button => callbacks._(button.title)), cancelButtonIndex: buttonList.length - 1, }, diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index dddefbd8132..416b7ee10c1 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -9,7 +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 { pmUiRecipientsFromMessage } from '../utils/recipient'; +import { streamNameOfStreamMessage, pmUiRecipientsFromMessage } from '../utils/recipient'; import { isUrlAnImage } from '../utils/url'; import * as logging from '../utils/logging'; import { filterUnreadMessagesInRange } from '../utils/unread'; @@ -218,7 +218,8 @@ const handleLongPress = ( showActionSheetWithOptions, callbacks: { dispatch, startEditMessage, _ }, backgroundData, - message, + stream: streamNameOfStreamMessage(message), + topic: message.subject, }); } else if (message.type === 'private') { const label = pmUiRecipientsFromMessage(message, backgroundData.ownUser.user_id) From 7dbc12a191cdecf0e5d970829dff4fbfec9c2797 Mon Sep 17 00:00:00 2001 From: Wesley Aptekar-Cassels Date: Thu, 18 Mar 2021 22:38:27 +0800 Subject: [PATCH 10/11] ActionSheets: Remove startEditMessage from showHeaderActionSheet --- src/message/messageActionSheet.js | 1 - src/webview/handleOutboundEvents.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index e967f857bbb..ba5aa740205 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -377,7 +377,6 @@ export const showHeaderActionSheet = ({ showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, - startEditMessage: (editMessage: EditMessage) => void, _: GetText, |}, backgroundData: $ReadOnly<{ diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index 416b7ee10c1..fdbd8b5a9c7 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -216,7 +216,7 @@ const handleLongPress = ( if (message.type === 'stream') { showHeaderActionSheet({ showActionSheetWithOptions, - callbacks: { dispatch, startEditMessage, _ }, + callbacks: { dispatch, _ }, backgroundData, stream: streamNameOfStreamMessage(message), topic: message.subject, From 96ef22d3d3df4e9e2bf14ecfc3201af2ae7286b2 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Fri, 26 Mar 2021 15:38:36 -0700 Subject: [PATCH 11/11] action sheets [nfc]: Make some types explicitly inexact. It's intentional that these "background data" types are inexact object types: they're meant to be passed values that may have a larger menu of different pieces of background data (in particular values of the BackgroundData type defined in MessageList.js), and these types just specify the particular pieces of data that the particular function requires. --- src/message/messageActionSheet.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/message/messageActionSheet.js b/src/message/messageActionSheet.js index ba5aa740205..264ab114e37 100644 --- a/src/message/messageActionSheet.js +++ b/src/message/messageActionSheet.js @@ -263,6 +263,7 @@ export const constructMessageActionButtons = ({ backgroundData: $ReadOnly<{ ownUser: User, flags: FlagsState, + ... }>, message: Message, narrow: Narrow, @@ -338,6 +339,7 @@ export const showMessageActionSheet = ({ subscriptions: Subscription[], ownUser: User, flags: FlagsState, + ... }>, message: Message | Outbox, narrow: Narrow, @@ -385,6 +387,7 @@ export const showHeaderActionSheet = ({ subscriptions: Subscription[], ownUser: User, flags: FlagsState, + ... }>, stream: string, topic: string,