Skip to content

Commit

Permalink
[BREAK] Remove show message in main thread preference (#26002)
Browse files Browse the repository at this point in the history
<!-- This is a pull request template, you do not need to uncomment or remove the comments, they won't show up in the PR text. -->

<!-- Your Pull Request name should start with one of the following tags
  [NEW] For new features
  [IMPROVE] For an improvement (performance or little improvements) in existing features
  [FIX] For bug fixes that affect the end-user
  [BREAK] For pull requests including breaking changes
  Chore: For small tasks
  Doc: For documentation
-->

<!-- Checklist!!! If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code. 
  - I have read the Contributing Guide - https://github.com/RocketChat/Rocket.Chat/blob/develop/.github/CONTRIBUTING.md#contributing-to-rocketchat doc
  - I have signed the CLA - https://cla-assistant.io/RocketChat/Rocket.Chat
  - Lint and unit tests pass locally with my changes
  - I have added tests that prove my fix is effective or that my feature works (if applicable)
  - I have added necessary documentation (if applicable)
  - Any dependent changes have been merged and published in downstream modules
-->

## Proposed changes (including videos or screenshots)
<!-- CHANGELOG -->
<!--
  Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request.
  If it fixes a bug or resolves a feature request, be sure to link to that issue below.
  This description will appear in the release notes if we accept the contribution.
-->
This PR removes the confusion between the `show message in main thread` and the function `also to send to channel`. In the past, we used the `show message in main thread` as a solution to help users to understand the thread feature, as this feature is now mature enough there's no reason to maintain this preference. 

Send the thread message to the main channel or just inside of the thread, should be a decision from the user where the function `also send to channel` appears. Because of that, and because of a bunch of requests and issues we received, we're introducing a new preference `also send thread to channel` where users will be able to decide the behavior of the checkbox. 

![image](https://user-images.githubusercontent.com/27704687/175655594-023c5907-adc8-4924-ba7d-467608d06fec.png)

Now there are three behavior options
- `Default`: when it unchecks after sending the first message
<img width='250px' height='350px' src='https://user-images.githubusercontent.com/27704687/175656500-34817639-7f13-4641-b4fa-9dd106e99443.gif' />

- `Always`: stay checked for all messages
<img width='250px' height='350px' src='https://user-images.githubusercontent.com/27704687/175657299-d88efaba-1c2b-4bb9-a23a-f9755dcec5ca.gif' />

- `Never`: stay unchecked for all messages
<img width='250px' height='350px' src='https://user-images.githubusercontent.com/27704687/175657544-3dcd0adc-05cf-4196-83a6-f6cc29a1de2b.gif' />

<!-- END CHANGELOG -->

## Issue(s)
<!-- Link the issues being closed by or related to this PR. For example, you can use #594 if this PR closes issue number 594 -->
Closes #20695
Closes #19167
Closes #18347

## Steps to test or reproduce
<!-- Mention how you would reproduce the bug if not mentioned on the issue page already. Also mention which screens are going to have the changes if applicable -->

## Further comments
<!-- If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc... -->


Co-authored-by: Guilherme Gazzo <[email protected]>
  • Loading branch information
dougfabris and ggazzo authored Jun 27, 2022
1 parent c8f93fa commit e85a63b
Show file tree
Hide file tree
Showing 13 changed files with 79 additions and 35 deletions.
20 changes: 17 additions & 3 deletions apps/meteor/app/lib/server/startup/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -426,10 +426,23 @@ settingsRegistry.addGroup('Accounts', function () {
i18nLabel: 'Sort_By',
});

this.add('Accounts_Default_User_Preferences_showMessageInMainThread', false, {
type: 'boolean',
this.add('Accounts_Default_User_Preferences_alsoSendThreadToChannel', 'default', {
type: 'select',
values: [
{
key: 'default',
i18nLabel: 'Default',
},
{
key: 'always',
i18nLabel: 'Always',
},
{
key: 'never',
i18nLabel: 'Never',
},
],
public: true,
i18nLabel: 'Show_Message_In_Main_Thread',
});

this.add('Accounts_Default_User_Preferences_sidebarShowFavorites', true, {
Expand Down Expand Up @@ -457,6 +470,7 @@ settingsRegistry.addGroup('Accounts', function () {
public: true,
i18nLabel: 'Enter_Behaviour',
});

this.add('Accounts_Default_User_Preferences_messageViewMode', 0, {
type: 'select',
values: [
Expand Down
22 changes: 20 additions & 2 deletions apps/meteor/app/threads/client/flextab/thread.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ Template.thread.helpers({
} = Template.currentData();

const showFormattingTips = settings.get('Message_ShowFormattingTips');
const alsoSendPreferenceState = getUserPreference(Meteor.userId(), 'alsoSendThreadToChannel');

return {
showFormattingTips,
tshow: instance.state.get('sendToChannel'),
Expand All @@ -85,7 +87,9 @@ Template.thread.helpers({
tmid,
onSend: (...args) => {
instance.sendToBottom();
instance.state.set('sendToChannel', false);
if (alsoSendPreferenceState === 'default') {
instance.state.set('sendToChannel', false);
}
return instance.chatMessages && instance.chatMessages.send.apply(instance.chatMessages, args);
},
onKeyUp: (...args) => instance.chatMessages && instance.chatMessages.keyup.apply(instance.chatMessages, args),
Expand Down Expand Up @@ -243,8 +247,22 @@ Template.thread.onRendered(function () {
Template.thread.onCreated(async function () {
this.Threads = new Mongo.Collection(null);

const preferenceState = getUserPreference(Meteor.userId(), 'alsoSendThreadToChannel');

let sendToChannel;
switch (preferenceState) {
case 'always':
sendToChannel = true;
break;
case 'never':
sendToChannel = false;
break;
default:
sendToChannel = !this.data.mainMessage.tcount;
}

this.state = new ReactiveDict({
sendToChannel: !this.data.mainMessage.tcount,
sendToChannel,
});

this.loadMore = async () => {
Expand Down
6 changes: 2 additions & 4 deletions apps/meteor/app/ui-utils/client/lib/RoomHistoryManager.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import { getConfig } from '../../../../client/lib/utils/getConfig';
import { ChatMessage, ChatSubscription, ChatRoom } from '../../../models/client';
import { callWithErrorHandling } from '../../../../client/lib/utils/callWithErrorHandling';
import { filterMarkdown } from '../../../markdown/lib/markdown';
import { getUserPreference } from '../../../utils/client';
import { onClientMessageReceived } from '../../../../client/lib/onClientMessageReceived';
import {
setHighlightMessage,
Expand Down Expand Up @@ -197,8 +196,7 @@ export const RoomHistoryManager = new (class extends Emitter {
typeName = (curRoomDoc ? curRoomDoc.t : undefined) + (curRoomDoc ? curRoomDoc.name : undefined);
}

const showMessageInMainThread = getUserPreference(Meteor.userId(), 'showMessageInMainThread', false);
const result = await callWithErrorHandling('loadHistory', rid, ts, limit, ls, showMessageInMainThread);
const result = await callWithErrorHandling('loadHistory', rid, ts, limit, ls);

this.unqueue();

Expand All @@ -224,7 +222,7 @@ export const RoomHistoryManager = new (class extends Emitter {
room.loaded = 0;
}

const visibleMessages = messages.filter((msg) => !msg.tmid || showMessageInMainThread || msg.tshow);
const visibleMessages = messages.filter((msg) => !msg.tmid || msg.tshow);

room.loaded += visibleMessages.length;

Expand Down
2 changes: 0 additions & 2 deletions apps/meteor/app/ui-utils/client/lib/messageContext.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const fields = {
'name': 1,
'username': 1,
'settings.preferences.enableNewMessageTemplate': 1,
'settings.preferences.showMessageInMainThread': 1,
'settings.preferences.autoImageLoad': 1,
'settings.preferences.saveMobileBandwidth': 1,
'settings.preferences.collapseMediaByDefault': 1,
Expand Down Expand Up @@ -115,7 +114,6 @@ export function messageContext({ rid } = Template.instance()) {
},
settings: {
translateLanguage: AutoTranslate.getLanguage(rid),
showMessageInMainThread: getUserPreference(user, 'showMessageInMainThread'),
autoImageLoad: getUserPreference(user, 'autoImageLoad'),
enableNewMessageTemplate: getUserPreference(user, 'enableNewMessageTemplate'),
saveMobileBandwidth: Meteor.Device.isPhone() && getUserPreference(user, 'saveMobileBandwidth'),
Expand Down
5 changes: 1 addition & 4 deletions apps/meteor/app/ui/client/views/app/room.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,6 @@ Template.roomOld.helpers({
return state.get('subscribed');
},
messagesHistory() {
const showInMainThread = getUserPreference(Meteor.userId(), 'showMessageInMainThread', false);
const { rid } = Template.instance();
const room = Rooms.findOne(rid, { fields: { sysMes: 1 } });
const hideSettings = settings.collection.findOne('Hide_System_Messages') || {};
Expand All @@ -191,9 +190,7 @@ Template.roomOld.helpers({
const query = {
rid,
_hidden: { $ne: true },
...(!showInMainThread && {
$or: [{ tmid: { $exists: 0 } }, { tshow: { $eq: true } }],
}),
$or: [{ tmid: { $exists: 0 } }, { tshow: { $eq: true } }],
};

if (hideMessagesOfType.size) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const PreferencesMessagesSection = ({ onChange, commitRef, ...props }) => {

const settings = {
unreadAlert: useUserPreference('unreadAlert'),
showMessageInMainThread: useUserPreference('showMessageInMainThread'),
alsoSendThreadToChannel: useUserPreference('alsoSendThreadToChannel'),
useEmojis: useUserPreference('useEmojis'),
convertAsciiEmoji: useUserPreference('convertAsciiEmoji'),
autoImageLoad: useUserPreference('autoImageLoad'),
Expand All @@ -30,7 +30,7 @@ const PreferencesMessagesSection = ({ onChange, commitRef, ...props }) => {

const {
unreadAlert,
showMessageInMainThread,
alsoSendThreadToChannel,
useEmojis,
convertAsciiEmoji,
autoImageLoad,
Expand All @@ -47,7 +47,7 @@ const PreferencesMessagesSection = ({ onChange, commitRef, ...props }) => {

const {
handleUnreadAlert,
handleShowMessageInMainThread,
handleAlsoSendThreadToChannel,
handleUseEmojis,
handleConvertAsciiEmoji,
handleAutoImageLoad,
Expand All @@ -62,6 +62,15 @@ const PreferencesMessagesSection = ({ onChange, commitRef, ...props }) => {
handleMessageViewMode,
} = handlers;

const alsoSendThreadMessageToChannelOptions = useMemo(
() => [
['default', t('Default')],
['always', t('Always')],
['never', t('Never')],
],
[t],
);

const timeFormatOptions = useMemo(
() => [
[0, t('Default')],
Expand Down Expand Up @@ -109,14 +118,19 @@ const PreferencesMessagesSection = ({ onChange, commitRef, ...props }) => {
)}
{useMemo(
() => (
<Field display='flex' flexDirection='row' justifyContent='spaceBetween' flexGrow={1}>
<Field.Label>{t('Show_Message_In_Main_Thread')}</Field.Label>
<Field>
<Field.Label>{t('Also_send_thread_message_to_channel_behavior')}</Field.Label>
<Field.Row>
<ToggleSwitch checked={showMessageInMainThread} onChange={handleShowMessageInMainThread} />
<Select
value={alsoSendThreadToChannel}
onChange={handleAlsoSendThreadToChannel}
options={alsoSendThreadMessageToChannelOptions}
/>
</Field.Row>
<Field.Hint>{t('Accounts_Default_User_Preferences_alsoSendThreadToChannel_Description')}</Field.Hint>
</Field>
),
[handleShowMessageInMainThread, showMessageInMainThread, t],
[alsoSendThreadToChannel, handleAlsoSendThreadToChannel, t, alsoSendThreadMessageToChannelOptions],
)}
{useMemo(
() => (
Expand Down
2 changes: 1 addition & 1 deletion apps/meteor/client/views/admin/settings/Setting.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function Setting({ className = undefined, settingId, sectionChanged }: SettingPr

const label = (t.has(i18nLabel) && t(i18nLabel)) || (t.has(_id) && t(_id));
const hint = useMemo(
() => (t.has(i18nDescription) ? <MarkdownText preserveHtml content={t(i18nDescription)} /> : undefined),
() => (t.has(i18nDescription) ? <MarkdownText variant='inline' preserveHtml content={t(i18nDescription)} /> : undefined),
[i18nDescription, t],
);
const callout = useMemo(() => (alert && t.has(alert) ? <span dangerouslySetInnerHTML={{ __html: t(alert) }} /> : undefined), [alert, t]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import type { IRoom } from '@rocket.chat/core-typings';
import { IMessage } from '@rocket.chat/core-typings';
import { useUserPreference } from '@rocket.chat/ui-contexts';
import { useCallback, useMemo } from 'react';

import { ChatMessage } from '../../../../../app/models/client';
Expand Down Expand Up @@ -43,7 +42,6 @@ const removePossibleNullValues = ({
});

export const useMessages = ({ rid }: { rid: IRoom['_id'] }): IMessage[] => {
const showInMainThread = useUserPreference<boolean>('showMessageInMainThread', false);
// const hideSettings = !!useSetting('Hide_System_Messages');

// const room = Rooms.findOne(rid, { fields: { sysMes: 1 } });
Expand All @@ -53,11 +51,9 @@ export const useMessages = ({ rid }: { rid: IRoom['_id'] }): IMessage[] => {
() => ({
rid,
_hidden: { $ne: true },
...(!showInMainThread && {
$or: [{ tmid: { $exists: 0 } }, { tshow: { $eq: true } }],
}),
$or: [{ tmid: { $exists: 0 } }, { tshow: { $eq: true } }],
}),
[rid, showInMainThread],
[rid],
);

// if (hideMessagesOfType.size) {
Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/packages/rocketchat-i18n/i18n/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@
"Accounts_CustomFieldsToShowInUserInfo": "Custom Fields to Show in User Info",
"Accounts_Default_User_Preferences": "Default User Preferences",
"Accounts_Default_User_Preferences_audioNotifications": "Audio Notifications Default Alert",
"Accounts_Default_User_Preferences_alsoSendThreadToChannel_Description": "Allow users to select the Also send to channel behavior",
"Accounts_Default_User_Preferences_desktopNotifications": "Desktop Notifications Default Alert",
"Accounts_Default_User_Preferences_pushNotifications": "Push Notifications Default Alert",
"Accounts_Default_User_Preferences_not_available": "Failed to retrieve User Preferences because they haven't been set up by the user yet",
Expand Down Expand Up @@ -341,6 +342,7 @@
"Allow_switching_departments": "Allow Visitor to Switch Departments",
"Almost_done": "Almost done",
"Alphabetical": "Alphabetical",
"Also_send_thread_message_to_channel_behavior": "Also send thread message to channel behavior",
"Also_send_to_channel": "Also send to channel",
"Always_open_in_new_window": "Always Open in New Window",
"Analytics": "Analytics",
Expand Down
1 change: 1 addition & 0 deletions apps/meteor/server/startup/migrations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,4 +93,5 @@ import './v266';
import './v267';
import './v268';
import './v269';
import './v271';
import './xrun';
11 changes: 11 additions & 0 deletions apps/meteor/server/startup/migrations/v271.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { Settings } from '@rocket.chat/models';

import { addMigration } from '../../lib/migrations';

// Removes deprecated Show Message In Main Thread preference
addMigration({
version: 271,
async up() {
await Settings.removeById('Accounts_Default_User_Preferences_showMessageInMainThread');
},
});
2 changes: 1 addition & 1 deletion apps/meteor/tests/end-to-end/api/00-miscellaneous.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ describe('miscellaneous', function () {
.expect(200)
.expect((res) => {
const allUserPreferencesKeys = [
'alsoSendThreadToChannel',
// 'language',
'newRoomNotification',
'newMessageNotification',
Expand All @@ -154,7 +155,6 @@ describe('miscellaneous', function () {
'enableAutoAway',
'enableNewMessageTemplate',
// 'highlights',
'showMessageInMainThread',
'desktopNotificationRequireInteraction',
'messageViewMode',
'hideUsernames',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ export type UsersSetPreferencesParamsPOST = {
highlights?: string[];
desktopNotificationRequireInteraction?: boolean;
messageViewMode?: number;
showMessageInMainThread?: boolean;
hideUsernames?: boolean;
hideRoles?: boolean;
displayAvatars?: boolean;
Expand Down Expand Up @@ -122,10 +121,6 @@ const UsersSetPreferencesParamsPostSchema = {
type: 'number',
nullable: true,
},
showMessageInMainThread: {
type: 'boolean',
nullable: true,
},
hideUsernames: {
type: 'boolean',
nullable: true,
Expand Down

0 comments on commit e85a63b

Please sign in to comment.