From f9ecfccc0a6d4c8918b87d2318928c7397b3f80d Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Thu, 29 Jul 2021 13:44:38 +1000 Subject: [PATCH] batch UI updates of message added events --- .../DataExtractionNotification.tsx | 2 +- .../conversation/GroupInvitation.tsx | 2 +- .../conversation/GroupNotification.tsx | 2 +- ts/components/conversation/Message.tsx | 77 ++++++++++- ts/components/conversation/MessageDetail.tsx | 6 +- .../conversation/ReadableMessage.tsx | 13 +- .../conversation/message/MessageMetadata.tsx | 6 +- .../conversation/message/MetadataBadge.tsx | 6 +- .../SessionMessagesListContainer.tsx | 122 +++--------------- ts/models/message.ts | 19 ++- ts/receiver/queuedJob.ts | 31 ++++- ts/session/types/PubKey.ts | 2 +- ts/state/ducks/conversations.ts | 44 ++++++- ts/state/selectors/conversations.ts | 47 +++---- 14 files changed, 211 insertions(+), 168 deletions(-) diff --git a/ts/components/conversation/DataExtractionNotification.tsx b/ts/components/conversation/DataExtractionNotification.tsx index a92de9afb..fde513c1e 100644 --- a/ts/components/conversation/DataExtractionNotification.tsx +++ b/ts/components/conversation/DataExtractionNotification.tsx @@ -23,7 +23,7 @@ export const DataExtractionNotification = (props: PropsForDataExtractionNotifica flexDirection="column" alignItems="center" margin={theme.common.margins.sm} - id={messageId} + id={`data-extraction-${messageId}`} > { const openGroupInvitation = window.i18n('openGroupInvitation'); return ( -
+
{ const { changes } = props; return ( -
+
{(changes || []).map((change, index) => (
{renderChange(change)} diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx index 92df95024..5a1c2b2a5 100644 --- a/ts/components/conversation/Message.tsx +++ b/ts/components/conversation/Message.tsx @@ -39,12 +39,20 @@ import { getMessageById } from '../../data/data'; import { connect } from 'react-redux'; import { StateType } from '../../state/reducer'; import { + areMoreMessagesBeingFetched, + getLoadedMessagesLength, + getMostRecentMessageId, + getOldestMessageId, getQuotedMessageToAnimate, + getSelectedConversationKey, getSelectedMessageIds, } from '../../state/selectors/conversations'; import { + fetchMessagesForConversation, + markConversationFullyRead, messageExpired, showLightBox, + showScrollToBottomButton, toggleSelectedMessageId, } from '../../state/ducks/conversations'; import { saveAttachmentToDisk } from '../../util/attachmentsUtil'; @@ -54,6 +62,7 @@ import { ReadableMessage } from './ReadableMessage'; import { isElectronWindowFocused } from '../../session/utils/WindowUtils'; import { getConversationController } from '../../session/conversations'; import { MessageMetadata } from './message/MessageMetadata'; +import { Constants } from '../../session'; // Same as MIN_WIDTH in ImageGrid.tsx const MINIMUM_LINK_PREVIEW_IMAGE_WIDTH = 200; @@ -70,6 +79,11 @@ const EXPIRED_DELAY = 600; type Props = MessageRenderingProps & { selectedMessages: Array; quotedMessageToAnimate: string | undefined; + mostRecentMessageId: string | undefined; + oldestMessageId: string | undefined; + areMoreMessagesBeingFetched: boolean; + loadedMessagesLength: number; + selectedConversationKey: string | undefined; }; function attachmentIsAttachmentTypeWithPath(attac: any): attac is AttachmentTypeWithPath { @@ -131,6 +145,7 @@ class MessageInner extends React.PureComponent { imageBroken: false, }; this.ctxMenuID = `ctx-menu-message-${uuid()}`; + this.loadMoreMessages = _.debounce(this.loadMoreMessages, 100); } public componentDidMount() { @@ -594,14 +609,21 @@ class MessageInner extends React.PureComponent { // tslint:disable-next-line: cyclomatic-complexity public render() { - const { direction, id, conversationType, isUnread, selectedMessages } = this.props; + const { + direction, + id: messageId, + conversationType, + areMoreMessagesBeingFetched: fetchingMore, + isUnread, + selectedMessages, + } = this.props; const { expired, expiring } = this.state; if (expired) { return null; } - const selected = selectedMessages.includes(id) || false; + const selected = selectedMessages.includes(messageId) || false; const width = this.getWidth(); const isShowingImage = this.isShowingImage(); @@ -618,13 +640,37 @@ class MessageInner extends React.PureComponent { divClasses.push('public-chat-message-wrapper'); } - if (this.props.quotedMessageToAnimate === this.props.id) { + if (this.props.quotedMessageToAnimate === messageId) { divClasses.push('flash-green-once'); } const onVisible = async (inView: boolean | Object) => { + // we are the bottom message + if (this.props.mostRecentMessageId === messageId) { + if (inView === true) { + window.inboxStore?.dispatch(showScrollToBottomButton(false)); + void getConversationController() + .get(this.props.selectedConversationKey as string) + ?.markRead(this.props.receivedAt || 0) + .then(() => { + window.inboxStore?.dispatch( + markConversationFullyRead(this.props.selectedConversationKey as string) + ); + }); + } else if (inView === false) { + window.inboxStore?.dispatch(showScrollToBottomButton(true)); + } + } + console.warn('oldestMessageId', this.props.oldestMessageId); + console.warn('mostRecentMessageId', this.props.mostRecentMessageId); + console.warn('messageId', messageId); + if (inView === true && this.props.oldestMessageId === messageId && !fetchingMore) { + console.warn('loadMoreMessages'); + + this.loadMoreMessages(); + } if (inView === true && shouldMarkReadWhenVisible && isElectronWindowFocused()) { - const found = await getMessageById(id); + const found = await getMessageById(messageId); if (found && Boolean(found.get('unread'))) { // mark the message as read. @@ -636,11 +682,11 @@ class MessageInner extends React.PureComponent { return ( {this.renderAvatar()}
{ {this.renderText()} { ); } + private loadMoreMessages() { + const { loadedMessagesLength, selectedConversationKey } = this.props; + + const numMessages = loadedMessagesLength + Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT; + (window.inboxStore?.dispatch as any)( + fetchMessagesForConversation({ + conversationKey: selectedConversationKey as string, + count: numMessages, + }) + ); + } + private handleContextMenu(e: any) { e.preventDefault(); e.stopPropagation(); @@ -869,6 +927,11 @@ const mapStateToProps = (state: StateType) => { return { selectedMessages: getSelectedMessageIds(state), quotedMessageToAnimate: getQuotedMessageToAnimate(state), + mostRecentMessageId: getMostRecentMessageId(state), + oldestMessageId: getOldestMessageId(state), + areMoreMessagesBeingFetched: areMoreMessagesBeingFetched(state), + selectedConversationKey: getSelectedConversationKey(state), + loadedMessagesLength: getLoadedMessagesLength(state), }; }; diff --git a/ts/components/conversation/MessageDetail.tsx b/ts/components/conversation/MessageDetail.tsx index a42304ac3..db3a81c5f 100644 --- a/ts/components/conversation/MessageDetail.tsx +++ b/ts/components/conversation/MessageDetail.tsx @@ -20,14 +20,14 @@ const AvatarItem = (props: { contact: ContactPropsMessageDetail }) => { ); }; -const DeleteButtonItem = (props: { id: string; convoId: string; isDeletable: boolean }) => { +const DeleteButtonItem = (props: { messageId: string; convoId: string; isDeletable: boolean }) => { const { i18n } = window; return props.isDeletable ? (
diff --git a/ts/components/conversation/ReadableMessage.tsx b/ts/components/conversation/ReadableMessage.tsx index 11e17c457..bcd845261 100644 --- a/ts/components/conversation/ReadableMessage.tsx +++ b/ts/components/conversation/ReadableMessage.tsx @@ -4,18 +4,25 @@ import { InView, useInView } from 'react-intersection-observer'; type ReadableMessageProps = { children: React.ReactNode; - id: string; + messageId: string; className: string; onChange: (inView: boolean) => void; onContextMenu: (e: any) => void; }; export const ReadableMessage = (props: ReadableMessageProps) => { - const { onChange } = props; + const { onChange, messageId } = props; useFocus(onChange); return ( - + {props.children} ); diff --git a/ts/components/conversation/message/MessageMetadata.tsx b/ts/components/conversation/message/MessageMetadata.tsx index 84bc58e39..d2359f3b6 100644 --- a/ts/components/conversation/message/MessageMetadata.tsx +++ b/ts/components/conversation/message/MessageMetadata.tsx @@ -10,7 +10,7 @@ import { MessageDeliveryStatus, MessageModelType } from '../../../models/message type Props = { isAdmin?: boolean; text?: string | null; - id: string; + messageId: string; collapseMetadata?: boolean; direction: MessageModelType; timestamp: number; @@ -53,7 +53,7 @@ const MetadatasContainer = styled.div<{ withImageNoCaption: boolean }>` */ export const MessageMetadata = (props: Props) => { const { - id, + messageId, collapseMetadata, direction, expirationLength, @@ -95,7 +95,7 @@ export const MessageMetadata = (props: Props) => { direction={direction} isPublic={isPublic} isAdmin={isAdmin} - id={id} + messageId={messageId} withImageNoCaption={withImageNoCaption} /> diff --git a/ts/components/conversation/message/MetadataBadge.tsx b/ts/components/conversation/message/MetadataBadge.tsx index a8ebca367..1edc65603 100644 --- a/ts/components/conversation/message/MetadataBadge.tsx +++ b/ts/components/conversation/message/MetadataBadge.tsx @@ -38,7 +38,7 @@ export const MetadataBadge = (props: BadgeProps): JSX.Element => { }; type BadgesProps = { - id: string; + messageId: string; direction: string; isPublic?: boolean; isAdmin?: boolean; @@ -46,7 +46,7 @@ type BadgesProps = { }; export const MetadataBadges = (props: BadgesProps): JSX.Element => { - const { id, direction, isPublic, isAdmin, withImageNoCaption } = props; + const { messageId, direction, isPublic, isAdmin, withImageNoCaption } = props; const badges = [(isPublic && 'Public') || null, (isPublic && isAdmin && 'Mod') || null].filter( nonNullish ); @@ -57,7 +57,7 @@ export const MetadataBadges = (props: BadgesProps): JSX.Element => { const badgesElements = badges.map(badgeText => ( { @@ -57,7 +56,6 @@ class SessionMessagesListContainerInner extends React.Component { autoBind(this); this.ignoreScrollEvents = true; - this.triggerFetchMoreMessages = _.debounce(this.triggerFetchMoreMessages, 100); } // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ @@ -75,7 +73,11 @@ class SessionMessagesListContainerInner extends React.Component { } } - public componentDidUpdate(prevProps: Props, _prevState: any, snapshot: any) { + public componentDidUpdate( + prevProps: Props, + _prevState: any, + snapshot: { scrollHeight: number; scrollTop: number } + ) { const isSameConvo = prevProps.conversationKey === this.props.conversationKey; const messageLengthChanged = prevProps.messagesProps.length !== this.props.messagesProps.length; if ( @@ -98,7 +100,17 @@ class SessionMessagesListContainerInner extends React.Component { this.ignoreScrollEvents = true; const list = this.props.messageContainerRef.current; - list.scrollTop = list.scrollHeight - snapshot; + + // if we added a message at the top, keep position from the bottom. + if ( + prevProps.messagesProps[0].propsForMessage.id === + this.props.messagesProps[0].propsForMessage.id + ) { + // list.scrollTop = list.scrollHeight - (snapshot.scrollHeight - snapshot.scrollTop); + } else { + // if we added a message at the bottom, keep position from the bottom. + // list.scrollTop = snapshot.scrollTop; + } this.ignoreScrollEvents = false; } } @@ -110,7 +122,8 @@ class SessionMessagesListContainerInner extends React.Component { // Capture the scroll position so we can adjust scroll later. if (prevProps.messagesProps.length < this.props.messagesProps.length) { const list = this.props.messageContainerRef.current; - return list.scrollHeight - list.scrollTop; + + return { scrollHeight: list.scrollHeight, scrollTop: list.scrollTop }; } return null; } @@ -210,91 +223,8 @@ class SessionMessagesListContainerInner extends React.Component { // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // ~~~~~~~~~~~~ SCROLLING METHODS ~~~~~~~~~~~~~ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - private async handleScroll() { - const messageContainer = this.props.messageContainerRef?.current; - - const { conversationKey } = this.props; - if (!messageContainer || !conversationKey) { - return; - } + private handleScroll() { contextMenu.hideAll(); - - if (this.ignoreScrollEvents) { - return; - } - // nothing to do if there are no message loaded - if (!this.props.messagesProps || this.props.messagesProps.length === 0) { - return; - } - - // ---- First lets see if we need to show the scroll to bottom button, without using clientHeight (which generates a full layout recalculation) - // get the message the most at the bottom - const bottomMessageId = this.props.messagesProps[0].propsForMessage.id; - const bottomMessageDomElement = document.getElementById(bottomMessageId); - - // get the message the most at the top - const topMessageId = this.props.messagesProps[this.props.messagesProps.length - 1] - .propsForMessage.id; - const topMessageDomElement = document.getElementById(topMessageId); - - const containerTop = messageContainer.getBoundingClientRect().top; - const containerBottom = messageContainer.getBoundingClientRect().bottom; - - // First handle what we gotta handle with the bottom message position - // either the showScrollButton or the markRead of all messages - if (!bottomMessageDomElement) { - window.log.warn('Could not find dom element for handle scroll'); - } else { - const topOfBottomMessage = bottomMessageDomElement.getBoundingClientRect().top; - const bottomOfBottomMessage = bottomMessageDomElement.getBoundingClientRect().bottom; - - // this is our limit for the showScrollDownButton. - const showScrollButton = topOfBottomMessage > window.innerHeight; - window.inboxStore?.dispatch(showScrollToBottomButton(showScrollButton)); - - // trigger markRead if we hit the bottom - const isScrolledToBottom = bottomOfBottomMessage <= containerBottom - 5; - if (isScrolledToBottom) { - // Mark messages read - this.updateReadMessages(true); - } - } - - // Then, see if we need to fetch more messages because the top message it - - if (!topMessageDomElement) { - window.log.warn('Could not find dom top element for handle scroll'); - } else { - const topTopMessage = topMessageDomElement.getBoundingClientRect().top; - - // this is our limit for the showScrollDownButton. - const shouldFetchMore = - topTopMessage > containerTop - 10 && !this.props.areMoreMessagesBeingFetched; - - if (shouldFetchMore) { - const { messagesProps } = this.props; - - const oldLen = messagesProps.length; - const previousTopMessage = messagesProps[oldLen - 1]?.propsForMessage.id; - - this.triggerFetchMoreMessages(); - if (previousTopMessage && oldLen !== messagesProps.length) { - this.scrollToMessage(previousTopMessage); - } - } - } - } - - private triggerFetchMoreMessages() { - const { messagesProps } = this.props; - - const numMessages = messagesProps.length + Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT; - (window.inboxStore?.dispatch as any)( - fetchMessagesForConversation({ - conversationKey: this.props.conversationKey as string, - count: numMessages, - }) - ); } /** @@ -363,19 +293,8 @@ class SessionMessagesListContainerInner extends React.Component { if (!messageContainer) { return; } + console.warn('scrollToBottom on messageslistcontainer'); messageContainer.scrollTop = messageContainer.scrollHeight - messageContainer.clientHeight; - const { messagesProps, conversationKey } = this.props; - - if (!messagesProps || messagesProps.length === 0 || !conversationKey) { - return; - } - - const conversation = getConversationController().get(conversationKey); - if (isElectronWindowFocused()) { - void conversation.markRead(Date.now()).then(() => { - window.inboxStore?.dispatch(markConversationFullyRead(conversationKey)); - }); - } } private async scrollToQuoteMessage(options: QuoteClickOptions) { @@ -449,7 +368,6 @@ const mapStateToProps = (state: StateType) => { messagesProps: getSortedMessagesOfSelectedConversation(state), showScrollButton: getShowScrollButton(state), animateQuotedMessageId: getQuotedMessageToAnimate(state), - areMoreMessagesBeingFetched: areMoreMessagesBeingFetched(state), }; }; diff --git a/ts/models/message.ts b/ts/models/message.ts index 405fc0180..643e391f0 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -27,6 +27,7 @@ import { LastMessageStatusType, MessageModelProps, MessagePropsDetails, + messagesChanged, PropsForAttachment, PropsForExpirationTimer, PropsForGroupInvitation, @@ -81,8 +82,6 @@ export class MessageModel extends Backbone.Model { } autoBind(this); - this.dispatchMessageUpdate = _.throttle(this.dispatchMessageUpdate, 300); - window.contextMenuShown = false; this.getProps(); @@ -1180,9 +1179,23 @@ export class MessageModel extends Backbone.Model { } } private dispatchMessageUpdate() { - window.inboxStore?.dispatch(conversationActions.messageChanged(this.getProps())); + trotthledAllMessagesDispatch(); + console.warn('adding dispatch for:', this.id); + + updatesToDispatch.set(this.id, this.getProps()); } } + +const trotthledAllMessagesDispatch = _.throttle(() => { + if (updatesToDispatch.size === 0) { + return; + } + console.warn('TRIGGERING ALL DISPATCH'); + window.inboxStore?.dispatch(messagesChanged([...updatesToDispatch.values()])); + updatesToDispatch.clear(); +}, 1000); + +const updatesToDispatch: Map = new Map(); export class MessageCollection extends Backbone.Collection {} MessageCollection.prototype.model = MessageModel; diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index 940d1b3bc..d71e0129a 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -10,7 +10,11 @@ import { ConversationModel, ConversationTypeEnum } from '../models/conversation' import { MessageModel } from '../models/message'; import { getMessageController } from '../session/messages'; import { getMessageById, getMessagesBySentAt } from '../../ts/data/data'; -import { actions as conversationActions } from '../state/ducks/conversations'; +import { + actions as conversationActions, + MessageModelProps, + messagesAdded, +} from '../state/ducks/conversations'; import { updateProfileOneAtATime } from './dataMessage'; import Long from 'long'; @@ -469,13 +473,12 @@ export async function handleMessageJob( // this updates the redux store. // if the convo on which this message should become visible, // it will be shown to the user, and might as well be read right away - window.inboxStore?.dispatch( - conversationActions.messageAdded({ - conversationKey: conversation.id, - messageModelProps: message.getProps(), - }) - ); + updatesToDispatch.set(message.id, { + conversationKey: conversation.id, + messageModelProps: message.getProps(), + }); + trotthledAllMessagesAddedDispatch(); if (message.get('unread')) { await conversation.throttledNotify(message); } @@ -490,3 +493,17 @@ export async function handleMessageJob( throw error; } } + +const trotthledAllMessagesAddedDispatch = _.throttle(() => { + if (updatesToDispatch.size === 0) { + return; + } + console.warn('TRIGGERING ALL ADDED DISPATCH'); + window.inboxStore?.dispatch(messagesAdded([...updatesToDispatch.values()])); + updatesToDispatch.clear(); +}, 1000); + +const updatesToDispatch: Map< + string, + { conversationKey: string; messageModelProps: MessageModelProps } +> = new Map(); diff --git a/ts/session/types/PubKey.ts b/ts/session/types/PubKey.ts index 63adcd9d9..3d23273bd 100644 --- a/ts/session/types/PubKey.ts +++ b/ts/session/types/PubKey.ts @@ -2,7 +2,7 @@ import { fromHexToArray } from '../utils/String'; export class PubKey { public static readonly PUBKEY_LEN = 66; - private static readonly HEX = '[0-9a-fA-F]'; + public static readonly HEX = '[0-9a-fA-F]'; // This is a temporary fix to allow groupPubkeys created from mobile to be handled correctly // They have a different regex to match diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 0940c65b2..21a3d3f69 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -350,19 +350,34 @@ function getEmptyState(): ConversationsStateType { function handleMessageAdded( state: ConversationsStateType, - action: PayloadAction<{ + payload: { conversationKey: string; messageModelProps: MessageModelProps; - }> + } ) { const { messages } = state; - const { conversationKey, messageModelProps: addedMessageProps } = action.payload; + const { conversationKey, messageModelProps: addedMessageProps } = payload; if (conversationKey === state.selectedConversation) { - const messagesWithNewMessage = [...messages, addedMessageProps]; + const messageInStoreIndex = state?.messages?.findIndex( + m => m.propsForMessage.id === addedMessageProps.propsForMessage.id + ); + if (messageInStoreIndex >= 0) { + // we cannot edit the array directly, so slice the first part, insert our edited message, and slice the second part + const editedMessages = [ + ...state.messages.slice(0, messageInStoreIndex), + addedMessageProps, + ...state.messages.slice(messageInStoreIndex + 1), + ]; + + return { + ...state, + messages: editedMessages, + }; + } return { ...state, - messages: messagesWithNewMessage, + messages: [...messages, addedMessageProps], // sorting happens in the selector }; } return state; @@ -564,7 +579,23 @@ const conversationsSlice = createSlice({ messageModelProps: MessageModelProps; }> ) { - return handleMessageAdded(state, action); + return handleMessageAdded(state, action.payload); + }, + messagesAdded( + state: ConversationsStateType, + action: PayloadAction< + Array<{ + conversationKey: string; + messageModelProps: MessageModelProps; + }> + > + ) { + action.payload.forEach(added => { + // tslint:disable-next-line: no-parameter-reassignment + state = handleMessageAdded(state, added); + }); + + return state; }, messageChanged(state: ConversationsStateType, action: PayloadAction) { @@ -717,6 +748,7 @@ export const { removeAllConversations, messageExpired, messageAdded, + messagesAdded, messageDeleted, conversationReset, messageChanged, diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index f24683956..e8db5a8df 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -402,39 +402,32 @@ function sortMessages( return messagesSorted; } -function getFirstMessageUnreadIndex(messages: Array) { - if (!messages || messages.length === 0) { - return -1; - } - - // this is to handle the case where 50 messages are loaded, some of them are already read at the top, but some not loaded yet are still unread. - if ( - messages.length < - getConversationController() - .get(messages[0].propsForMessage.convoId) - ?.get('unreadCount') - ) { - return -2; +export const getFirstUnreadMessageId = createSelector( + getConversations, + (state: ConversationsStateType): string | undefined => { + return state.firstUnreadMessageId; } +); - // iterate over the incoming messages from the oldest one. the first one with isUnread !== undefined is our first unread - for (let index = messages.length - 1; index > 0; index--) { - const message = messages[index]; - if ( - message.propsForMessage.direction === 'incoming' && - message.propsForMessage.isUnread === true - ) { - return index; - } +export const getMostRecentMessageId = createSelector( + getConversations, + (state: ConversationsStateType): string | undefined => { + return state.messages.length ? state.messages[0].propsForMessage.id : undefined; } +); - return -1; -} +export const getOldestMessageId = createSelector(getConversations, (state: ConversationsStateType): + | string + | undefined => { + return state.messages.length + ? state.messages[state.messages.length - 1].propsForMessage.id + : undefined; +}); -export const getFirstUnreadMessageId = createSelector( +export const getLoadedMessagesLength = createSelector( getConversations, - (state: ConversationsStateType): string | undefined => { - return state.firstUnreadMessageId; + (state: ConversationsStateType): number => { + return state.messages.length || 0; } );