Tweaks on sequential threads messages layout (#858)

* Tweaks on sequential threads messages

* Update tests

* Fix quote

* Prevent from deleting thread start message when positioned inside the thread

* Remove thread listener from RightButtons

* Fix error on thread start parse

* Stop parsing threads on render

* Check replied thread only if necessary

* Fix messages don't displaying

* Fix threads e2e

* RoomsListView.updateState slice

* Stop fetching hidden messages on threads

* Set initialNumToRender to 5

app/containers/MessageActions.js

@@ -198,6 +198,11 @@ export default class MessageActions extends React.Component {
 		if (this.isRoomReadOnly()) {
 			return false;
 		}
+
+		// Prevent from deleting thread start message when positioned inside the thread
+		if (props.tmid && props.tmid === props.actionMessage._id) {
+			return false;
+		}
 		const deleteOwn = this.isOwn(props);
 		const { Message_AllowDeleting: isDeleteAllowed, Message_AllowDeleting_BlockDeleteInMinutes } = this.props;
 		if (!(this.hasDeletePermission || (isDeleteAllowed && deleteOwn) || this.hasForceDeletePermission)) {

app/containers/MessageBox/index.js

@@ -606,10 +606,10 @@ class MessageBox extends Component {
 			const { replyMessage, closeReply, threadsEnabled } = this.props;
 
 			// Thread
-			if (threadsEnabled) {
+			if (threadsEnabled && replyMessage.mention) {
 				onSubmit(message, replyMessage._id);
 
-			// Legacy reply
+			// Legacy reply or quote (quote is a reply without mention)
 			} else {
 				const { user, roomType } = this.props;
 				const permalink = await this.getPermalink(replyMessage);

app/containers/message/Message.js

@@ -25,6 +25,8 @@ import messagesStatus from '../../constants/messagesStatus';
 import { CustomIcon } from '../../lib/Icons';
 import { COLOR_DANGER } from '../../constants/colors';
 import debounce from '../../utils/debounce';
+import DisclosureIndicator from '../DisclosureIndicator';
+import sharedStyles from '../../views/Styles';
 
 const SYSTEM_MESSAGES = [
 	'r',
@@ -118,6 +120,8 @@ export default class Message extends PureComponent {
 		reactionsModal: PropTypes.bool,
 		type: PropTypes.string,
 		header: PropTypes.bool,
+		isThreadReply: PropTypes.bool,
+		isThreadSequential: PropTypes.bool,
 		avatar: PropTypes.string,
 		alias: PropTypes.string,
 		ts: PropTypes.oneOfType([
@@ -230,17 +234,17 @@ export default class Message extends PureComponent {
 		return status === messagesStatus.ERROR;
 	}
 
-	renderAvatar = () => {
+	renderAvatar = (small = false) => {
 		const {
 			header, avatar, author, baseUrl, user
 		} = this.props;
 		if (header) {
 			return (
 				<Avatar
-					style={styles.avatar}
+					style={small ? styles.avatarSmall : styles.avatar}
 					text={avatar ? '' : author.username}
-					size={36}
-					borderRadius={4}
+					size={small ? 20 : 36}
+					borderRadius={small ? 2 : 4}
 					avatar={avatar}
 					baseUrl={baseUrl}
 					userId={user.id}
@@ -493,14 +497,15 @@ export default class Message extends PureComponent {
 
 		return (
 			<View style={styles.repliedThread} testID={`message-thread-replied-on-${ msg }`}>
-				<CustomIcon name='thread' size={16} style={styles.repliedThreadIcon} />
+				<CustomIcon name='thread' size={20} style={styles.repliedThreadIcon} />
 				<Text style={styles.repliedThreadName} numberOfLines={1}>{msg}</Text>
+				<DisclosureIndicator />
 			</View>
 		);
 	}
 
 	renderInner = () => {
-		const { type, tmid } = this.props;
+		const { type } = this.props;
 		if (type === 'discussion-created') {
 			return (
 				<React.Fragment>
@@ -509,15 +514,6 @@ export default class Message extends PureComponent {
 				</React.Fragment>
 			);
 		}
-		if (tmid) {
-			return (
-				<React.Fragment>
-					{this.renderUsername()}
-					{this.renderRepliedThread()}
-					{this.renderContent()}
-				</React.Fragment>
-			);
-		}
 		return (
 			<React.Fragment>
 				{this.renderUsername()}
@@ -531,9 +527,52 @@ export default class Message extends PureComponent {
 		);
 	}
 
+	renderMessage = () => {
+		const { header, isThreadReply, isThreadSequential } = this.props;
+
+		if (isThreadReply || isThreadSequential || this.isInfoMessage()) {
+			const thread = isThreadReply ? this.renderRepliedThread() : null;
+			return (
+				<React.Fragment>
+					{thread}
+					<View style={[styles.flex, sharedStyles.alignItemsCenter]}>
+						{this.renderAvatar(true)}
+						<View
+							style={[
+								styles.messageContent,
+								header && styles.messageContentWithHeader,
+								this.hasError() && header && styles.messageContentWithHeader,
+								this.hasError() && !header && styles.messageContentWithError,
+								this.isTemp() && styles.temp
+							]}
+						>
+							{this.renderContent()}
+						</View>
+					</View>
+				</React.Fragment>
+			);
+		}
+		return (
+			<View style={styles.flex}>
+				{this.renderAvatar()}
+				<View
+					style={[
+						styles.messageContent,
+						header && styles.messageContentWithHeader,
+						this.hasError() && header && styles.messageContentWithHeader,
+						this.hasError() && !header && styles.messageContentWithError,
+						this.isTemp() && styles.temp
+					]}
+				>
+					{this.renderInner()}
+				</View>
+			</View>
+		);
+	}
+
 	render() {
 		const {
-			editing, style, header, reactionsModal, closeReactions, msg, ts, reactions, author, user, timeFormat, customEmojis, baseUrl
+			editing, style, reactionsModal, closeReactions, msg, ts, reactions, author, user, timeFormat, customEmojis, baseUrl
 		} = this.props;
 		const accessibilityLabel = I18n.t('Message_accessibility', { user: author.username, time: moment(ts).format(timeFormat), message: msg });
 
@@ -545,23 +584,10 @@ export default class Message extends PureComponent {
 					onPress={this.onPress}
 				>
 					<View
-						style={[styles.container, header && styles.marginTop, editing && styles.editing, style]}
+						style={[styles.container, editing && styles.editing, style]}
 						accessibilityLabel={accessibilityLabel}
 					>
-						<View style={styles.flex}>
-							{this.renderAvatar()}
-							<View
-								style={[
-									styles.messageContent,
-									header && styles.messageContentWithHeader,
-									this.hasError() && header && styles.messageContentWithHeader,
-									this.hasError() && !header && styles.messageContentWithError,
-									this.isTemp() && styles.temp
-								]}
-							>
-								{this.renderInner()}
-							</View>
-						</View>
+						{this.renderMessage()}
 						{reactionsModal
 							? (
 								<ReactionsModal

app/containers/message/index.js

@@ -163,6 +163,26 @@ export default class MessageContainer extends React.Component {
 		return true;
 	}
 
+	isThreadReply = () => {
+		const {
+			item, previousItem
+		} = this.props;
+		if (previousItem && item.tmid && (previousItem.tmid !== item.tmid) && (previousItem._id !== item.tmid)) {
+			return true;
+		}
+		return false;
+	}
+
+	isThreadSequential = () => {
+		const {
+			item, previousItem
+		} = this.props;
+		if (previousItem && item.tmid && ((previousItem.tmid === item.tmid) || (previousItem._id === item.tmid))) {
+			return true;
+		}
+		return false;
+	}
+
 	parseMessage = () => {
 		const { item } = this.props;
 		return JSON.parse(JSON.stringify(item));
@@ -201,6 +221,8 @@ export default class MessageContainer extends React.Component {
 				alias={alias}
 				editing={isEditing}
 				header={this.isHeader()}
+				isThreadReply={this.isThreadReply()}
+				isThreadSequential={this.isThreadSequential()}
 				avatar={avatar}
 				user={user}
 				edited={editedBy && !!editedBy.username}

app/containers/message/styles.js

@@ -102,6 +102,9 @@ export default StyleSheet.create({
 	avatar: {
 		marginTop: 4
 	},
+	avatarSmall: {
+		marginLeft: 16
+	},
 	addReaction: {
 		color: COLOR_PRIMARY
 	},
@@ -217,16 +220,18 @@ export default StyleSheet.create({
 	},
 	repliedThread: {
 		flexDirection: 'row',
-		flex: 1
+		flex: 1,
+		alignItems: 'center',
+		marginTop: 6,
+		marginBottom: 12
 	},
 	repliedThreadIcon: {
 		color: COLOR_PRIMARY,
-		marginRight: 2
+		marginRight: 10,
+		marginLeft: 16
 	},
 	repliedThreadName: {
-		fontSize: 14,
-		lineHeight: 16,
-		fontStyle: 'normal',
+		fontSize: 16,
 		flex: 1,
 		color: COLOR_PRIMARY,
 		...sharedStyles.textRegular

app/lib/methods/loadThreadMessages.js

@@ -9,7 +9,7 @@ async function load({ tmid, offset }) {
 	try {
 		// RC 1.0
 		const result = await this.sdk.get('chat.getThreadMessages', {
-			tmid, count: 50, offset, sort: { ts: -1 }
+			tmid, count: 50, offset, sort: { ts: -1 }, query: { _hidden: { $ne: true } }
 		});
 		if (!result || !result.success) {
 			return [];

app/views/RoomView/Header/RightButtons.js

@@ -48,6 +48,12 @@ class RightButtonsContainer extends React.PureComponent {
 		};
 	}
 
+	componentWillUnmount() {
+		if (this.thread && this.thread.removeAllListeners) {
+			this.thread.removeAllListeners();
+		}
+	}
+
 	updateThread = () => {
 		const { userId } = this.props;
 		this.setState({

app/views/RoomView/List.js

@@ -59,23 +59,24 @@ export class List extends React.PureComponent {
 		if (this.updateState && this.updateState.stop) {
 			this.updateState.stop();
 		}
-		if (this.updateThreads && this.updateThreads.stop) {
-			this.updateThreads.stop();
-		}
 		if (this.interactionManagerState && this.interactionManagerState.cancel) {
 			this.interactionManagerState.cancel();
 		}
-		if (this.interactionManagerThreads && this.interactionManagerThreads.cancel) {
-			this.interactionManagerThreads.cancel();
-		}
 		console.countReset(`${ this.constructor.name }.render calls`);
 	}
 
 	// eslint-disable-next-line react/sort-comp
 	updateState = debounce(() => {
 		this.interactionManagerState = InteractionManager.runAfterInteractions(() => {
+			const { tmid } = this.props;
+			let messages = this.data;
+			if (tmid && this.threads[0]) {
+				const thread = { ...this.threads[0] };
+				thread.tlm = null;
+				messages = [...messages, thread];
+			}
 			this.setState({
-				messages: this.data.slice(),
+				messages: messages.slice(),
 				threads: this.threads.slice(),
 				loading: false
 			});
@@ -95,7 +96,8 @@ export class List extends React.PureComponent {
 		try {
 			let result;
 			if (tmid) {
-				result = await RocketChat.loadThreadMessages({ tmid, offset: messages.length });
+				// `offset` is `messages.length - 1` because we append thread start to `messages` obj
+				result = await RocketChat.loadThreadMessages({ tmid, offset: messages.length - 1 });
 			} else {
 				result = await RocketChat.loadMessagesForRoom({ rid, t, latest: messages[messages.length - 1].ts });
 			}
@@ -130,31 +132,22 @@ export class List extends React.PureComponent {
 
 	render() {
 		console.count(`${ this.constructor.name }.render calls`);
-		const { messages, threads } = this.state;
-		const { tmid } = this.props;
-		let data = [];
-		if (tmid) {
-			const thread = { ...threads[0] };
-			thread.tlm = null;
-			data = [...messages, thread];
-		} else {
-			data = messages;
-		}
+		const { messages } = this.state;
 		return (
 			<React.Fragment>
-				<EmptyRoom length={data.length} />
+				<EmptyRoom length={messages.length} />
 				<FlatList
 					testID='room-view-messages'
 					ref={ref => this.list = ref}
 					keyExtractor={item => item._id}
-					data={data}
+					data={messages}
 					extraData={this.state}
 					renderItem={this.renderItem}
 					contentContainerStyle={styles.contentContainer}
 					style={styles.list}
 					inverted
 					removeClippedSubviews
-					initialNumToRender={1}
+					initialNumToRender={5}
 					onEndReached={this.onEndReached}
 					onEndReachedThreshold={0.5}
 					maxToRenderPerBatch={5}

app/views/RoomView/index.js

@@ -510,7 +510,7 @@ export default class RoomView extends LoggedView {
 		return (
 			<React.Fragment>
 				{room._id && showActions
-					? <MessageActions room={room} user={user} />
+					? <MessageActions room={room} tmid={this.tmid} user={user} />
 					: null
 				}
 				{showErrorActions ? <MessageErrorActions /> : null}

app/views/RoomsListView/index.js

@@ -271,14 +271,14 @@ export default class RoomsListView extends LoggedView {
 	updateState = debounce(() => {
 		this.updateStateInteraction = InteractionManager.runAfterInteractions(() => {
 			this.internalSetState({
-				chats: this.chats,
-				unread: this.unread,
-				favorites: this.favorites,
-				discussions: this.discussions,
-				channels: this.channels,
-				privateGroup: this.privateGroup,
-				direct: this.direct,
-				livechat: this.livechat,
+				chats: this.chats ? this.chats.slice() : [],
+				unread: this.unread ? this.unread.slice() : [],
+				favorites: this.favorites ? this.favorites.slice() : [],
+				discussions: this.discussions ? this.discussions.slice() : [],
+				channels: this.channels ? this.channels.slice() : [],
+				privateGroup: this.privateGroup ? this.privateGroup.slice() : [],
+				direct: this.direct ? this.direct.slice() : [],
+				livechat: this.livechat ? this.livechat.slice() : [],
 				loading: false
 			});
 			this.forceUpdate();

app/views/Styles.js

@@ -66,6 +66,9 @@ export default StyleSheet.create({
 	alignItemsFlexStart: {
 		alignItems: 'flex-start'
 	},
+	alignItemsCenter: {
+		alignItems: 'center'
+	},
 	textAlignRight: {
 		textAlign: 'right'
 	},

e2e/08-room.spec.js

@@ -286,8 +286,6 @@ describe('Room screen', () => {
 				await element(by.id('messagebox-send-message')).tap();
 				await waitFor(element(by.id(`message-thread-button-${ thread }`))).toExist().withTimeout(5000);
 				await expect(element(by.id(`message-thread-button-${ thread }`))).toExist();
-				await waitFor(element(by.id(`message-thread-replied-on-${ thread }`))).toExist().withTimeout(5000);
-				await expect(element(by.id(`message-thread-replied-on-${ thread }`))).toExist();
 			});
 
 			it('should navigate to thread from button', async() => {
@@ -313,6 +311,16 @@ describe('Room screen', () => {
 			});
 
 			it('should navigate to thread from thread name', async() => {
+				await mockMessage('dummymessagebetweenthethread');
+				await element(by.text(thread)).longPress();
+				await waitFor(element(by.text('Message actions'))).toBeVisible().withTimeout(5000);
+				await expect(element(by.text('Message actions'))).toBeVisible();
+				await element(by.text('Reply')).tap();
+				await element(by.id('messagebox-input')).typeText('repliedagain');
+				await element(by.id('messagebox-send-message')).tap();
+				await waitFor(element(by.id(`message-thread-replied-on-${ thread }`))).toExist().withTimeout(5000);
+				await expect(element(by.id(`message-thread-replied-on-${ thread }`))).toExist();
+
 				await element(by.id(`message-thread-replied-on-${ thread }`)).tap();
 				await waitFor(element(by.id('room-view'))).toBeVisible().withTimeout(5000);
 				await waitFor(element(by.id(`room-view-title-${ thread }`))).toBeVisible().withTimeout(5000);

storybook/stories/Message.js

@@ -298,31 +298,37 @@ export default (
 			msg="I'm fine!"
 			tmid='1'
 			tmsg='How are you?'
+			isThreadReply
 		/>
 		<Message
 			msg="I'm fine!"
 			tmid='1'
 			tmsg='Thread with emoji :) :joy:'
+			isThreadReply
 		/>
 		<Message
 			msg="I'm fine!"
 			tmid='1'
 			tmsg='Markdown: [link](http://www.google.com/) ```block code```'
+			isThreadReply
 		/>
 		<Message
 			msg="I'm fine!"
 			tmid='1'
 			tmsg={longText}
+			isThreadReply
 		/>
 		<Message
 			msg={longText}
 			tmid='1'
 			tmsg='How are you?'
+			isThreadReply
 		/>
 		<Message
 			msg={longText}
 			tmid='1'
 			tmsg={longText}
+			isThreadReply
 		/>
 		<Message
 			tmid='1'
@@ -332,6 +338,60 @@ export default (
 				description: 'This is a description',
 				audio_url: '/file-upload/c4wcNhrbXJLBvAJtN/1535569819516.aac'
 			}]}
+			isThreadReply
+		/>
+
+		<Separator title='Sequential thread messages following thread button' />
+		<Message
+			msg='How are you?'
+			tcount={1}
+			tlm={date}
+		/>
+		<Message
+			msg="I'm fine!"
+			tmid='1'
+			isThreadSequential
+		/>
+		<Message
+			msg={longText}
+			tmid='1'
+			isThreadSequential
+		/>
+		<Message
+			attachments={[{
+				title: 'This is a title',
+				description: 'This is a description',
+				audio_url: '/file-upload/c4wcNhrbXJLBvAJtN/1535569819516.aac'
+			}]}
+			tmid='1'
+			isThreadSequential
+		/>
+
+		<Separator title='Sequential thread messages following thread reply' />
+		<Message
+			msg="I'm fine!"
+			tmid='1'
+			tmsg='How are you?'
+			isThreadReply
+		/>
+		<Message
+			msg='Cool!'
+			tmid='1'
+			isThreadSequential
+		/>
+		<Message
+			msg={longText}
+			tmid='1'
+			isThreadSequential
+		/>
+		<Message
+			attachments={[{
+				title: 'This is a title',
+				description: 'This is a description',
+				audio_url: '/file-upload/c4wcNhrbXJLBvAJtN/1535569819516.aac'
+			}]}
+			tmid='1'
+			isThreadSequential
 		/>
 
 		{/* <Message