From a3525c16efb16952d22e8859d97135d089cc0846 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Fri, 19 Nov 2021 09:19:55 -0800 Subject: [PATCH] When you send a message, scroll it into view --- ts/components/CompositionArea.stories.tsx | 1 - ts/components/CompositionArea.tsx | 6 +- ts/components/CompositionInput.stories.tsx | 2 - ts/components/CompositionInput.tsx | 5 - ts/components/ForwardMessageModal.stories.tsx | 1 - ts/components/ForwardMessageModal.tsx | 16 +- .../conversation/Timeline.stories.tsx | 4 - ts/components/conversation/Timeline.tsx | 20 +- ts/models/conversations.ts | 357 ++++++++++++++++-- ts/state/ducks/conversations.ts | 83 ++-- ts/state/selectors/conversations.ts | 12 +- ts/state/smart/ForwardMessageModal.tsx | 3 - .../state/ducks/conversations_test.ts | 2 - ts/views/conversation_view.ts | 313 +-------------- 14 files changed, 387 insertions(+), 438 deletions(-) diff --git a/ts/components/CompositionArea.stories.tsx b/ts/components/CompositionArea.stories.tsx index 2702294e06..ed778ea2fd 100644 --- a/ts/components/CompositionArea.stories.tsx +++ b/ts/components/CompositionArea.stories.tsx @@ -68,7 +68,6 @@ const useProps = (overrideProps: Partial = {}): Props => ({ clearQuotedMessage: action('clearQuotedMessage'), getPreferredBadge: () => undefined, getQuotedMessage: action('getQuotedMessage'), - scrollToBottom: action('scrollToBottom'), sortedGroupMembers: [], // EmojiButton onPickEmoji: action('onPickEmoji'), diff --git a/ts/components/CompositionArea.tsx b/ts/components/CompositionArea.tsx index 1ab63f89b1..5baa1679f9 100644 --- a/ts/components/CompositionArea.tsx +++ b/ts/components/CompositionArea.tsx @@ -123,7 +123,6 @@ export type OwnProps = Readonly<{ setQuotedMessage(message: undefined): unknown; shouldSendHighQualityAttachments: boolean; startRecording: () => unknown; - scrollToBottom: (converstionId: string) => unknown; theme: ThemeType; }>; @@ -203,7 +202,6 @@ export const CompositionArea = ({ clearQuotedMessage, getPreferredBadge, getQuotedMessage, - scrollToBottom, sortedGroupMembers, // EmojiButton onPickEmoji, @@ -630,14 +628,13 @@ export const CompositionArea = ({ {!large ? leftHandSideButtonsFragment : null}
= {}): Props => ({ i18n, - conversationId: 'conversation-id', disabled: boolean('disabled', overrideProps.disabled || false), onSubmit: action('onSubmit'), onEditorStateChange: action('onEditorStateChange'), @@ -33,7 +32,6 @@ const useProps = (overrideProps: Partial = {}): Props => ({ getQuotedMessage: action('getQuotedMessage'), onPickEmoji: action('onPickEmoji'), large: boolean('large', overrideProps.large || false), - scrollToBottom: action('scrollToBottom'), sortedGroupMembers: overrideProps.sortedGroupMembers || [], skinTone: select( 'skinTone', diff --git a/ts/components/CompositionInput.tsx b/ts/components/CompositionInput.tsx index f106150ed6..834b24b5aa 100644 --- a/ts/components/CompositionInput.tsx +++ b/ts/components/CompositionInput.tsx @@ -62,7 +62,6 @@ export type InputApi = { export type Props = { readonly i18n: LocalizerType; - readonly conversationId: string; readonly disabled?: boolean; readonly getPreferredBadge: PreferredBadgeSelectorType; readonly large?: boolean; @@ -88,7 +87,6 @@ export type Props = { ): unknown; getQuotedMessage(): unknown; clearQuotedMessage(): unknown; - scrollToBottom: (converstionId: string) => unknown; }; const MAX_LENGTH = 64 * 1024; @@ -97,7 +95,6 @@ const BASE_CLASS_NAME = 'module-composition-input'; export function CompositionInput(props: Props): React.ReactElement { const { i18n, - conversationId, disabled, large, inputApi, @@ -110,7 +107,6 @@ export function CompositionInput(props: Props): React.ReactElement { getPreferredBadge, getQuotedMessage, clearQuotedMessage, - scrollToBottom, sortedGroupMembers, theme, } = props; @@ -241,7 +237,6 @@ export function CompositionInput(props: Props): React.ReactElement { `CompositionInput: Submitting message ${timestamp} with ${mentions.length} mentions` ); onSubmit(text, mentions, timestamp); - scrollToBottom(conversationId); }; if (inputApi) { diff --git a/ts/components/ForwardMessageModal.stories.tsx b/ts/components/ForwardMessageModal.stories.tsx index c5a29a4cbe..beb8f3daff 100644 --- a/ts/components/ForwardMessageModal.stories.tsx +++ b/ts/components/ForwardMessageModal.stories.tsx @@ -44,7 +44,6 @@ const candidateConversations = Array.from(Array(100), () => const useProps = (overrideProps: Partial = {}): PropsType => ({ attachments: overrideProps.attachments, - conversationId: 'conversation-id', candidateConversations, doForwardMessage: action('doForwardMessage'), getPreferredBadge: () => undefined, diff --git a/ts/components/ForwardMessageModal.tsx b/ts/components/ForwardMessageModal.tsx index 65bc724275..c924da6dea 100644 --- a/ts/components/ForwardMessageModal.tsx +++ b/ts/components/ForwardMessageModal.tsx @@ -41,7 +41,6 @@ import { useAnimated } from '../hooks/useAnimated'; export type DataPropsType = { attachments?: Array; candidateConversations: ReadonlyArray; - conversationId: string; doForwardMessage: ( selectedContacts: Array, messageBody?: string, @@ -77,7 +76,6 @@ const MAX_FORWARD = 5; export const ForwardMessageModal: FunctionComponent = ({ attachments, candidateConversations, - conversationId, doForwardMessage, getPreferredBadge, i18n, @@ -188,10 +186,10 @@ export const ForwardMessageModal: FunctionComponent = ({ }, [candidateConversations]); const toggleSelectedConversation = useCallback( - (selectedConversationId: string) => { + (conversationId: string) => { let removeContact = false; const nextSelectedContacts = selectedContacts.filter(contact => { - if (contact.id === selectedConversationId) { + if (contact.id === conversationId) { removeContact = true; return false; } @@ -201,7 +199,7 @@ export const ForwardMessageModal: FunctionComponent = ({ setSelectedContacts(nextSelectedContacts); return; } - const selectedContact = contactLookup.get(selectedConversationId); + const selectedContact = contactLookup.get(conversationId); if (selectedContact) { if (selectedContact.announcementsOnly && !selectedContact.areWeAdmin) { setCannotMessage(true); @@ -337,7 +335,6 @@ export const ForwardMessageModal: FunctionComponent = ({ ) : null}
= ({ inputApi={inputApiRef} large moduleClassName="module-ForwardMessageModal__input" - scrollToBottom={noop} onEditorStateChange={( messageText, bodyRanges, @@ -403,7 +399,7 @@ export const ForwardMessageModal: FunctionComponent = ({ i18n={i18n} onClickArchiveButton={shouldNeverBeCalled} onClickContactCheckbox={( - selectedConversationId: string, + conversationId: string, disabledReason: | undefined | ContactCheckboxDisabledReason @@ -412,9 +408,7 @@ export const ForwardMessageModal: FunctionComponent = ({ disabledReason !== ContactCheckboxDisabledReason.MaximumContactsSelected ) { - toggleSelectedConversation( - selectedConversationId - ); + toggleSelectedConversation(conversationId); } }} onSelectConversation={shouldNeverBeCalled} diff --git a/ts/components/conversation/Timeline.stories.tsx b/ts/components/conversation/Timeline.stories.tsx index 969be7b505..b598e96fd3 100644 --- a/ts/components/conversation/Timeline.stories.tsx +++ b/ts/components/conversation/Timeline.stories.tsx @@ -471,10 +471,7 @@ const createProps = (overrideProps: Partial = {}): PropsType => ({ overrideProps.isLoadingMessages === false ), items: overrideProps.items || Object.keys(items), - loadCountdownStart: undefined, - messageHeightChangeIndex: undefined, resetCounter: 0, - scrollToBottomCounter: 0, scrollToIndex: overrideProps.scrollToIndex, scrollToIndexCounter: 0, totalUnread: number('totalUnread', overrideProps.totalUnread || 0), @@ -486,7 +483,6 @@ const createProps = (overrideProps: Partial = {}): PropsType => ({ warning: overrideProps.warning, id: uuid(), - isNearBottom: false, renderItem, renderLastSeenIndicator, renderHeroRow, diff --git a/ts/components/conversation/Timeline.tsx b/ts/components/conversation/Timeline.tsx index b021b0751c..8a4fb7668c 100644 --- a/ts/components/conversation/Timeline.tsx +++ b/ts/components/conversation/Timeline.tsx @@ -77,14 +77,13 @@ export type PropsDataType = { haveNewest: boolean; haveOldest: boolean; isLoadingMessages: boolean; - isNearBottom: boolean; + isNearBottom?: boolean; items: ReadonlyArray; - loadCountdownStart: number | undefined; - messageHeightChangeIndex: number | undefined; - oldestUnreadIndex: number | undefined; + loadCountdownStart?: number; + messageHeightChangeIndex?: number; + oldestUnreadIndex?: number; resetCounter: number; - scrollToBottomCounter: number; - scrollToIndex: number | undefined; + scrollToIndex?: number; scrollToIndexCounter: number; totalUnread: number; }; @@ -957,7 +956,7 @@ export class Timeline extends React.PureComponent { this.scrollDown(false); }; - public scrollDown = (setFocus?: boolean, forceScrollDown?: boolean): void => { + public scrollDown = (setFocus?: boolean): void => { const { haveNewest, id, @@ -974,7 +973,7 @@ export class Timeline extends React.PureComponent { const lastId = items[items.length - 1]; const lastSeenIndicatorRow = this.getLastSeenIndicatorRow(); - if (!this.visibleRows || forceScrollDown) { + if (!this.visibleRows) { if (haveNewest) { this.scrollToBottom(setFocus); } else if (!isLoadingMessages) { @@ -1031,7 +1030,6 @@ export class Timeline extends React.PureComponent { messageHeightChangeIndex, oldestUnreadIndex, resetCounter, - scrollToBottomCounter, scrollToIndex, typingContactId, } = this.props; @@ -1049,10 +1047,6 @@ export class Timeline extends React.PureComponent { this.resizeHeroRow(); } - if (scrollToBottomCounter !== prevProps.scrollToBottomCounter) { - this.scrollDown(false, true); - } - // There are a number of situations which can necessitate that we forget about row // heights previously calculated. We reset the minimum number of rows to minimize // unexpected changes to the scroll position. Those changes happen because diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index ec5bab4c3d..5416b0b5c0 100644 --- a/ts/models/conversations.ts +++ b/ts/models/conversations.ts @@ -101,6 +101,7 @@ import { getAvatarData } from '../util/getAvatarData'; import { createIdenticon } from '../util/createIdenticon'; import * as log from '../logging/log'; import * as Errors from '../types/errors'; +import { isMessageUnread } from '../util/isMessageUnread'; /* eslint-disable more/no-then */ window.Whisper = window.Whisper || {}; @@ -116,7 +117,13 @@ const { upgradeMessageSchema, writeNewAttachmentData, } = window.Signal.Migrations; -const { addStickerPackReference } = window.Signal.Data; +const { + addStickerPackReference, + getOlderMessagesByConversation, + getMessageMetricsForConversation, + getMessageById, + getNewerMessagesByConversation, +} = window.Signal.Data; const THREE_HOURS = durations.HOUR * 3; const FIVE_MINUTES = durations.MINUTE * 5; @@ -124,6 +131,8 @@ const FIVE_MINUTES = durations.MINUTE * 5; const JOB_REPORTING_THRESHOLD_MS = 25; const SEND_REPORTING_THRESHOLD_MS = 25; +const MESSAGE_LOAD_CHUNK_SIZE = 30; + const ATTRIBUTES_THAT_DONT_INVALIDATE_PROPS_CACHE = new Set([ 'profileLastFetchedAt', 'needsStorageServiceSync', @@ -163,7 +172,7 @@ export class ConversationModel extends window.Backbone inProgressFetch?: Promise; - incomingMessageQueue?: typeof window.PQueueType; + newMessageQueue?: typeof window.PQueueType; jobQueue?: typeof window.PQueueType; @@ -1303,34 +1312,340 @@ export class ConversationModel extends window.Backbone this.debouncedUpdateLastMessage!(); } - addSingleMessage(message: MessageModel): void { - const { messagesAdded } = window.reduxActions.conversations; - const isNewMessage = true; - messagesAdded( - this.id, - [{ ...message.attributes }], - isNewMessage, - window.isActive() - ); + addIncomingMessage(message: MessageModel): void { + this.addSingleMessage(message); } - // For incoming messages, they might arrive while we're in the middle of a bulk fetch - // from the database. We'll wait until that is done to process this newly-arrived - // message. - addIncomingMessage(message: MessageModel): void { - if (!this.incomingMessageQueue) { - this.incomingMessageQueue = new window.PQueue({ + // New messages might arrive while we're in the middle of a bulk fetch from the + // database. We'll wait until that is done before moving forward. + async addSingleMessage( + message: MessageModel, + { isJustSent }: { isJustSent: boolean } = { isJustSent: false } + ): Promise { + if (!this.newMessageQueue) { + this.newMessageQueue = new window.PQueue({ concurrency: 1, timeout: 1000 * 60 * 2, }); } // We use a queue here to ensure messages are added to the UI in the order received - this.incomingMessageQueue.add(async () => { + await this.newMessageQueue.add(async () => { await this.inProgressFetch; - - this.addSingleMessage(message); }); + + const { messagesAdded } = window.reduxActions.conversations; + const { conversations } = window.reduxStore.getState(); + const { messagesByConversation } = conversations; + + const conversationId = this.id; + const existingConversation = messagesByConversation[conversationId]; + const newestId = existingConversation?.metrics?.newest?.id; + const messageIds = existingConversation?.messageIds; + + const isLatestInMemory = + newestId && messageIds && messageIds[messageIds.length - 1] === newestId; + + if (!isJustSent || isLatestInMemory) { + messagesAdded({ + conversationId, + messages: [{ ...message.attributes }], + isActive: window.isActive(), + isJustSent, + isNewMessage: true, + }); + } + + await this.loadNewestMessages(undefined, undefined); + } + + setInProgressFetch(): () => unknown { + let resolvePromise: (value?: unknown) => void; + this.inProgressFetch = new Promise(resolve => { + resolvePromise = resolve; + }); + + const finish = () => { + resolvePromise(); + this.inProgressFetch = undefined; + }; + + return finish; + } + + async loadNewestMessages( + newestMessageId: string | undefined, + setFocus: boolean | undefined + ): Promise { + const { messagesReset, setMessagesLoading } = + window.reduxActions.conversations; + const conversationId = this.id; + + setMessagesLoading(conversationId, true); + const finish = this.setInProgressFetch(); + + try { + let scrollToLatestUnread = true; + + if (newestMessageId) { + const newestInMemoryMessage = await getMessageById(newestMessageId, { + Message: window.Whisper.Message, + }); + if (newestInMemoryMessage) { + // If newest in-memory message is unread, scrolling down would mean going to + // the very bottom, not the oldest unread. + if (isMessageUnread(newestInMemoryMessage.attributes)) { + scrollToLatestUnread = false; + } + } else { + log.warn( + `loadNewestMessages: did not find message ${newestMessageId}` + ); + } + } + + const metrics = await getMessageMetricsForConversation(conversationId); + + // If this is a message request that has not yet been accepted, we always show the + // oldest messages, to ensure that the ConversationHero is shown. We don't want to + // scroll directly to the oldest message, because that could scroll the hero off + // the screen. + if (!newestMessageId && !this.getAccepted() && metrics.oldest) { + this.loadAndScroll(metrics.oldest.id, { disableScroll: true }); + return; + } + + if (scrollToLatestUnread && metrics.oldestUnread) { + this.loadAndScroll(metrics.oldestUnread.id, { + disableScroll: !setFocus, + }); + return; + } + + const messages = await getOlderMessagesByConversation(conversationId, { + limit: MESSAGE_LOAD_CHUNK_SIZE, + MessageCollection: window.Whisper.MessageCollection, + }); + + const cleaned: Array = await this.cleanModels(messages); + const scrollToMessageId = + setFocus && metrics.newest ? metrics.newest.id : undefined; + + // Because our `getOlderMessages` fetch above didn't specify a receivedAt, we got + // the most recent N messages in the conversation. If it has a conflict with + // metrics, fetched a bit before, that's likely a race condition. So we tell our + // reducer to trust the message set we just fetched for determining if we have + // the newest message loaded. + const unboundedFetch = true; + messagesReset( + conversationId, + cleaned.map((messageModel: MessageModel) => ({ + ...messageModel.attributes, + })), + metrics, + scrollToMessageId, + unboundedFetch + ); + } catch (error) { + setMessagesLoading(conversationId, false); + throw error; + } finally { + finish(); + } + } + async loadOlderMessages(oldestMessageId: string): Promise { + const { messagesAdded, setMessagesLoading, repairOldestMessage } = + window.reduxActions.conversations; + const conversationId = this.id; + + setMessagesLoading(conversationId, true); + const finish = this.setInProgressFetch(); + + try { + const message = await getMessageById(oldestMessageId, { + Message: window.Whisper.Message, + }); + if (!message) { + throw new Error( + `loadOlderMessages: failed to load message ${oldestMessageId}` + ); + } + + const receivedAt = message.get('received_at'); + const sentAt = message.get('sent_at'); + const models = await getOlderMessagesByConversation(conversationId, { + receivedAt, + sentAt, + messageId: oldestMessageId, + limit: MESSAGE_LOAD_CHUNK_SIZE, + MessageCollection: window.Whisper.MessageCollection, + }); + + if (models.length < 1) { + log.warn('loadOlderMessages: requested, but loaded no messages'); + repairOldestMessage(conversationId); + return; + } + + const cleaned = await this.cleanModels(models); + messagesAdded({ + conversationId, + messages: cleaned.map((messageModel: MessageModel) => ({ + ...messageModel.attributes, + })), + isActive: window.isActive(), + isJustSent: false, + isNewMessage: false, + }); + } catch (error) { + setMessagesLoading(conversationId, true); + throw error; + } finally { + finish(); + } + } + + async loadNewerMessages(newestMessageId: string): Promise { + const { messagesAdded, setMessagesLoading, repairNewestMessage } = + window.reduxActions.conversations; + const conversationId = this.id; + + setMessagesLoading(conversationId, true); + const finish = this.setInProgressFetch(); + + try { + const message = await getMessageById(newestMessageId, { + Message: window.Whisper.Message, + }); + if (!message) { + throw new Error( + `loadNewerMessages: failed to load message ${newestMessageId}` + ); + } + + const receivedAt = message.get('received_at'); + const sentAt = message.get('sent_at'); + const models = await getNewerMessagesByConversation(conversationId, { + receivedAt, + sentAt, + limit: MESSAGE_LOAD_CHUNK_SIZE, + MessageCollection: window.Whisper.MessageCollection, + }); + + if (models.length < 1) { + log.warn('loadNewerMessages: requested, but loaded no messages'); + repairNewestMessage(conversationId); + return; + } + + const cleaned = await this.cleanModels(models); + messagesAdded({ + conversationId, + messages: cleaned.map((messageModel: MessageModel) => ({ + ...messageModel.attributes, + })), + isActive: window.isActive(), + isJustSent: false, + isNewMessage: false, + }); + } catch (error) { + setMessagesLoading(conversationId, false); + throw error; + } finally { + finish(); + } + } + + async loadAndScroll( + messageId: string, + options?: { disableScroll?: boolean } + ): Promise { + const { messagesReset, setMessagesLoading } = + window.reduxActions.conversations; + const conversationId = this.id; + + setMessagesLoading(conversationId, true); + const finish = this.setInProgressFetch(); + + try { + const message = await getMessageById(messageId, { + Message: window.Whisper.Message, + }); + if (!message) { + throw new Error( + `loadMoreAndScroll: failed to load message ${messageId}` + ); + } + + const receivedAt = message.get('received_at'); + const sentAt = message.get('sent_at'); + const older = await getOlderMessagesByConversation(conversationId, { + limit: MESSAGE_LOAD_CHUNK_SIZE, + receivedAt, + sentAt, + messageId, + MessageCollection: window.Whisper.MessageCollection, + }); + const newer = await getNewerMessagesByConversation(conversationId, { + limit: MESSAGE_LOAD_CHUNK_SIZE, + receivedAt, + sentAt, + MessageCollection: window.Whisper.MessageCollection, + }); + const metrics = await getMessageMetricsForConversation(conversationId); + + const all = [...older.models, message, ...newer.models]; + + const cleaned: Array = await this.cleanModels(all); + const scrollToMessageId = + options && options.disableScroll ? undefined : messageId; + + messagesReset( + conversationId, + cleaned.map((messageModel: MessageModel) => ({ + ...messageModel.attributes, + })), + metrics, + scrollToMessageId + ); + } catch (error) { + setMessagesLoading(conversationId, false); + throw error; + } finally { + finish(); + } + } + + async cleanModels( + collection: MessageModelCollectionType | Array + ): Promise> { + const result = collection + .filter((message: MessageModel) => Boolean(message.id)) + .map((message: MessageModel) => + window.MessageController.register(message.id, message) + ); + + const eliminated = collection.length - result.length; + if (eliminated > 0) { + log.warn(`cleanModels: Eliminated ${eliminated} messages without an id`); + } + + for (let max = result.length, i = 0; i < max; i += 1) { + const message = result[i]; + const { attributes } = message; + const { schemaVersion } = attributes; + + if (schemaVersion < Message.VERSION_NEEDED_FOR_DISPLAY) { + // Yep, we really do want to wait for each of these + // eslint-disable-next-line no-await-in-loop + const upgradedMessage = await upgradeMessageSchema(attributes); + message.set(upgradedMessage); + // eslint-disable-next-line no-await-in-loop + await window.Signal.Data.saveMessage(upgradedMessage); + } + } + + return result; } format(): ConversationType { @@ -3659,7 +3974,7 @@ export class ConversationModel extends window.Backbone const enableProfileSharing = Boolean( mandatoryProfileSharingEnabled && !this.get('profileSharing') ); - this.addSingleMessage(model); + this.addSingleMessage(model, { isJustSent: true }); const draftProperties = dontClearDraft ? {} diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index fad8fa11c7..d20a360340 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -248,7 +248,6 @@ export type ConversationMessageType = { messageIds: Array; metrics: MessageMetricsType; resetCounter: number; - scrollToBottomCounter: number; scrollToMessageId?: string; scrollToMessageCounter: number; }; @@ -552,9 +551,10 @@ export type MessagesAddedActionType = { type: 'MESSAGES_ADDED'; payload: { conversationId: string; - messages: Array; - isNewMessage: boolean; isActive: boolean; + isJustSent: boolean; + isNewMessage: boolean; + messages: Array; }; }; @@ -611,12 +611,6 @@ export type SetSelectedConversationPanelDepthActionType = { type: 'SET_SELECTED_CONVERSATION_PANEL_DEPTH'; payload: { panelDepth: number }; }; -export type ScrollToBpttomActionType = { - type: 'SCROLL_TO_BOTTOM'; - payload: { - conversationId: string; - }; -}; export type ScrollToMessageActionType = { type: 'SCROLL_TO_MESSAGE'; payload: { @@ -773,7 +767,6 @@ export type ConversationActionType = | ReplaceAvatarsActionType | ReviewGroupMemberNameCollisionActionType | ReviewMessageRequestNameCollisionActionType - | ScrollToBpttomActionType | ScrollToMessageActionType | SelectedConversationChangedActionType | SetComposeGroupAvatarActionType @@ -845,7 +838,6 @@ export const actions = { reviewMessageRequestNameCollision, saveAvatarToDisk, saveUsername, - scrollToBottom, scrollToMessage, selectMessage, setComposeGroupAvatar, @@ -1556,19 +1548,27 @@ function messageSizeChanged( }, }; } -function messagesAdded( - conversationId: string, - messages: Array, - isNewMessage: boolean, - isActive: boolean -): MessagesAddedActionType { +function messagesAdded({ + conversationId, + isActive, + isJustSent, + isNewMessage, + messages, +}: { + conversationId: string; + isActive: boolean; + isJustSent: boolean; + isNewMessage: boolean; + messages: Array; +}): MessagesAddedActionType { return { type: 'MESSAGES_ADDED', payload: { conversationId, - messages, - isNewMessage, isActive, + isJustSent, + isNewMessage, + messages, }, }; } @@ -1734,15 +1734,6 @@ function closeMaximumGroupSizeModal(): CloseMaximumGroupSizeModalActionType { function closeRecommendedGroupSizeModal(): CloseRecommendedGroupSizeModalActionType { return { type: 'CLOSE_RECOMMENDED_GROUP_SIZE_MODAL' }; } -function scrollToBottom(conversationId: string): ScrollToBpttomActionType { - return { - type: 'SCROLL_TO_BOTTOM', - payload: { - conversationId, - }, - }; -} - function scrollToMessage( conversationId: string, messageId: string @@ -2657,9 +2648,6 @@ export function reducer( scrollToMessageCounter: existingConversation ? existingConversation.scrollToMessageCounter + 1 : 0, - scrollToBottomCounter: existingConversation - ? existingConversation.scrollToBottomCounter - : 0, messageIds, metrics: { ...metrics, @@ -2739,28 +2727,6 @@ export function reducer( }, }; } - if (action.type === 'SCROLL_TO_BOTTOM') { - const { payload } = action; - const { conversationId } = payload; - const { messagesByConversation } = state; - const existingConversation = messagesByConversation[conversationId]; - - if (!existingConversation) { - return state; - } - - return { - ...state, - messagesByConversation: { - ...messagesByConversation, - [conversationId]: { - ...existingConversation, - scrollToBottomCounter: existingConversation.scrollToBottomCounter + 1, - }, - }, - }; - } - if (action.type === 'SCROLL_TO_MESSAGE') { const { payload } = action; const { conversationId, messageId } = payload; @@ -2947,7 +2913,8 @@ export function reducer( } if (action.type === 'MESSAGES_ADDED') { - const { conversationId, isActive, isNewMessage, messages } = action.payload; + const { conversationId, isActive, isJustSent, isNewMessage, messages } = + action.payload; const { messagesByConversation, messagesLookup } = state; const existingConversation = messagesByConversation[conversationId]; @@ -2994,6 +2961,12 @@ export function reducer( // won't add new messages to our message list. const haveLatest = newest && newest.id === lastMessageId; if (!haveLatest) { + if (isJustSent) { + log.warn( + 'reducer/MESSAGES_ADDED: isJustSent is true, but haveLatest is false' + ); + } + return state; } } @@ -3055,7 +3028,7 @@ export function reducer( isLoadingMessages: false, messageIds, heightChangeMessageIds, - scrollToMessageId: undefined, + scrollToMessageId: isJustSent ? last.id : undefined, metrics: { ...existingConversation.metrics, newest, diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index 28f54ddc0c..a0addd6abf 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -849,7 +849,6 @@ export function _conversationMessagesSelector( messageIds, metrics, resetCounter, - scrollToBottomCounter, scrollToMessageId, scrollToMessageCounter, } = conversation; @@ -888,7 +887,7 @@ export function _conversationMessagesSelector( isLoadingMessages, loadCountdownStart, items, - isNearBottom: isNearBottom || false, + isNearBottom, messageHeightChangeIndex: isNumber(messageHeightChangeIndex) && messageHeightChangeIndex >= 0 ? messageHeightChangeIndex @@ -898,7 +897,6 @@ export function _conversationMessagesSelector( ? oldestUnreadIndex : undefined, resetCounter, - scrollToBottomCounter, scrollToIndex: isNumber(scrollToIndex) && scrollToIndex >= 0 ? scrollToIndex : undefined, scrollToIndexCounter: scrollToMessageCounter, @@ -934,16 +932,10 @@ export const getConversationMessagesSelector = createSelector( haveNewest: false, haveOldest: false, isLoadingMessages: false, - isNearBottom: false, - items: [], - loadCountdownStart: undefined, - messageHeightChangeIndex: undefined, - oldestUnreadIndex: undefined, resetCounter: 0, - scrollToBottomCounter: 0, - scrollToIndex: undefined, scrollToIndexCounter: 0, totalUnread: 0, + items: [], }; } diff --git a/ts/state/smart/ForwardMessageModal.tsx b/ts/state/smart/ForwardMessageModal.tsx index 14f8da2e0f..c3525ff5b1 100644 --- a/ts/state/smart/ForwardMessageModal.tsx +++ b/ts/state/smart/ForwardMessageModal.tsx @@ -18,7 +18,6 @@ import type { AttachmentDraftType } from '../../types/Attachment'; export type SmartForwardMessageModalProps = { attachments?: Array; - conversationId: string; doForwardMessage: ( selectedContacts: Array, messageBody?: string, @@ -42,7 +41,6 @@ const mapStateToProps = ( ): DataPropsType => { const { attachments, - conversationId, doForwardMessage, isSticker, messageBody, @@ -59,7 +57,6 @@ const mapStateToProps = ( return { attachments, candidateConversations, - conversationId, doForwardMessage, getPreferredBadge: getPreferredBadgeSelector(state), i18n: getIntl(state), diff --git a/ts/test-electron/state/ducks/conversations_test.ts b/ts/test-electron/state/ducks/conversations_test.ts index bda711b419..0a65ff48b0 100644 --- a/ts/test-electron/state/ducks/conversations_test.ts +++ b/ts/test-electron/state/ducks/conversations_test.ts @@ -337,7 +337,6 @@ describe('both/state/ducks/conversations', () => { totalUnread: 0, }, resetCounter: 0, - scrollToBottomCounter: 0, scrollToMessageCounter: 0, }; } @@ -840,7 +839,6 @@ describe('both/state/ducks/conversations', () => { messageIds: [messageId], metrics: { totalUnread: 0 }, resetCounter: 0, - scrollToBottomCounter: 0, scrollToMessageCounter: 0, }, }, diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts index 9929a1a69e..7b67dc8b44 100644 --- a/ts/views/conversation_view.ts +++ b/ts/views/conversation_view.ts @@ -19,7 +19,6 @@ import { sniffImageMimeType } from '../util/sniffImageMimeType'; import type { ConversationModel } from '../models/conversations'; import type { GroupV2PendingMemberType, - MessageModelCollectionType, MessageAttributesType, ConversationModelCollectionType, QuotedMessageType, @@ -49,7 +48,6 @@ import { isOutgoing, isTapToView, } from '../state/selectors/message'; -import { isMessageUnread } from '../util/isMessageUnread'; import { getConversationSelector, getMessagesByConversation, @@ -138,13 +136,7 @@ const { upgradeMessageSchema, } = window.Signal.Migrations; -const { - getOlderMessagesByConversation, - getMessageMetricsForConversation, - getMessageById, - getMessagesBySentAt, - getNewerMessagesByConversation, -} = window.Signal.Data; +const { getMessageById, getMessagesBySentAt } = window.Signal.Data; type MessageActionsType = { deleteMessage: (messageId: string) => unknown; @@ -474,107 +466,6 @@ export class ConversationView extends window.Backbone.View { this.scrollToMessage(message.id); }; - const loadOlderMessages = async (oldestMessageId: string) => { - const { messagesAdded, setMessagesLoading, repairOldestMessage } = - window.reduxActions.conversations; - const conversationId = this.model.id; - - setMessagesLoading(conversationId, true); - const finish = this.setInProgressFetch(); - - try { - const message = await getMessageById(oldestMessageId, { - Message: Whisper.Message, - }); - if (!message) { - throw new Error( - `loadOlderMessages: failed to load message ${oldestMessageId}` - ); - } - - const receivedAt = message.get('received_at'); - const sentAt = message.get('sent_at'); - const models = await getOlderMessagesByConversation(conversationId, { - receivedAt, - sentAt, - messageId: oldestMessageId, - limit: 30, - MessageCollection: Whisper.MessageCollection, - }); - - if (models.length < 1) { - log.warn('loadOlderMessages: requested, but loaded no messages'); - repairOldestMessage(conversationId); - return; - } - - const cleaned = await this.cleanModels(models); - const isNewMessage = false; - messagesAdded( - this.model.id, - cleaned.map((messageModel: MessageModel) => ({ - ...messageModel.attributes, - })), - isNewMessage, - window.isActive() - ); - } catch (error) { - setMessagesLoading(conversationId, true); - throw error; - } finally { - finish(); - } - }; - const loadNewerMessages = async (newestMessageId: string) => { - const { messagesAdded, setMessagesLoading, repairNewestMessage } = - window.reduxActions.conversations; - const conversationId = this.model.id; - - setMessagesLoading(conversationId, true); - const finish = this.setInProgressFetch(); - - try { - const message = await getMessageById(newestMessageId, { - Message: Whisper.Message, - }); - if (!message) { - throw new Error( - `loadNewerMessages: failed to load message ${newestMessageId}` - ); - } - - const receivedAt = message.get('received_at'); - const sentAt = message.get('sent_at'); - const models = await getNewerMessagesByConversation(conversationId, { - receivedAt, - sentAt, - limit: 30, - MessageCollection: Whisper.MessageCollection, - }); - - if (models.length < 1) { - log.warn('loadNewerMessages: requested, but loaded no messages'); - repairNewestMessage(conversationId); - return; - } - - const cleaned = await this.cleanModels(models); - const isNewMessage = false; - messagesAdded( - conversationId, - cleaned.map((messageModel: MessageModel) => ({ - ...messageModel.attributes, - })), - isNewMessage, - window.isActive() - ); - } catch (error) { - setMessagesLoading(conversationId, false); - throw error; - } finally { - finish(); - } - }; const markMessageRead = async (messageId: string) => { if (!window.isActive()) { return; @@ -615,10 +506,10 @@ export class ConversationView extends window.Backbone.View { }, contactSupport, learnMoreAboutDeliveryIssue, - loadNewerMessages, - loadNewestMessages: this.loadNewestMessages.bind(this), - loadAndScroll: this.loadAndScroll.bind(this), - loadOlderMessages, + loadNewerMessages: this.model.loadNewerMessages.bind(this.model), + loadNewestMessages: this.model.loadNewestMessages.bind(this.model), + loadAndScroll: this.model.loadAndScroll.bind(this.model), + loadOlderMessages: this.model.loadOlderMessages.bind(this.model), markMessageRead, onBlock: createMessageRequestResponseHandler( 'onBlock', @@ -991,38 +882,6 @@ export class ConversationView extends window.Backbone.View { }; } - async cleanModels( - collection: MessageModelCollectionType | Array - ): Promise> { - const result = collection - .filter((message: MessageModel) => Boolean(message.id)) - .map((message: MessageModel) => - window.MessageController.register(message.id, message) - ); - - const eliminated = collection.length - result.length; - if (eliminated > 0) { - log.warn(`cleanModels: Eliminated ${eliminated} messages without an id`); - } - - for (let max = result.length, i = 0; i < max; i += 1) { - const message = result[i]; - const { attributes } = message; - const { schemaVersion } = attributes; - - if (schemaVersion < Message.VERSION_NEEDED_FOR_DISPLAY) { - // Yep, we really do want to wait for each of these - // eslint-disable-next-line no-await-in-loop - const upgradedMessage = await upgradeMessageSchema(attributes); - message.set(upgradedMessage); - // eslint-disable-next-line no-await-in-loop - await window.Signal.Data.saveMessage(upgradedMessage); - } - } - - return result; - } - async scrollToMessage(messageId: string): Promise { const message = await getMessageById(messageId, { Message: Whisper.Message, @@ -1053,162 +912,7 @@ export class ConversationView extends window.Backbone.View { return; } - this.loadAndScroll(messageId); - } - - setInProgressFetch(): () => unknown { - let resolvePromise: (value?: unknown) => void; - this.model.inProgressFetch = new Promise(resolve => { - resolvePromise = resolve; - }); - - const finish = () => { - resolvePromise(); - this.model.inProgressFetch = undefined; - }; - - return finish; - } - - async loadAndScroll( - messageId: string, - options?: { disableScroll?: boolean } - ): Promise { - const { messagesReset, setMessagesLoading } = - window.reduxActions.conversations; - const conversationId = this.model.id; - - setMessagesLoading(conversationId, true); - const finish = this.setInProgressFetch(); - - try { - const message = await getMessageById(messageId, { - Message: Whisper.Message, - }); - if (!message) { - throw new Error( - `loadMoreAndScroll: failed to load message ${messageId}` - ); - } - - const receivedAt = message.get('received_at'); - const sentAt = message.get('sent_at'); - const older = await getOlderMessagesByConversation(conversationId, { - limit: 30, - receivedAt, - sentAt, - messageId, - MessageCollection: Whisper.MessageCollection, - }); - const newer = await getNewerMessagesByConversation(conversationId, { - limit: 30, - receivedAt, - sentAt, - MessageCollection: Whisper.MessageCollection, - }); - const metrics = await getMessageMetricsForConversation(conversationId); - - const all = [...older.models, message, ...newer.models]; - - const cleaned: Array = await this.cleanModels(all); - const scrollToMessageId = - options && options.disableScroll ? undefined : messageId; - - messagesReset( - conversationId, - cleaned.map((messageModel: MessageModel) => ({ - ...messageModel.attributes, - })), - metrics, - scrollToMessageId - ); - } catch (error) { - setMessagesLoading(conversationId, false); - throw error; - } finally { - finish(); - } - } - - async loadNewestMessages( - newestMessageId: string | undefined, - setFocus: boolean | undefined - ): Promise { - const { messagesReset, setMessagesLoading } = - window.reduxActions.conversations; - const conversationId = this.model.id; - - setMessagesLoading(conversationId, true); - const finish = this.setInProgressFetch(); - - try { - let scrollToLatestUnread = true; - - if (newestMessageId) { - const newestInMemoryMessage = await getMessageById(newestMessageId, { - Message: Whisper.Message, - }); - if (newestInMemoryMessage) { - // If newest in-memory message is unread, scrolling down would mean going to - // the very bottom, not the oldest unread. - if (isMessageUnread(newestInMemoryMessage.attributes)) { - scrollToLatestUnread = false; - } - } else { - log.warn( - `loadNewestMessages: did not find message ${newestMessageId}` - ); - } - } - - const metrics = await getMessageMetricsForConversation(conversationId); - - // If this is a message request that has not yet been accepted, we always show the - // oldest messages, to ensure that the ConversationHero is shown. We don't want to - // scroll directly to the oldest message, because that could scroll the hero off - // the screen. - if (!newestMessageId && !this.model.getAccepted() && metrics.oldest) { - this.loadAndScroll(metrics.oldest.id, { disableScroll: true }); - return; - } - - if (scrollToLatestUnread && metrics.oldestUnread) { - this.loadAndScroll(metrics.oldestUnread.id, { - disableScroll: !setFocus, - }); - return; - } - - const messages = await getOlderMessagesByConversation(conversationId, { - limit: 30, - MessageCollection: Whisper.MessageCollection, - }); - - const cleaned: Array = await this.cleanModels(messages); - const scrollToMessageId = - setFocus && metrics.newest ? metrics.newest.id : undefined; - - // Because our `getOlderMessages` fetch above didn't specify a receivedAt, we got - // the most recent 30 messages in the conversation. If it has a conflict with - // metrics, fetched a bit before, that's likely a race condition. So we tell our - // reducer to trust the message set we just fetched for determining if we have - // the newest message loaded. - const unboundedFetch = true; - messagesReset( - conversationId, - cleaned.map((messageModel: MessageModel) => ({ - ...messageModel.attributes, - })), - metrics, - scrollToMessageId, - unboundedFetch - ); - } catch (error) { - setMessagesLoading(conversationId, false); - throw error; - } finally { - finish(); - } + this.model.loadAndScroll(messageId); } async startMigrationToGV2(): Promise { @@ -1502,7 +1206,7 @@ export class ConversationView extends window.Backbone.View { }); if (message) { - this.loadAndScroll(messageId); + this.model.loadAndScroll(messageId); return; } @@ -1514,7 +1218,7 @@ export class ConversationView extends window.Backbone.View { await retryPlaceholders.findByConversationAndMarkOpened(this.model.id); } - this.loadNewestMessages(undefined, undefined); + this.model.loadNewestMessages(undefined, undefined); this.model.updateLastMessage(); this.focusMessageField(); @@ -1582,7 +1286,6 @@ export class ConversationView extends window.Backbone.View { window.reduxStore, { attachments: draftAttachments, - conversationId: this.model.id, doForwardMessage: async ( conversationIds: Array, messageBody?: string,