From de271d7570588d11e90d59bf76162010da785fef Mon Sep 17 00:00:00 2001 From: Leslie Ngo Date: Thu, 29 Sep 2022 01:04:13 -0700 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 | 62 ++++++ src/chat/ChatScreen.js | 3 + src/common/ZulipTextButton.js | 19 +- 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 | 8 +- 13 files changed, 318 insertions(+), 13 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 3de98c302f0..12eaaeb2860 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 TopicEditModalProvider from './boot/TopicEditModalProvider'; 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..39ddd7a5c75 100644 --- a/src/action-sheets/index.js +++ b/src/action-sheets/index.js @@ -77,6 +77,7 @@ type TopicArgs = { zulipFeatureLevel: number, dispatch: Dispatch, _: GetText, + startEditTopic: (streamId: number, topic: string) => void, ... }; @@ -169,6 +170,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', @@ -502,9 +511,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 { @@ -515,7 +533,7 @@ export const constructTopicActionButtons = (args: {| } else { buttons.push(unresolveTopic); } - if (roleIsAtLeast(ownUserRole, Role.Admin)) { + if (isAdmin) { buttons.push(deleteTopic); } const sub = subscriptions.get(streamId); @@ -666,6 +684,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..e2631264d08 --- /dev/null +++ b/src/boot/TopicEditModalProvider.js @@ -0,0 +1,62 @@ +/* @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, +|}>; + +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({ + visible: false, + streamId: -1, + oldTopic: '', + }); + + const { visible } = topicModalProviderState; + + const startEditTopic = useCallback( + (streamId: number, oldTopic: string) => { + if (visible) { + return; + } + setTopicModalProviderState({ + visible: true, + streamId, + oldTopic, + }); + }, + [visible], + ); + + const closeEditTopicModal = useCallback(() => { + setTopicModalProviderState({ + visible: false, + streamId: -1, + oldTopic: '', + }); + }, []); + + return ( + + + {children} + + ); +} diff --git a/src/chat/ChatScreen.js b/src/chat/ChatScreen.js index 9342c1f4f53..316034f3a45 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 { useStartEditTopic } from '../boot/TopicEditModalProvider'; type Props = $ReadOnly<{| navigation: AppNavigationProp<'chat'>, @@ -133,6 +134,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)); @@ -221,6 +223,7 @@ export default function ChatScreen(props: Props): Node { } showMessagePlaceholders={showMessagePlaceholders} startEditMessage={setEditMessage} + startEditTopic={startEditTopic} /> ); } diff --git a/src/common/ZulipTextButton.js b/src/common/ZulipTextButton.js index 1c888acd7cc..2261ea1f159 100644 --- a/src/common/ZulipTextButton.js +++ b/src/common/ZulipTextButton.js @@ -86,7 +86,7 @@ type Props = $ReadOnly<{| /** * isPressHandledWhenDisabled - Whether `onPress` is used even when * `disabled` is true. - */ + */ isPressHandledWhenDisabled?: boolean, onPress: () => void | Promise, @@ -111,9 +111,20 @@ type Props = $ReadOnly<{| // things like project-specific styles and making any sensible adjustments // to the interface. export default function ZulipTextButton(props: Props): Node { - const { variant = 'standard', leftMargin, rightMargin, label, onPress, disabled = false, isPressHandledWhenDisabled = false } = props; - - const variantStyles = useMemo(() => styleSheetForVariant(variant, disabled === true), [variant, disabled]); + const { + variant = 'standard', + leftMargin, + rightMargin, + label, + onPress, + disabled = false, + isPressHandledWhenDisabled = false, + } = props; + + const variantStyles = useMemo( + () => styleSheetForVariant(variant, disabled === true), + [variant, disabled], + ); return ( 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..5855e776c5d --- /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, BRAND_COLOR, createStyleSheet } from '../styles'; +import { updateMessage } from '../api'; +import type { Auth, GetText, Stream } from '../types'; +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: { + visible: boolean, + oldTopic: string, + streamId: number, + }, + closeEditTopicModal: () => void, +|}>; + +export default function TopicEditModal(props: Props): Node { + const { topicModalProviderState, closeEditTopicModal } = props; + + const auth: Auth = useSelector(getAuth); + const zulipFeatureLevel: number = useSelector(getZulipFeatureLevel); + const streamsById: Map = useSelector(getStreamsById); + const mandatoryTopics = useSelector(state => getRealm(state).mandatoryTopics); + const _: GetText = useContext(TranslationContext); + + const { visible, 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 { backgroundColor } = useContext(ThemeContext); + + const modalStyles = createStyleSheet({ + backdrop: { + position: 'absolute', + backgroundColor: 'black', + opacity: 0.25, + top: 0, + left: 0, + right: 0, + bottom: 0, + }, + wrapper: { + flex: 1, + justifyContent: 'center', + alignItems: 'center', + }, + modal: { + justifyContent: 'flex-start', + backgroundColor, + padding: 15, + shadowOpacity: 0.5, + shadowColor: 'gray', + shadowOffset: { + height: 5, + width: 5, + }, + shadowRadius: 5, + borderRadius: 5, + width: '90%', + }, + buttonContainer: { + flexDirection: 'row', + justifyContent: 'flex-end', + }, + titleText: { + fontSize: 18, + lineHeight: 21, + color: BRAND_COLOR, + marginBottom: 10, + fontWeight: 'bold', + }, + }); + + const validationErrors = useMemo(() => { + const result = []; + if (mandatoryTopics && newTopicInputValue.trim() === '') { + result.push('mandatory-topic-empty'); + } + if (newTopicInputValue === oldTopic) { + result.push('user-did-not-edit'); + } + // Max topic length: + // https://zulip.com/api/update-message#parameter-topic + if (newTopicInputValue.length > 60) { + result.push('topic-too-long'); + } + + return result; + }, [mandatoryTopics, newTopicInputValue, oldTopic]); + + const handleSubmit = async () => { + if (validationErrors.length > 0) { + const errorMessages = validationErrors + .map(error => { + // 'mandatory-topic-empty' | 'user-did-not-edit' | 'topic-too-long' + 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 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(); + } + }; + + return ( + + + + + + + + + + + + + + ); +} diff --git a/src/webview/MessageList.js b/src/webview/MessageList.js index 8e6e7353560..97f5a4ebd91 100644 --- a/src/webview/MessageList.js +++ b/src/webview/MessageList.js @@ -45,6 +45,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 7e508fe32f5..48603c876f3 100644 --- a/static/translations/messages_en.json +++ b/static/translations/messages_en.json @@ -337,5 +337,11 @@ "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", + "You may need to enter a topic in this stream.": "You may need to enter a topic in this stream.", + "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." }