From 5cc72d42575a5cca9691a7981f787ec4427a1e6e Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Thu, 11 Aug 2022 14:50:58 -0700 Subject: [PATCH 1/3] api: Add getSingleMessage binding for GET messages/{message_id} We'll use this for #5306; see the plan in discussion: https://chat.zulip.org/#narrow/stream/243-mobile-team/topic/.23M5306.20Follow.20.2Fnear.2F.20links.20through.20topic.20moves.2Frenames/near/1407930 In particular, we want the stream and topic for a stream message that might not be in our data structures. We'll use this endpoint to fetch that information. topic edit modal [nfc]: Add TopicModalProvider context component. Contains visibility context and handler callback context. Sets up context for modal handler to be called inside topic action sheets. topic edit modal [nfc]: Provide topic modal Context hook to children. The useTopicModalHandler is called normally in TopicItem and TitleStream. In order to deliver the callbacks to the action sheets in MessageList, the context hook is called in ChatScreen and a bit of prop-drilling is performed. topic edit modal: Add translation for action sheet button. topic edit modal: Add modal and functionality to perform topic name updates. Fixes #5365 topid edit modal [nfc]: Revise Flow types for relevant components. topic edit modal: Modify webview unit tests to accommodate feature update. --- src/ZulipMobile.js | 9 +- src/action-sheets/index.js | 21 +++ src/api/index.js | 2 + src/api/messages/getSingleMessage.js | 55 ++++++ src/boot/TopicModalProvider.js | 92 ++++++++++ src/chat/ChatScreen.js | 3 + src/search/MessageListWrapper.js | 34 ++++ src/search/SearchMessagesCard.js | 19 +- src/streams/TopicItem.js | 4 +- src/title/TitleStream.js | 4 +- src/topics/TopicEditModal.js | 172 ++++++++++++++++++ src/webview/MessageList.js | 9 +- .../__tests__/generateInboundEvents-test.js | 1 + src/webview/handleOutboundEvents.js | 28 ++- static/translations/messages_en.json | 3 +- 15 files changed, 430 insertions(+), 26 deletions(-) create mode 100644 src/api/messages/getSingleMessage.js create mode 100644 src/boot/TopicModalProvider.js create mode 100644 src/search/MessageListWrapper.js create mode 100644 src/topics/TopicEditModal.js diff --git a/src/ZulipMobile.js b/src/ZulipMobile.js index 3de98c302f0..2c85af4ada5 100644 --- a/src/ZulipMobile.js +++ b/src/ZulipMobile.js @@ -16,6 +16,7 @@ import CompatibilityChecker from './boot/CompatibilityChecker'; import AppEventHandlers from './boot/AppEventHandlers'; import { initializeSentry } from './sentry'; import ZulipSafeAreaProvider from './boot/ZulipSafeAreaProvider'; +import TopicModalProvider from './boot/TopicModalProvider'; initializeSentry(); @@ -55,9 +56,11 @@ export default function ZulipMobile(): Node { - - - + + + + + diff --git a/src/action-sheets/index.js b/src/action-sheets/index.js index c8bc17b9e3f..687bf8dd132 100644 --- a/src/action-sheets/index.js +++ b/src/action-sheets/index.js @@ -77,6 +77,12 @@ type TopicArgs = { zulipFeatureLevel: number, dispatch: Dispatch, _: GetText, + startEditTopic: ( + streamId: number, + topic: string, + streamsById: Map, + _: GetText, + ) => Promise, ... }; @@ -251,6 +257,14 @@ const toggleResolveTopic = async ({ auth, streamId, topic, _, streams, zulipFeat }); }; +const editTopic = { + title: 'Edit topic', + errorMessage: 'Failed to resolve topic', + action: ({ streamId, topic, streams, _, startEditTopic }) => { + startEditTopic(streamId, topic, streams, _); + }, +}; + const resolveTopic = { title: 'Resolve topic', errorMessage: 'Failed to resolve topic', @@ -505,6 +519,7 @@ export const constructTopicActionButtons = (args: {| if (unreadCount > 0) { buttons.push(markTopicAsRead); } + buttons.push(editTopic); if (isTopicMuted(streamId, topic, mute)) { buttons.push(unmuteTopic); } else { @@ -666,6 +681,12 @@ export const showTopicActionSheet = (args: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, + startEditTopic: ( + streamId: number, + topic: string, + streamsById: Map, + _: GetText, + ) => Promise, _: GetText, |}, backgroundData: $ReadOnly<{ diff --git a/src/api/index.js b/src/api/index.js index be8d8f7aa6a..a84055978c8 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -30,6 +30,7 @@ import deleteMessage from './messages/deleteMessage'; import deleteTopic from './messages/deleteTopic'; import getRawMessageContent from './messages/getRawMessageContent'; import getMessages from './messages/getMessages'; +import getSingleMessage from './messages/getSingleMessage'; import getMessageHistory from './messages/getMessageHistory'; import messagesFlags from './messages/messagesFlags'; import sendMessage from './messages/sendMessage'; @@ -78,6 +79,7 @@ export { deleteTopic, getRawMessageContent, getMessages, + getSingleMessage, getMessageHistory, messagesFlags, sendMessage, diff --git a/src/api/messages/getSingleMessage.js b/src/api/messages/getSingleMessage.js new file mode 100644 index 00000000000..703652840a8 --- /dev/null +++ b/src/api/messages/getSingleMessage.js @@ -0,0 +1,55 @@ +/* @flow strict-local */ + +import type { Auth, ApiResponseSuccess } from '../transportTypes'; +import type { Message } from '../apiTypes'; +import { transformFetchedMessage, type FetchedMessage } from '../rawModelTypes'; +import { apiGet } from '../apiFetch'; +import { identityOfAuth } from '../../account/accountMisc'; + +// The actual response from the server. We convert the message to a proper +// Message before returning it to application code. +type ServerApiResponseSingleMessage = {| + ...$Exact, + -raw_content: string, // deprecated + + // Until we narrow FetchedMessage into its FL 120+ form, FetchedMessage + // will be a bit less precise than we could be here. That's because we + // only get this field from servers FL 120+. + // TODO(server-5.0): Make this field required, and remove FL-120 comment. + +message?: FetchedMessage, +|}; + +/** + * See https://zulip.com/api/get-message + * + * Gives undefined if the `message` field is missing, which it will be for + * FL <120. + */ +// TODO(server-5.0): Simplify FL-120 condition in jsdoc and implementation. +export default async ( + auth: Auth, + args: {| + +message_id: number, + |}, + + // TODO(#4659): Don't get this from callers. + zulipFeatureLevel: number, + + // TODO(#4659): Don't get this from callers? + allowEditHistory: boolean, +): Promise => { + const { message_id } = args; + const response: ServerApiResponseSingleMessage = await apiGet(auth, `messages/${message_id}`, { + apply_markdown: true, + }); + + return ( + response.message + && transformFetchedMessage( + response.message, + identityOfAuth(auth), + zulipFeatureLevel, + allowEditHistory, + ) + ); +}; diff --git a/src/boot/TopicModalProvider.js b/src/boot/TopicModalProvider.js new file mode 100644 index 00000000000..91bb0b399a3 --- /dev/null +++ b/src/boot/TopicModalProvider.js @@ -0,0 +1,92 @@ +/* @flow strict-local */ +import React, { createContext, useState, useMemo, useCallback, useContext } from 'react'; +import type { Context, Node } from 'react'; +import { useSelector } from '../react-redux'; +import TopicEditModal from '../topics/TopicEditModal'; +import type { Stream, GetText } from '../types'; +import { fetchSomeMessageIdForConversation } from '../message/fetchActions'; +import { getAuth, getZulipFeatureLevel } from '../selectors'; + +type Props = $ReadOnly<{| + children: Node, +|}>; + +type TopicModalContext = $ReadOnly<{| + startEditTopic: ( + streamId: number, + topic: string, + streamsById: Map, + _: GetText, + ) => Promise, + closeEditTopicModal: () => void, +|}>; + +// $FlowIssue[incompatible-type] +const TopicModal: Context = createContext(undefined); + +export const useTopicModalHandler = (): TopicModalContext => useContext(TopicModal); + +export default function TopicModalProvider(props: Props): Node { + const { children } = props; + const auth = useSelector(getAuth); + const zulipFeatureLevel = useSelector(getZulipFeatureLevel); + const [topicModalState, setTopicModalState] = useState({ + visible: false, + topic: '', + fetchArgs: { + auth: null, + messageId: null, + zulipFeatureLevel: null, + }, + }); + + const startEditTopic = useCallback( + async (streamId, topic, streamsById, _) => { + const messageId = await fetchSomeMessageIdForConversation( + auth, + streamId, + topic, + streamsById, + zulipFeatureLevel, + ); + if (messageId == null) { + throw new Error( + _('No messages in topic: {streamAndTopic}', { + streamAndTopic: `#${streamsById.get(streamId)?.name ?? 'unknown'} > ${topic}`, + }), + ); + } + setTopicModalState({ + visible: true, + topic, + fetchArgs: { auth, messageId, zulipFeatureLevel }, + }); + }, + [auth, zulipFeatureLevel], + ); + + const closeEditTopicModal = useCallback(() => { + setTopicModalState({ + visible: false, + topic: null, + fetchArgs: { auth: null, messageId: null, zulipFeatureLevel: null }, + }); + }, []); + + const topicModalHandler = useMemo( + () => ({ + startEditTopic, + closeEditTopicModal, + }), + [startEditTopic, closeEditTopicModal], + ); + + return ( + + {topicModalState.visible && ( + + )} + {children} + + ); +} diff --git a/src/chat/ChatScreen.js b/src/chat/ChatScreen.js index 9342c1f4f53..472e0fcd163 100644 --- a/src/chat/ChatScreen.js +++ b/src/chat/ChatScreen.js @@ -30,6 +30,7 @@ import { showErrorAlert } from '../utils/info'; import { TranslationContext } from '../boot/TranslationProvider'; import * as api from '../api'; import { useConditionalEffect } from '../reactUtils'; +import { useTopicModalHandler } from '../boot/TopicModalProvider'; type Props = $ReadOnly<{| navigation: AppNavigationProp<'chat'>, @@ -127,6 +128,7 @@ const useMessagesWithFetch = args => { export default function ChatScreen(props: Props): Node { const { route, navigation } = props; const { backgroundColor } = React.useContext(ThemeContext); + const { startEditTopic } = useTopicModalHandler(); const { narrow, editMessage } = route.params; const setEditMessage = useCallback( @@ -221,6 +223,7 @@ export default function ChatScreen(props: Props): Node { } showMessagePlaceholders={showMessagePlaceholders} startEditMessage={setEditMessage} + startEditTopic={startEditTopic} /> ); } diff --git a/src/search/MessageListWrapper.js b/src/search/MessageListWrapper.js new file mode 100644 index 00000000000..858e2e3a50f --- /dev/null +++ b/src/search/MessageListWrapper.js @@ -0,0 +1,34 @@ +/* @flow strict-local */ +import React from 'react'; +import type { Node } from 'react'; +import MessageList from '../webview/MessageList'; +import { useTopicModalHandler } from '../boot/TopicModalProvider'; +import type { Message, Narrow } from '../types'; + +type Props = $ReadOnly<{| + messages: $ReadOnlyArray, + narrow: Narrow, +|}>; + +/* We can't call Context hooks from SearchMessagesCard because it's a class component. This wrapper allows the startEditTopic callback to be passed to this particular MessageList child without breaking Rules of Hooks. */ + +export default function MessageListWrapper({ messages, narrow }: Props): Node { + const { startEditTopic } = useTopicModalHandler(); + + return ( + undefined} + startEditTopic={startEditTopic} + /> + ); +} diff --git a/src/search/SearchMessagesCard.js b/src/search/SearchMessagesCard.js index bf34f5877cb..1b649bad242 100644 --- a/src/search/SearchMessagesCard.js +++ b/src/search/SearchMessagesCard.js @@ -8,7 +8,7 @@ import type { Message, Narrow } from '../types'; import { createStyleSheet } from '../styles'; import LoadingIndicator from '../common/LoadingIndicator'; import SearchEmptyState from '../common/SearchEmptyState'; -import MessageList from '../webview/MessageList'; +import MessageListWrapper from './MessageListWrapper'; const styles = createStyleSheet({ results: { @@ -24,8 +24,7 @@ type Props = $ReadOnly<{| export default class SearchMessagesCard extends PureComponent { render(): Node { - const { isFetching, messages } = this.props; - + const { isFetching, messages, narrow } = this.props; if (isFetching) { // Display loading indicator only if there are no messages to // display from a previous search. @@ -44,19 +43,7 @@ export default class SearchMessagesCard extends PureComponent { return ( - undefined} - /> + ); } diff --git a/src/streams/TopicItem.js b/src/streams/TopicItem.js index d60e6b4f5fd..c197e3e9abb 100644 --- a/src/streams/TopicItem.js +++ b/src/streams/TopicItem.js @@ -25,6 +25,7 @@ import { import { getMute } from '../mute/muteModel'; import { getUnread } from '../unread/unreadModel'; import { getOwnUserRole } from '../permissionSelectors'; +import { useTopicModalHandler } from '../boot/TopicModalProvider'; const componentStyles = createStyleSheet({ selectedRow: { @@ -70,6 +71,7 @@ export default function TopicItem(props: Props): Node { useActionSheet().showActionSheetWithOptions; const _ = useContext(TranslationContext); const dispatch = useDispatch(); + const { startEditTopic } = useTopicModalHandler(); const backgroundData = useSelector(state => ({ auth: getAuth(state), mute: getMute(state), @@ -88,7 +90,7 @@ export default function TopicItem(props: Props): Node { onLongPress={() => { showTopicActionSheet({ showActionSheetWithOptions, - callbacks: { dispatch, _ }, + callbacks: { dispatch, startEditTopic, _ }, backgroundData, streamId, topic: name, diff --git a/src/title/TitleStream.js b/src/title/TitleStream.js index a6f4190dbfc..5ffce4e9509 100644 --- a/src/title/TitleStream.js +++ b/src/title/TitleStream.js @@ -27,6 +27,7 @@ import { showStreamActionSheet, showTopicActionSheet } from '../action-sheets'; import type { ShowActionSheetWithOptions } from '../action-sheets'; import { getUnread } from '../unread/unreadModel'; import { getOwnUserRole } from '../permissionSelectors'; +import { useTopicModalHandler } from '../boot/TopicModalProvider'; type Props = $ReadOnly<{| narrow: Narrow, @@ -67,6 +68,7 @@ export default function TitleStream(props: Props): Node { const showActionSheetWithOptions: ShowActionSheetWithOptions = useActionSheet().showActionSheetWithOptions; const _ = useContext(TranslationContext); + const { startEditTopic } = useTopicModalHandler(); return ( { showTopicActionSheet({ showActionSheetWithOptions, - callbacks: { dispatch, _ }, + callbacks: { dispatch, startEditTopic, _ }, backgroundData, streamId: stream.stream_id, topic: topicOfNarrow(narrow), diff --git a/src/topics/TopicEditModal.js b/src/topics/TopicEditModal.js new file mode 100644 index 00000000000..709bc8872a6 --- /dev/null +++ b/src/topics/TopicEditModal.js @@ -0,0 +1,172 @@ +// @flow +import React, { useState, useContext, useMemo } from 'react'; +import { Modal, Text, Pressable, View, TextInput, Platform } from 'react-native'; +import type { JSX$Element } from 'tsflower/subst/react'; +import { ThemeContext, BRAND_COLOR, createStyleSheet } from '../styles'; +import { updateMessage } from '../api'; +import type { Auth, Stream, GetText } from '../types'; + +type TopicEditModalArgs = { + topicModalState: { + visible: boolean, + topic: string, + fetchArgs: { + auth: Auth, + messageId: number, + zulipFeatureLevel: number, + }, + }, + topicModalHandler: { + startEditTopic: ( + streamId: number, + topic: string, + streamsById: Map, + _: GetText, + ) => Promise, + closeEditTopicModal: () => void, + }, +}; + +export default function TopicEditModal({ + topicModalState, + topicModalHandler, +}: TopicEditModalArgs): JSX$Element { + const { topic, fetchArgs } = topicModalState; + const [topicState, onChangeTopic] = useState(topic); + const { closeEditTopicModal } = topicModalHandler; + const { auth, messageId, zulipFeatureLevel } = fetchArgs; + + const { backgroundColor } = useContext(ThemeContext); + + const inputMarginPadding = useMemo( + () => ({ + paddingHorizontal: 8, + paddingVertical: Platform.select({ + ios: 8, + android: 2, + }), + }), + [], + ); + + const styles = createStyleSheet({ + wrapper: { + flex: 1, + justifyContent: 'center', + alignItems: 'center', + }, + modalView: { + margin: 10, + alignItems: 'center', + backgroundColor, + padding: 20, + borderStyle: 'solid', + borderWidth: 1, + borderColor: 'gray', + borderRadius: 20, + width: '90%', + }, + buttonContainer: { + display: 'flex', + flexDirection: 'row', + justifyContent: 'space-between', + width: '60%', + }, + input: { + width: '90%', + borderWidth: 1, + borderRadius: 5, + marginBottom: 16, + ...inputMarginPadding, + backgroundColor: 'white', + borderStyle: 'solid', + borderColor: 'black', + }, + button: { + position: 'relative', + justifyContent: 'center', + alignItems: 'center', + borderRadius: 32, + padding: 8, + }, + titleText: { + fontSize: 18, + lineHeight: 21, + fontWeight: 'bold', + color: BRAND_COLOR, + marginBottom: 12, + }, + text: { + fontSize: 14, + lineHeight: 21, + fontWeight: 'bold', + }, + submitButtonText: { + color: 'white', + }, + cancelButtonText: { + color: BRAND_COLOR, + }, + }); + + const handleSubmit = async () => { + await updateMessage(auth, messageId, { + propagate_mode: 'change_all', + subject: topicState, + ...(zulipFeatureLevel >= 9 && { + send_notification_to_old_thread: true, + send_notification_to_new_thread: true, + }), + }); + closeEditTopicModal(); + }; + return ( + { + closeEditTopicModal(); + }} + > + + + Edit topic + + + { + closeEditTopicModal(); + }} + > + Cancel + + + Submit + + + + + + ); +} diff --git a/src/webview/MessageList.js b/src/webview/MessageList.js index 8e6e7353560..74132f9fea9 100644 --- a/src/webview/MessageList.js +++ b/src/webview/MessageList.js @@ -14,6 +14,7 @@ import type { MessageListElement, UserOrBot, EditMessage, + Stream, } from '../types'; import { assumeSecretlyGlobalState } from '../reduxTypes'; import { connect } from '../react-redux'; @@ -45,6 +46,12 @@ type OuterProps = $ReadOnly<{| initialScrollMessageId: number | null, showMessagePlaceholders: boolean, startEditMessage: (editMessage: EditMessage) => void, + startEditTopic: ( + streamId: number, + topic: string, + streamsById: Map, + _: GetText, + ) => Promise, |}>; type SelectorProps = {| @@ -153,6 +160,7 @@ class MessageListInner extends Component { doNotMarkMessagesAsRead, _, } = this.props; + const contentHtml = messageListElementsForShownMessages .map(element => messageListElementHtml({ @@ -291,7 +299,6 @@ const MessageList: ComponentType = connect( // they should probably turn into a `connectGlobal` call. const globalSettings = getGlobalSettings(assumeSecretlyGlobalState(state)); const debug = getDebug(assumeSecretlyGlobalState(state)); - return { backgroundData: getBackgroundData(state, globalSettings, debug), fetching: getFetchingForNarrow(state, props.narrow), diff --git a/src/webview/__tests__/generateInboundEvents-test.js b/src/webview/__tests__/generateInboundEvents-test.js index e870d420522..f6db1fb1b9e 100644 --- a/src/webview/__tests__/generateInboundEvents-test.js +++ b/src/webview/__tests__/generateInboundEvents-test.js @@ -29,6 +29,7 @@ describe('generateInboundEvents', () => { narrow: HOME_NARROW, showMessagePlaceholders: false, startEditMessage: jest.fn(), + startEditTopic: jest.fn(), dispatch: jest.fn(), ...baseSelectorProps, showActionSheetWithOptions: jest.fn(), diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index c74a1592b8d..eae496f7cc6 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -4,7 +4,16 @@ import { Clipboard, Alert } from 'react-native'; import * as NavigationService from '../nav/NavigationService'; import * as api from '../api'; import config from '../config'; -import type { Dispatch, GetText, Message, Narrow, Outbox, EditMessage, UserId } from '../types'; +import type { + Dispatch, + GetText, + Message, + Narrow, + Outbox, + EditMessage, + UserId, + Stream, +} from '../types'; import type { BackgroundData } from './backgroundData'; import type { ShowActionSheetWithOptions } from '../action-sheets'; import type { JSONableDict } from '../utils/jsonable'; @@ -169,6 +178,12 @@ type Props = $ReadOnly<{ doNotMarkMessagesAsRead: boolean, showActionSheetWithOptions: ShowActionSheetWithOptions, startEditMessage: (editMessage: EditMessage) => void, + startEditTopic: ( + streamId: number, + topic: string, + streamsById: Map, + _: GetText, + ) => Promise, ... }>; @@ -222,12 +237,19 @@ const handleLongPress = ( if (!message) { return; } - const { dispatch, showActionSheetWithOptions, backgroundData, narrow, startEditMessage } = props; + const { + dispatch, + showActionSheetWithOptions, + backgroundData, + narrow, + startEditMessage, + startEditTopic, + } = props; if (target === 'header') { if (message.type === 'stream') { showTopicActionSheet({ showActionSheetWithOptions, - callbacks: { dispatch, _ }, + callbacks: { dispatch, startEditTopic, _ }, backgroundData, streamId: message.stream_id, topic: message.subject, diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index 7e508fe32f5..a6dc550672e 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -337,5 +337,6 @@ "Copy link to stream": "Copy link to stream", "Failed to copy stream link": "Failed to copy stream link", "A stream with this name already exists.": "A stream with this name already exists.", - "Streams": "Streams" + "Streams": "Streams", + "Edit topic": "Edit topic" } From 943d68ec2bd7d6cb231ed457d7ba0ba56663d09c Mon Sep 17 00:00:00 2001 From: Leslie Ngo Date: Tue, 27 Sep 2022 11:51:20 -0700 Subject: [PATCH 2/3] topic edit modal [nfc]: Rebase branch over function-component-conversion branch. --- src/search/MessageListWrapper.js | 34 ------------------- src/search/SearchMessagesCard.js | 58 ++++++++++++++++++++------------ 2 files changed, 36 insertions(+), 56 deletions(-) delete mode 100644 src/search/MessageListWrapper.js diff --git a/src/search/MessageListWrapper.js b/src/search/MessageListWrapper.js deleted file mode 100644 index 858e2e3a50f..00000000000 --- a/src/search/MessageListWrapper.js +++ /dev/null @@ -1,34 +0,0 @@ -/* @flow strict-local */ -import React from 'react'; -import type { Node } from 'react'; -import MessageList from '../webview/MessageList'; -import { useTopicModalHandler } from '../boot/TopicModalProvider'; -import type { Message, Narrow } from '../types'; - -type Props = $ReadOnly<{| - messages: $ReadOnlyArray, - narrow: Narrow, -|}>; - -/* We can't call Context hooks from SearchMessagesCard because it's a class component. This wrapper allows the startEditTopic callback to be passed to this particular MessageList child without breaking Rules of Hooks. */ - -export default function MessageListWrapper({ messages, narrow }: Props): Node { - const { startEditTopic } = useTopicModalHandler(); - - return ( - undefined} - startEditTopic={startEditTopic} - /> - ); -} diff --git a/src/search/SearchMessagesCard.js b/src/search/SearchMessagesCard.js index 1b649bad242..4d00da798a2 100644 --- a/src/search/SearchMessagesCard.js +++ b/src/search/SearchMessagesCard.js @@ -1,6 +1,6 @@ /* @flow strict-local */ -import React, { PureComponent } from 'react'; +import React from 'react'; import type { Node } from 'react'; import { View } from 'react-native'; @@ -8,7 +8,8 @@ import type { Message, Narrow } from '../types'; import { createStyleSheet } from '../styles'; import LoadingIndicator from '../common/LoadingIndicator'; import SearchEmptyState from '../common/SearchEmptyState'; -import MessageListWrapper from './MessageListWrapper'; +import MessageList from '../webview/MessageList'; +import { useTopicModalHandler } from '../boot/TopicModalProvider'; const styles = createStyleSheet({ results: { @@ -22,29 +23,42 @@ type Props = $ReadOnly<{| isFetching: boolean, |}>; -export default class SearchMessagesCard extends PureComponent { - render(): Node { - const { isFetching, messages, narrow } = this.props; - if (isFetching) { - // Display loading indicator only if there are no messages to - // display from a previous search. - if (!messages || messages.length === 0) { - return ; - } - } +export default function SearchMessagesCard(props: Props): Node { + const { narrow, isFetching, messages } = props; + const { startEditTopic } = useTopicModalHandler(); - if (!messages) { - return null; + if (isFetching) { + // Display loading indicator only if there are no messages to + // display from a previous search. + if (!messages || messages.length === 0) { + return ; } + } - if (messages.length === 0) { - return ; - } + if (!messages) { + return null; + } - return ( - - - - ); + if (messages.length === 0) { + return ; } + + return ( + + undefined} + startEditTopic={startEditTopic} + /> + + ); } From 2b499555a756e5dbebcd7027a69fde6d5a67ca56 Mon Sep 17 00:00:00 2001 From: Leslie Ngo Date: Wed, 28 Sep 2022 16:17:47 -0700 Subject: [PATCH 3/3] topic edit modal: Add a new edit-topic UI Fixes: #5365 --- src/ZulipMobile.js | 6 +- src/action-sheets/index.js | 26 +++---- src/boot/TopicEditModalProvider.js | 70 +++++++++++++++++ src/boot/TopicModalProvider.js | 92 ---------------------- src/chat/ChatScreen.js | 4 +- src/search/SearchMessagesCard.js | 4 +- src/streams/TopicItem.js | 4 +- src/title/TitleStream.js | 4 +- src/topics/TopicEditModal.js | 112 +++++++++++++++------------ src/webview/MessageList.js | 8 +- src/webview/handleOutboundEvents.js | 18 +---- static/translations/messages_en.json | 3 +- 12 files changed, 159 insertions(+), 192 deletions(-) create mode 100644 src/boot/TopicEditModalProvider.js delete mode 100644 src/boot/TopicModalProvider.js diff --git a/src/ZulipMobile.js b/src/ZulipMobile.js index 2c85af4ada5..12eaaeb2860 100644 --- a/src/ZulipMobile.js +++ b/src/ZulipMobile.js @@ -16,7 +16,7 @@ import CompatibilityChecker from './boot/CompatibilityChecker'; import AppEventHandlers from './boot/AppEventHandlers'; import { initializeSentry } from './sentry'; import ZulipSafeAreaProvider from './boot/ZulipSafeAreaProvider'; -import TopicModalProvider from './boot/TopicModalProvider'; +import TopicEditModalProvider from './boot/TopicEditModalProvider'; initializeSentry(); @@ -56,11 +56,11 @@ export default function ZulipMobile(): Node { - + - + diff --git a/src/action-sheets/index.js b/src/action-sheets/index.js index 687bf8dd132..bb6993d09f3 100644 --- a/src/action-sheets/index.js +++ b/src/action-sheets/index.js @@ -77,12 +77,7 @@ type TopicArgs = { zulipFeatureLevel: number, dispatch: Dispatch, _: GetText, - startEditTopic: ( - streamId: number, - topic: string, - streamsById: Map, - _: GetText, - ) => Promise, + startEditTopic: (streamId: number, topic: string) => Promise, ... }; @@ -260,8 +255,8 @@ const toggleResolveTopic = async ({ auth, streamId, topic, _, streams, zulipFeat const editTopic = { title: 'Edit topic', errorMessage: 'Failed to resolve topic', - action: ({ streamId, topic, streams, _, startEditTopic }) => { - startEditTopic(streamId, topic, streams, _); + action: ({ streamId, topic, startEditTopic }) => { + startEditTopic(streamId, topic); }, }; @@ -516,10 +511,14 @@ export const constructTopicActionButtons = (args: {| const buttons = []; const unreadCount = getUnreadCountForTopic(unread, streamId, topic); + const isAdmin = roleIsAtLeast(ownUserRole, Role.Admin); if (unreadCount > 0) { buttons.push(markTopicAsRead); } - buttons.push(editTopic); + // Set back to isAdmin after testing feature + if (true) { + buttons.push(editTopic); + } if (isTopicMuted(streamId, topic, mute)) { buttons.push(unmuteTopic); } else { @@ -530,7 +529,7 @@ export const constructTopicActionButtons = (args: {| } else { buttons.push(unresolveTopic); } - if (roleIsAtLeast(ownUserRole, Role.Admin)) { + if (isAdmin) { buttons.push(deleteTopic); } const sub = subscriptions.get(streamId); @@ -681,12 +680,7 @@ export const showTopicActionSheet = (args: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, - startEditTopic: ( - streamId: number, - topic: string, - streamsById: Map, - _: GetText, - ) => Promise, + startEditTopic: (streamId: number, topic: string) => Promise, _: GetText, |}, backgroundData: $ReadOnly<{ diff --git a/src/boot/TopicEditModalProvider.js b/src/boot/TopicEditModalProvider.js new file mode 100644 index 00000000000..50f0e10c64b --- /dev/null +++ b/src/boot/TopicEditModalProvider.js @@ -0,0 +1,70 @@ +/* @flow strict-local */ +import React, { createContext, useState, useCallback, useContext } from 'react'; +import type { Context, Node } from 'react'; +import { useSelector } from '../react-redux'; +import TopicEditModal from '../topics/TopicEditModal'; +import { getAuth, getZulipFeatureLevel, getStreamsById } from '../selectors'; +import { TranslationContext } from './TranslationProvider'; + +type Props = $ReadOnly<{| + children: Node, +|}>; + +type StartEditTopicContext = ( + streamId: number, + topic: string, +) => Promise; + +// $FlowIssue[incompatible-type] +const TopicModal: Context = createContext(undefined); + +export const useStartEditTopic = ():StartEditTopicContext => useContext(TopicModal); + +export default function TopicEditModalProvider(props: Props): Node { + const { children } = props; + const auth = useSelector(getAuth); + const zulipFeatureLevel = useSelector(getZulipFeatureLevel); + const streamsById = useSelector(getStreamsById); + const _ = useContext(TranslationContext); + + const [topicModalProviderState, setTopicModalProviderState] = useState({ + visible: false, + streamId: -1, + topic: '', + }); + + const startEditTopic = useCallback( + async (streamId: number, topic: string) => { + const { visible } = topicModalProviderState; + if (visible) { + return; + } + setTopicModalProviderState({ + visible: true, + streamId, + topic, + }); + }, [topicModalProviderState]); + + const closeEditTopicModal = useCallback(() => { + setTopicModalProviderState({ + visible: false, + streamId: -1, + topic: '', + }); + }, []); + + return ( + + + {children} + + ); +} diff --git a/src/boot/TopicModalProvider.js b/src/boot/TopicModalProvider.js deleted file mode 100644 index 91bb0b399a3..00000000000 --- a/src/boot/TopicModalProvider.js +++ /dev/null @@ -1,92 +0,0 @@ -/* @flow strict-local */ -import React, { createContext, useState, useMemo, useCallback, useContext } from 'react'; -import type { Context, Node } from 'react'; -import { useSelector } from '../react-redux'; -import TopicEditModal from '../topics/TopicEditModal'; -import type { Stream, GetText } from '../types'; -import { fetchSomeMessageIdForConversation } from '../message/fetchActions'; -import { getAuth, getZulipFeatureLevel } from '../selectors'; - -type Props = $ReadOnly<{| - children: Node, -|}>; - -type TopicModalContext = $ReadOnly<{| - startEditTopic: ( - streamId: number, - topic: string, - streamsById: Map, - _: GetText, - ) => Promise, - closeEditTopicModal: () => void, -|}>; - -// $FlowIssue[incompatible-type] -const TopicModal: Context = createContext(undefined); - -export const useTopicModalHandler = (): TopicModalContext => useContext(TopicModal); - -export default function TopicModalProvider(props: Props): Node { - const { children } = props; - const auth = useSelector(getAuth); - const zulipFeatureLevel = useSelector(getZulipFeatureLevel); - const [topicModalState, setTopicModalState] = useState({ - visible: false, - topic: '', - fetchArgs: { - auth: null, - messageId: null, - zulipFeatureLevel: null, - }, - }); - - const startEditTopic = useCallback( - async (streamId, topic, streamsById, _) => { - const messageId = await fetchSomeMessageIdForConversation( - auth, - streamId, - topic, - streamsById, - zulipFeatureLevel, - ); - if (messageId == null) { - throw new Error( - _('No messages in topic: {streamAndTopic}', { - streamAndTopic: `#${streamsById.get(streamId)?.name ?? 'unknown'} > ${topic}`, - }), - ); - } - setTopicModalState({ - visible: true, - topic, - fetchArgs: { auth, messageId, zulipFeatureLevel }, - }); - }, - [auth, zulipFeatureLevel], - ); - - const closeEditTopicModal = useCallback(() => { - setTopicModalState({ - visible: false, - topic: null, - fetchArgs: { auth: null, messageId: null, zulipFeatureLevel: null }, - }); - }, []); - - const topicModalHandler = useMemo( - () => ({ - startEditTopic, - closeEditTopicModal, - }), - [startEditTopic, closeEditTopicModal], - ); - - return ( - - {topicModalState.visible && ( - - )} - {children} - - ); -} diff --git a/src/chat/ChatScreen.js b/src/chat/ChatScreen.js index 472e0fcd163..0dd1a74f23c 100644 --- a/src/chat/ChatScreen.js +++ b/src/chat/ChatScreen.js @@ -30,7 +30,7 @@ import { showErrorAlert } from '../utils/info'; import { TranslationContext } from '../boot/TranslationProvider'; import * as api from '../api'; import { useConditionalEffect } from '../reactUtils'; -import { useTopicModalHandler } from '../boot/TopicModalProvider'; +import { useStartEditTopic } from '../boot/TopicEditModalProvider'; type Props = $ReadOnly<{| navigation: AppNavigationProp<'chat'>, @@ -128,7 +128,7 @@ const useMessagesWithFetch = args => { export default function ChatScreen(props: Props): Node { const { route, navigation } = props; const { backgroundColor } = React.useContext(ThemeContext); - const { startEditTopic } = useTopicModalHandler(); + const startEditTopic = useStartEditTopic(); const { narrow, editMessage } = route.params; const setEditMessage = useCallback( diff --git a/src/search/SearchMessagesCard.js b/src/search/SearchMessagesCard.js index 4d00da798a2..ede07efd0d4 100644 --- a/src/search/SearchMessagesCard.js +++ b/src/search/SearchMessagesCard.js @@ -9,7 +9,7 @@ import { createStyleSheet } from '../styles'; import LoadingIndicator from '../common/LoadingIndicator'; import SearchEmptyState from '../common/SearchEmptyState'; import MessageList from '../webview/MessageList'; -import { useTopicModalHandler } from '../boot/TopicModalProvider'; +import { useStartEditTopic } from '../boot/TopicEditModalProvider'; const styles = createStyleSheet({ results: { @@ -25,7 +25,7 @@ type Props = $ReadOnly<{| export default function SearchMessagesCard(props: Props): Node { const { narrow, isFetching, messages } = props; - const { startEditTopic } = useTopicModalHandler(); + const startEditTopic = useStartEditTopic(); if (isFetching) { // Display loading indicator only if there are no messages to diff --git a/src/streams/TopicItem.js b/src/streams/TopicItem.js index c197e3e9abb..2d58dc2e3b6 100644 --- a/src/streams/TopicItem.js +++ b/src/streams/TopicItem.js @@ -25,7 +25,7 @@ import { import { getMute } from '../mute/muteModel'; import { getUnread } from '../unread/unreadModel'; import { getOwnUserRole } from '../permissionSelectors'; -import { useTopicModalHandler } from '../boot/TopicModalProvider'; +import { useStartEditTopic } from '../boot/TopicEditModalProvider'; const componentStyles = createStyleSheet({ selectedRow: { @@ -71,7 +71,7 @@ export default function TopicItem(props: Props): Node { useActionSheet().showActionSheetWithOptions; const _ = useContext(TranslationContext); const dispatch = useDispatch(); - const { startEditTopic } = useTopicModalHandler(); + const startEditTopic = useStartEditTopic(); const backgroundData = useSelector(state => ({ auth: getAuth(state), mute: getMute(state), diff --git a/src/title/TitleStream.js b/src/title/TitleStream.js index 5ffce4e9509..d1b927d5e8e 100644 --- a/src/title/TitleStream.js +++ b/src/title/TitleStream.js @@ -27,7 +27,7 @@ import { showStreamActionSheet, showTopicActionSheet } from '../action-sheets'; import type { ShowActionSheetWithOptions } from '../action-sheets'; import { getUnread } from '../unread/unreadModel'; import { getOwnUserRole } from '../permissionSelectors'; -import { useTopicModalHandler } from '../boot/TopicModalProvider'; +import { useStartEditTopic } from '../boot/TopicEditModalProvider'; type Props = $ReadOnly<{| narrow: Narrow, @@ -68,7 +68,7 @@ export default function TitleStream(props: Props): Node { const showActionSheetWithOptions: ShowActionSheetWithOptions = useActionSheet().showActionSheetWithOptions; const _ = useContext(TranslationContext); - const { startEditTopic } = useTopicModalHandler(); + const startEditTopic = useStartEditTopic(); return ( , - _: GetText, - ) => Promise, - closeEditTopicModal: () => void, + streamId: number, }, -}; + auth: Auth, + zulipFeatureLevel: number, + streamsById: Map, + _: GetText, + closeEditTopicModal: () => void, +|}>; + +export default function TopicEditModal(props: Props): Node { + const { + topicModalProviderState, + closeEditTopicModal, + auth, + zulipFeatureLevel, + streamsById, + _, + } = props; -export default function TopicEditModal({ - topicModalState, - topicModalHandler, -}: TopicEditModalArgs): JSX$Element { - const { topic, fetchArgs } = topicModalState; - const [topicState, onChangeTopic] = useState(topic); - const { closeEditTopicModal } = topicModalHandler; - const { auth, messageId, zulipFeatureLevel } = fetchArgs; + const { visible, topic, streamId } = topicModalProviderState; + + const [topicName, onChangeTopicName] = useState(); + + useEffect(() => { + onChangeTopicName(topic); + }, [topic]); const { backgroundColor } = useContext(ThemeContext); @@ -110,9 +113,23 @@ export default function TopicEditModal({ }); const handleSubmit = async () => { + const messageId = await fetchSomeMessageIdForConversation( + auth, + streamId, + topic, + streamsById, + zulipFeatureLevel, + ); + if (messageId == null) { + throw new Error( + _('No messages in topic: {streamAndTopic}', { + streamAndTopic: `#${streamsById.get(streamId)?.name ?? 'unknown'} > ${topic}`, + }), + ); + } await updateMessage(auth, messageId, { propagate_mode: 'change_all', - subject: topicState, + subject: topicName, ...(zulipFeatureLevel >= 9 && { send_notification_to_old_thread: true, send_notification_to_new_thread: true, @@ -121,21 +138,14 @@ export default function TopicEditModal({ closeEditTopicModal(); }; return ( - { - closeEditTopicModal(); - }} - > + - Edit topic + @@ -147,22 +157,26 @@ export default function TopicEditModal({ borderColor: BRAND_COLOR, ...styles.button, }} - onPress={() => { - closeEditTopicModal(); - }} + onPress={closeEditTopicModal} > - Cancel + - Submit + diff --git a/src/webview/MessageList.js b/src/webview/MessageList.js index 74132f9fea9..68699bccd8d 100644 --- a/src/webview/MessageList.js +++ b/src/webview/MessageList.js @@ -14,7 +14,6 @@ import type { MessageListElement, UserOrBot, EditMessage, - Stream, } from '../types'; import { assumeSecretlyGlobalState } from '../reduxTypes'; import { connect } from '../react-redux'; @@ -46,12 +45,7 @@ type OuterProps = $ReadOnly<{| initialScrollMessageId: number | null, showMessagePlaceholders: boolean, startEditMessage: (editMessage: EditMessage) => void, - startEditTopic: ( - streamId: number, - topic: string, - streamsById: Map, - _: GetText, - ) => Promise, + startEditTopic: (streamId: number, topic: string) => Promise, |}>; type SelectorProps = {| diff --git a/src/webview/handleOutboundEvents.js b/src/webview/handleOutboundEvents.js index eae496f7cc6..cf8a7127bdb 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -4,16 +4,7 @@ import { Clipboard, Alert } from 'react-native'; import * as NavigationService from '../nav/NavigationService'; import * as api from '../api'; import config from '../config'; -import type { - Dispatch, - GetText, - Message, - Narrow, - Outbox, - EditMessage, - UserId, - Stream, -} from '../types'; +import type { Dispatch, GetText, Message, Narrow, Outbox, EditMessage, UserId } from '../types'; import type { BackgroundData } from './backgroundData'; import type { ShowActionSheetWithOptions } from '../action-sheets'; import type { JSONableDict } from '../utils/jsonable'; @@ -178,12 +169,7 @@ type Props = $ReadOnly<{ doNotMarkMessagesAsRead: boolean, showActionSheetWithOptions: ShowActionSheetWithOptions, startEditMessage: (editMessage: EditMessage) => void, - startEditTopic: ( - streamId: number, - topic: string, - streamsById: Map, - _: GetText, - ) => Promise, + startEditTopic: (streamId: number, topic: string) => Promise, ... }>; diff --git a/static/translations/messages_en.json b/static/translations/messages_en.json index a6dc550672e..65b74e2fb26 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -338,5 +338,6 @@ "Failed to copy stream link": "Failed to copy stream link", "A stream with this name already exists.": "A stream with this name already exists.", "Streams": "Streams", - "Edit topic": "Edit topic" + "Edit topic": "Edit topic", + "Submit": "Submit" }