Skip to content

Commit

Permalink
Revert "message list: Modify "Reply" action when in announcement-only…
Browse files Browse the repository at this point in the history
… narrow."

This reverts commit 674f9a2 (#3811).

That commit fixed only some cases of #3810: it relies on the current
narrow rather than the specific message, and so it misses relevant
messages when we're in a starred narrow, a search, etc.  The right fix
will involve plumbing a different set of data down through these
layers, rather than `isAnnouncementOnly`, so it'll be simpler to do
that fresh than starting from here.
  • Loading branch information
gnprice committed Feb 4, 2020
1 parent 4d3cec3 commit 8f58f46
Show file tree
Hide file tree
Showing 4 changed files with 3 additions and 31 deletions.
17 changes: 2 additions & 15 deletions src/message/messageActionSheet.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,6 @@ const reply = ({ message, dispatch, ownEmail }) => {
reply.title = 'Reply';
reply.errorMessage = 'Failed to reply';

const narrowToTopic = ({ message, dispatch, ownEmail }) => {
dispatch(doNarrow(getNarrowFromMessage(message, ownEmail), message.id));
};
narrowToTopic.title = 'Narrow to topic';
narrowToTopic.errorMessage = 'Failed to narrow to topic';

const copyToClipboard = async ({ _, auth, message }) => {
const rawMessage = isAnOutboxMessage(message) /* $FlowFixMe: then really type Outbox */
? message.markdownContent
Expand Down Expand Up @@ -154,7 +148,6 @@ const allButtonsRaw = {
// For messages
addReaction,
reply,
narrowToTopic,
copyToClipboard,
shareMessage,
editMessage,
Expand Down Expand Up @@ -185,7 +178,6 @@ type ConstructSheetParams = {|
backgroundData: BackgroundData,
message: Message | Outbox,
narrow: Narrow,
isAnnouncementOnly: boolean,
|};

export const constructHeaderActionButtons = ({
Expand Down Expand Up @@ -214,10 +206,9 @@ const messageNotDeleted = (message: Message | Outbox): boolean =>
message.content !== '<p>(deleted)</p>';

export const constructMessageActionButtons = ({
backgroundData: { ownEmail, flags, isAdmin },
backgroundData: { ownEmail, flags },
message,
narrow,
isAnnouncementOnly,
}: ConstructSheetParams): ButtonCode[] => {
const buttons = [];
if (message.reactions.length > 0) {
Expand All @@ -227,11 +218,7 @@ export const constructMessageActionButtons = ({
buttons.push('addReaction');
}
if (!isAnOutboxMessage(message) && !isTopicNarrow(narrow) && !isPrivateOrGroupNarrow(narrow)) {
if (isAnnouncementOnly && !isAdmin) {
buttons.push('narrowToTopic');
} else {
buttons.push('reply');
}
buttons.push('reply');
}
if (messageNotDeleted(message)) {
buttons.push('copyToClipboard');
Expand Down
5 changes: 0 additions & 5 deletions src/webview/MessageList.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ import { getUpdateEvents } from './webViewHandleUpdates';
import { handleMessageListEvent } from './webViewEventHandlers';
import { base64Utf8Encode } from '../utils/encoding';
import * as logging from '../utils/logging';
import { getIsActiveStreamAnnouncementOnly } from '../subscriptions/subscriptionSelectors';

// ESLint doesn't notice how `this.props` escapes, and complains about some
// props not being used here.
Expand All @@ -74,7 +73,6 @@ export type BackgroundData = $ReadOnly<{
mute: MuteState,
ownEmail: string,
ownUserId: number,
isAdmin: boolean,
allImageEmojiById: $ReadOnly<{ [id: string]: ImageEmojiType }>,
twentyFourHourTime: boolean,
subscriptions: Subscription[],
Expand All @@ -89,7 +87,6 @@ type SelectorProps = {|
showMessagePlaceholders: boolean,
theme: ThemeName,
typingUsers: $ReadOnlyArray<User>,
isAnnouncementOnly: boolean,
|};

// TODO get a type for `connectActionSheet` so this gets fully type-checked.
Expand Down Expand Up @@ -343,7 +340,6 @@ export default connect<SelectorProps, _, _>((state, props: OuterProps) => {
mute: getMute(state),
ownEmail: getOwnEmail(state),
ownUserId: getOwnUser(state).user_id,
isAdmin: getOwnUser(state).is_admin,
allImageEmojiById: getAllImageEmojiById(state),
subscriptions: getSubscriptions(state),
twentyFourHourTime: getRealm(state).twentyFourHourTime,
Expand All @@ -361,6 +357,5 @@ export default connect<SelectorProps, _, _>((state, props: OuterProps) => {
: getShowMessagePlaceholders(props.narrow)(state),
theme: getSettings(state).theme,
typingUsers: props.typingUsers || getCurrentTypingUsers(state, props.narrow),
isAnnouncementOnly: getIsActiveStreamAnnouncementOnly(state, props.narrow),
};
})(connectActionSheet(withGetText(MessageList)));
10 changes: 1 addition & 9 deletions src/webview/webViewEventHandlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,6 @@ type Props = $ReadOnly<{
messages: $ReadOnlyArray<Message | Outbox>,
narrow: Narrow,
showActionSheetWithOptions: ShowActionSheetWithOptions,
isAnnouncementOnly: boolean,
}>;

const fetchMore = (props: Props, event: MessageListEventScroll) => {
Expand Down Expand Up @@ -181,18 +180,11 @@ const handleLongPress = (
if (!message) {
return;
}
const {
dispatch,
showActionSheetWithOptions,
backgroundData,
narrow,
isAnnouncementOnly,
} = props;
const { dispatch, showActionSheetWithOptions, backgroundData, narrow } = props;
showActionSheet(target === 'header', dispatch, showActionSheetWithOptions, _, {
backgroundData,
message,
narrow,
isAnnouncementOnly,
});
};

Expand Down
2 changes: 0 additions & 2 deletions static/translations/messages_en.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@
"Narrow to conversation": "Narrow to conversation",
"Reply": "Reply",
"Add a reaction": "Add a reaction",
"Narrow to topic": "Narrow to topic",
"Copy to clipboard": "Copy to clipboard",
"Link copied to clipboard": "Link copied to clipboard",
"Mute topic": "Mute topic",
Expand Down Expand Up @@ -123,7 +122,6 @@
"Edit message": "Edit message",
"Network request failed": "Network request failed",
"Failed to add reaction": "Failed to add reaction",
"Failed to narrow to topic": "Failed to narrow to topic",
"Failed to reply": "Failed to reply",
"Failed to copy message to clipboard": "Failed to copy message to clipboard",
"Failed to share message": "Failed to share message",
Expand Down

0 comments on commit 8f58f46

Please sign in to comment.