From 6bdb7bf487e91a52513e3d80ec64066c088f7afd Mon Sep 17 00:00:00 2001 From: Leslie Ngo Date: Mon, 7 Nov 2022 22:53:06 -0800 Subject: [PATCH] topic edit modal: Add new edit-topic UI. Fixes: #5365 --- src/ZulipMobile.js | 9 +- src/action-sheets/index.js | 21 +- src/boot/TopicEditModalProvider.js | 57 ++++++ src/chat/ChatScreen.js | 3 + src/search/SearchMessagesCard.js | 3 + src/streams/TopicItem.js | 4 +- src/title/TitleStream.js | 4 +- src/topics/TopicEditModal.js | 184 ++++++++++++++++++ src/webview/MessageList.js | 1 + .../__tests__/generateInboundEvents-test.js | 1 + src/webview/handleOutboundEvents.js | 12 +- static/translations/messages_en.json | 7 +- 12 files changed, 297 insertions(+), 9 deletions(-) create mode 100644 src/boot/TopicEditModalProvider.js create mode 100644 src/topics/TopicEditModal.js diff --git a/src/ZulipMobile.js b/src/ZulipMobile.js index cfbdc532da6..34c0d662b13 100644 --- a/src/ZulipMobile.js +++ b/src/ZulipMobile.js @@ -17,6 +17,7 @@ import AppEventHandlers from './boot/AppEventHandlers'; import { initializeSentry } from './sentry'; import ZulipSafeAreaProvider from './boot/ZulipSafeAreaProvider'; import { OfflineNoticeProvider } from './boot/OfflineNoticeProvider'; +import TopicEditModalProvider from './boot/TopicEditModalProvider'; initializeSentry(); @@ -79,9 +80,11 @@ export default function ZulipMobile(): Node { - - - + + + + + diff --git a/src/action-sheets/index.js b/src/action-sheets/index.js index e697c7cb1c3..7b714309872 100644 --- a/src/action-sheets/index.js +++ b/src/action-sheets/index.js @@ -79,6 +79,7 @@ type TopicArgs = { zulipFeatureLevel: number, dispatch: Dispatch, _: GetText, + startEditTopic: (streamId: number, topic: string) => void, ... }; @@ -171,6 +172,14 @@ const deleteMessage = { }, }; +const editTopic = { + title: 'Edit topic', + errorMessage: 'Failed to edit topic', + action: ({ streamId, topic, startEditTopic }) => { + startEditTopic(streamId, topic); + }, +}; + const markTopicAsRead = { title: 'Mark topic as read', errorMessage: 'Failed to mark topic as read', @@ -532,9 +541,18 @@ export const constructTopicActionButtons = (args: {| const buttons = []; const unreadCount = getUnreadCountForTopic(unread, streamId, topic); + const isAdmin = roleIsAtLeast(ownUserRole, Role.Admin); if (unreadCount > 0) { buttons.push(markTopicAsRead); } + // At present, the permissions for editing the topic of a message are highly complex. + // Until we move to a better set of policy options, we'll only display the edit topic + // button to admins. + // Issue: https://github.com/zulip/zulip/issues/21739 + // Relevant comment: https://github.com/zulip/zulip-mobile/issues/5365#issuecomment-1197093294 + if (isAdmin) { + buttons.push(editTopic); + } if (isTopicMuted(streamId, topic, mute)) { buttons.push(unmuteTopic); } else { @@ -545,7 +563,7 @@ export const constructTopicActionButtons = (args: {| } else { buttons.push(unresolveTopic); } - if (roleIsAtLeast(ownUserRole, Role.Admin)) { + if (isAdmin) { buttons.push(deleteTopic); } const sub = subscriptions.get(streamId); @@ -705,6 +723,7 @@ export const showTopicActionSheet = (args: {| showActionSheetWithOptions: ShowActionSheetWithOptions, callbacks: {| dispatch: Dispatch, + startEditTopic: (streamId: number, topic: string) => void, _: GetText, |}, backgroundData: $ReadOnly<{ diff --git a/src/boot/TopicEditModalProvider.js b/src/boot/TopicEditModalProvider.js new file mode 100644 index 00000000000..f082e6c5b93 --- /dev/null +++ b/src/boot/TopicEditModalProvider.js @@ -0,0 +1,57 @@ +/* @flow strict-local */ +import React, { createContext, useState, useCallback, useContext } from 'react'; +import type { Context, Node } from 'react'; + +import TopicEditModal from '../topics/TopicEditModal'; + +type Props = $ReadOnly<{| + children: Node, +|}>; + +export type TopicEditProviderStateType = { + streamId: number, + oldTopic: string, +}; + +type StartEditTopicContext = (streamId: number, oldTopic: string) => void; + +const TopicEditModalContext: Context = createContext(() => { + throw new Error( + 'Tried to open the edit-topic UI from a component without TopicEditModalProvider above it in the tree.', + ); +}); + +export const useStartEditTopic = (): StartEditTopicContext => useContext(TopicEditModalContext); + +export default function TopicEditModalProvider(props: Props): Node { + const { children } = props; + + const [topicModalProviderState, setTopicModalProviderState] = + useState(null); + + const startEditTopic = useCallback( + (streamIdArg, oldTopicArg) => { + if (!topicModalProviderState) { + setTopicModalProviderState({ + streamId: streamIdArg, + oldTopic: oldTopicArg, + }); + } + }, + [topicModalProviderState], + ); + + const closeEditTopicModal = () => { + setTopicModalProviderState(null); + }; + + return ( + + + {children} + + ); +} diff --git a/src/chat/ChatScreen.js b/src/chat/ChatScreen.js index c4b16876b2a..7cc1f3c3cf8 100644 --- a/src/chat/ChatScreen.js +++ b/src/chat/ChatScreen.js @@ -29,6 +29,7 @@ import { showErrorAlert } from '../utils/info'; import { TranslationContext } from '../boot/TranslationProvider'; import * as api from '../api'; import { useConditionalEffect } from '../reactUtils'; +import { useStartEditTopic } from '../boot/TopicEditModalProvider'; type Props = $ReadOnly<{| navigation: AppNavigationProp<'chat'>, @@ -132,6 +133,7 @@ export default function ChatScreen(props: Props): Node { (value: EditMessage | null) => navigation.setParams({ editMessage: value }), [navigation], ); + const startEditTopic = useStartEditTopic(); const isNarrowValid = useSelector(state => getIsNarrowValid(state, narrow)); const draft = useSelector(state => getDraftForNarrow(state, narrow)); @@ -219,6 +221,7 @@ export default function ChatScreen(props: Props): Node { } showMessagePlaceholders={showMessagePlaceholders} startEditMessage={setEditMessage} + startEditTopic={startEditTopic} /> ); } diff --git a/src/search/SearchMessagesCard.js b/src/search/SearchMessagesCard.js index da94b7d4386..ede07efd0d4 100644 --- a/src/search/SearchMessagesCard.js +++ b/src/search/SearchMessagesCard.js @@ -9,6 +9,7 @@ import { createStyleSheet } from '../styles'; import LoadingIndicator from '../common/LoadingIndicator'; import SearchEmptyState from '../common/SearchEmptyState'; import MessageList from '../webview/MessageList'; +import { useStartEditTopic } from '../boot/TopicEditModalProvider'; const styles = createStyleSheet({ results: { @@ -24,6 +25,7 @@ type Props = $ReadOnly<{| export default function SearchMessagesCard(props: Props): Node { const { narrow, isFetching, messages } = props; + const startEditTopic = useStartEditTopic(); if (isFetching) { // Display loading indicator only if there are no messages to @@ -55,6 +57,7 @@ export default function SearchMessagesCard(props: Props): Node { // TODO: handle editing a message from the search results, // or make this prop optional startEditMessage={() => undefined} + startEditTopic={startEditTopic} /> ); diff --git a/src/streams/TopicItem.js b/src/streams/TopicItem.js index d60e6b4f5fd..2d58dc2e3b6 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 { useStartEditTopic } from '../boot/TopicEditModalProvider'; 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 = useStartEditTopic(); 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..db6ec4997c6 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 { useStartEditTopic } from '../boot/TopicEditModalProvider'; type Props = $ReadOnly<{| narrow: Narrow, @@ -51,6 +52,7 @@ export default function TitleStream(props: Props): Node { const { narrow, color } = props; const dispatch = useDispatch(); const stream = useSelector(state => getStreamInNarrow(state, narrow)); + const startEditTopic = useStartEditTopic(); const backgroundData = useSelector(state => ({ auth: getAuth(state), mute: getMute(state), @@ -75,7 +77,7 @@ export default function TitleStream(props: Props): Node { ? () => { 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..f40ab4c987d --- /dev/null +++ b/src/topics/TopicEditModal.js @@ -0,0 +1,184 @@ +/* @flow strict-local */ +import React, { useState, useContext, useEffect, useMemo } from 'react'; +import { Modal, View } from 'react-native'; +import type { Node } from 'react'; + +import { useSelector } from '../react-redux'; +import styles, { ThemeContext, createStyleSheet } from '../styles'; +import * as api from '../api'; +import { fetchSomeMessageIdForConversation } from '../message/fetchActions'; +import ZulipTextIntl from '../common/ZulipTextIntl'; +import ZulipTextButton from '../common/ZulipTextButton'; +import Input from '../common/Input'; +import { getAuth, getZulipFeatureLevel, getStreamsById, getRealm } from '../selectors'; +import { TranslationContext } from '../boot/TranslationProvider'; +import { showErrorAlert } from '../utils/info'; +import { ensureUnreachable } from '../generics'; + +type Props = $ReadOnly<{| + topicModalProviderState: { + oldTopic: string, + streamId: number, + } | null, + closeEditTopicModal: () => void, +|}>; + +export default function TopicEditModal(props: Props): Node { + const { topicModalProviderState, closeEditTopicModal } = props; + + const auth = useSelector(getAuth); + const zulipFeatureLevel = useSelector(getZulipFeatureLevel); + const streamsById = useSelector(getStreamsById); + const mandatoryTopics = useSelector(state => getRealm(state).mandatoryTopics); + const _ = useContext(TranslationContext); + const { backgroundColor } = useContext(ThemeContext); + + const { oldTopic, streamId } = topicModalProviderState || {}; + + const [newTopicInputValue, setNewTopicInputValue] = useState(oldTopic); + + // This resets the input value when we enter a new topic-editing session. + useEffect(() => { + setNewTopicInputValue(oldTopic); + }, [oldTopic]); + + const validationErrors = useMemo(() => { + const result = []; + if (typeof newTopicInputValue === 'string') { + const trimmedInput = newTopicInputValue.trim(); + if (mandatoryTopics && trimmedInput === '') { + result.push('mandatory-topic-empty'); + } + if (trimmedInput === oldTopic) { + result.push('user-did-not-edit'); + } + // Max topic length: + // https://zulip.com/api/update-message#parameter-topic + if (trimmedInput.length > 60) { + result.push('topic-too-long'); + } + } + return result; + }, [mandatoryTopics, newTopicInputValue, oldTopic]); + + const handleSubmit = async () => { + if (!topicModalProviderState) { + throw new Error(_('Topic, streamId, or input value is null.')); + } + if (validationErrors.length > 0) { + const errorMessages = validationErrors + .map(error => { + switch (error) { + case 'mandatory-topic-empty': + return _('Topic is required in this stream.'); + case 'user-did-not-edit': + return _("You haven't made any changes."); + case 'topic-too-long': + return _('Topic too long (max 60 characters).'); + default: + ensureUnreachable(error); + throw new Error(); + } + }) + .join('\n\n'); + showErrorAlert(errorMessages); + return; + } + try { + const messageId = await fetchSomeMessageIdForConversation( + auth, + streamId, + oldTopic, + streamsById, + zulipFeatureLevel, + ); + if (messageId == null) { + throw new Error( + _('No messages in topic: {streamAndTopic}', { + streamAndTopic: `#${streamsById.get(streamId)?.name ?? 'unknown'} > ${oldTopic}`, + }), + ); + } + await api.updateMessage(auth, messageId, { + propagate_mode: 'change_all', + subject: newTopicInputValue.trim(), + ...(zulipFeatureLevel >= 9 && { + send_notification_to_old_thread: true, + send_notification_to_new_thread: true, + }), + }); + } catch (error) { + showErrorAlert('Failed to edit topic.'); + } finally { + closeEditTopicModal(); + } + }; + + const modalStyles = createStyleSheet({ + backdrop: { + position: 'absolute', + // backgroundColor and opacity aims to match how much our + // action sheets darken the background when they are toggled. + backgroundColor: 'black', + opacity: 0.25, + top: 0, + left: 0, + right: 0, + bottom: 0, + }, + wrapper: { + flex: 1, + justifyContent: 'center', + alignItems: 'center', + }, + modal: { + // these values are borrowed from Popup.js + justifyContent: 'flex-start', + backgroundColor, + padding: 15, + shadowOpacity: 0.5, + elevation: 8, + shadowRadius: 16, + borderRadius: 5, + width: 280, + }, + buttonContainer: { + flexDirection: 'row', + justifyContent: 'flex-end', + }, + titleText: { + fontSize: 18, + lineHeight: 21, + marginBottom: 10, + fontWeight: 'bold', + }, + }); + + return ( + + + + + + + + + + + + + + ); +} diff --git a/src/webview/MessageList.js b/src/webview/MessageList.js index 70a9b8eedd5..405e5663ea2 100644 --- a/src/webview/MessageList.js +++ b/src/webview/MessageList.js @@ -46,6 +46,7 @@ type OuterProps = $ReadOnly<{| initialScrollMessageId: number | null, showMessagePlaceholders: boolean, startEditMessage: (editMessage: EditMessage) => void, + startEditTopic: (streamId: number, topic: string) => void, |}>; type SelectorProps = {| 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..8c798f416c8 100644 --- a/src/webview/handleOutboundEvents.js +++ b/src/webview/handleOutboundEvents.js @@ -169,6 +169,7 @@ type Props = $ReadOnly<{ doNotMarkMessagesAsRead: boolean, showActionSheetWithOptions: ShowActionSheetWithOptions, startEditMessage: (editMessage: EditMessage) => void, + startEditTopic: (streamId: number, topic: string) => void, ... }>; @@ -222,12 +223,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 074764a61cc..1e41ec749e8 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -350,5 +350,10 @@ "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", + "Submit": "Submit", + "Topic is required in this stream.": "Topic is required in this stream.", + "You haven't made any changes.": "You haven't made any changes.", + "Topic too long (max 60 characters).": "Topic too long (max 60 characters)." }