From 4e638d162da02d2474746f03ed3a5cf70d7125a6 Mon Sep 17 00:00:00 2001 From: Audric Ackermann <audric@loki.network> Date: Thu, 27 Jan 2022 16:22:53 +1100 Subject: [PATCH] fix the scroll to bottom with button and on send message --- app/sql.js | 19 +++++++ ts/components/SessionScrollButton.tsx | 8 +-- .../conversation/SessionConversation.tsx | 35 +++++++++--- .../SessionMessagesListContainer.tsx | 17 ++---- .../composition/CompositionBox.tsx | 6 +-- .../message-content/MessageContent.tsx | 14 ++++- .../message/message-content/MessageQuote.tsx | 1 + ts/components/search/MessageSearchResults.tsx | 1 + ts/data/data.ts | 21 ++++++++ ts/receiver/queuedJob.ts | 2 - ts/state/ducks/conversations.ts | 53 ++++++++++++++----- ts/state/selectors/conversations.ts | 6 +++ 12 files changed, 137 insertions(+), 46 deletions(-) diff --git a/app/sql.js b/app/sql.js index fda0bf21e..2f96b8b12 100644 --- a/app/sql.js +++ b/app/sql.js @@ -72,6 +72,7 @@ module.exports = { getNextExpiringMessage, getMessagesByConversation, getLastMessagesByConversation, + getOldestMessageInConversation, getFirstUnreadMessageIdInConversation, hasConversationOutgoingMessage, trimMessages, @@ -2235,6 +2236,7 @@ function getUnreadCountByConversation(conversationId) { // Note: Sorting here is necessary for getting the last message (with limit 1) // be sure to update the sorting order to sort messages on redux too (sortMessages) const orderByClause = 'ORDER BY COALESCE(serverTimestamp, sent_at, received_at) DESC'; +const orderByClauseASC = 'ORDER BY COALESCE(serverTimestamp, sent_at, received_at) ASC'; function getMessagesByConversation(conversationId, { messageId = null } = {}) { const absLimit = 20; @@ -2317,6 +2319,23 @@ function getLastMessagesByConversation(conversationId, limit) { return map(rows, row => jsonToObject(row.json)); } +function getOldestMessageInConversation(conversationId) { + const rows = globalInstance + .prepare( + ` + SELECT json FROM ${MESSAGES_TABLE} WHERE + conversationId = $conversationId + ${orderByClauseASC} + LIMIT $limit; + ` + ) + .all({ + conversationId, + limit: 1, + }); + return map(rows, row => jsonToObject(row.json)); +} + function hasConversationOutgoingMessage(conversationId) { const row = globalInstance .prepare( diff --git a/ts/components/SessionScrollButton.tsx b/ts/components/SessionScrollButton.tsx index 314396f05..84fb4e177 100644 --- a/ts/components/SessionScrollButton.tsx +++ b/ts/components/SessionScrollButton.tsx @@ -5,10 +5,6 @@ import { getShowScrollButton } from '../state/selectors/conversations'; import { SessionIconButton } from './icon'; -type Props = { - onClick?: () => any; -}; - const SessionScrollButtonDiv = styled.div` position: fixed; z-index: 2; @@ -16,7 +12,7 @@ const SessionScrollButtonDiv = styled.div` animation: fadein var(--default-duration); `; -export const SessionScrollButton = (props: Props) => { +export const SessionScrollButton = (props: { onClickScrollBottom: () => void }) => { const show = useSelector(getShowScrollButton); return ( @@ -25,7 +21,7 @@ export const SessionScrollButton = (props: Props) => { iconType="chevron" iconSize={'huge'} isHidden={!show} - onClick={props.onClick} + onClick={props.onClickScrollBottom} /> </SessionScrollButtonDiv> ); diff --git a/ts/components/conversation/SessionConversation.tsx b/ts/components/conversation/SessionConversation.tsx index e2405bd50..1e348d58c 100644 --- a/ts/components/conversation/SessionConversation.tsx +++ b/ts/components/conversation/SessionConversation.tsx @@ -18,10 +18,11 @@ import autoBind from 'auto-bind'; import { InConversationCallContainer } from '../calling/InConversationCallContainer'; import { SplitViewContainer } from '../SplitViewContainer'; import { LightboxGallery, MediaItemType } from '../lightbox/LightboxGallery'; -import { getPubkeysInPublicConversation } from '../../data/data'; +import { getLastMessageInConversation, getPubkeysInPublicConversation } from '../../data/data'; import { getConversationController } from '../../session/conversations'; import { ToastUtils, UserUtils } from '../../session/utils'; import { + openConversationToSpecificMessage, quoteMessage, ReduxConversationType, resetSelectedMessageIds, @@ -168,12 +169,9 @@ export class SessionConversation extends React.Component<Props, State> { return; } - const sendAndScroll = () => { + const sendAndScroll = async () => { void conversationModel.sendMessage(msg); - if (this.messageContainerRef.current) { - (this.messageContainerRef - .current as any).scrollTop = this.messageContainerRef.current?.scrollHeight; - } + await this.scrollToNow(); }; // const recoveryPhrase = window.textsecure.storage.get('mnemonic'); @@ -245,7 +243,10 @@ export class SessionConversation extends React.Component<Props, State> { <SplitViewContainer top={<InConversationCallContainer />} bottom={ - <SessionMessagesListContainer messageContainerRef={this.messageContainerRef} /> + <SessionMessagesListContainer + messageContainerRef={this.messageContainerRef} + scrollToNow={this.scrollToNow} + /> } disableTop={!this.props.hasOngoingCallWithFocusedConvo} /> @@ -268,6 +269,26 @@ export class SessionConversation extends React.Component<Props, State> { ); } + private async scrollToNow() { + if (!this.props.selectedConversationKey) { + return; + } + const mostNowMessage = await getLastMessageInConversation(this.props.selectedConversationKey); + + if (mostNowMessage) { + await openConversationToSpecificMessage({ + conversationKey: this.props.selectedConversationKey, + messageIdToNavigateTo: mostNowMessage.id, + shouldHighlightMessage: false, + }); + const messageContainer = this.messageContainerRef.current; + if (!messageContainer) { + return; + } + messageContainer.scrollTop = messageContainer.scrollHeight - messageContainer.clientHeight; + } + } + // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // ~~~~~~~~~~~ KEYBOARD NAVIGATION ~~~~~~~~~~~~ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/ts/components/conversation/SessionMessagesListContainer.tsx b/ts/components/conversation/SessionMessagesListContainer.tsx index f61c15f36..228358664 100644 --- a/ts/components/conversation/SessionMessagesListContainer.tsx +++ b/ts/components/conversation/SessionMessagesListContainer.tsx @@ -23,7 +23,6 @@ import { getQuotedMessageToAnimate, getSelectedConversation, getSelectedConversationKey, - getShowScrollButton, getSortedMessagesOfSelectedConversation, isFirstUnreadMessageIdAbove, } from '../../state/selectors/conversations'; @@ -76,9 +75,9 @@ type Props = SessionMessageListProps & { messagesProps: Array<SortedMessageModelProps>; conversation?: ReduxConversationType; - showScrollButton: boolean; animateQuotedMessageId: string | undefined; firstUnreadOnOpen: string | undefined; + scrollToNow: () => Promise<void>; }; class SessionMessagesListContainerInner extends React.Component<Props> { @@ -162,7 +161,10 @@ class SessionMessagesListContainerInner extends React.Component<Props> { /> </ScrollToLoadedMessageContext.Provider> - <SessionScrollButton onClick={this.scrollToMostRecentMessage} key="scroll-down-button" /> + <SessionScrollButton + onClickScrollBottom={this.props.scrollToNow} + key="scroll-down-button" + /> </div> ); } @@ -249,14 +251,6 @@ class SessionMessagesListContainerInner extends React.Component<Props> { } } - private scrollToMostRecentMessage() { - const messageContainer = this.props.messageContainerRef.current; - if (!messageContainer) { - return; - } - messageContainer.scrollTop = messageContainer.scrollHeight - messageContainer.clientHeight; - } - private scrollPgUp() { const messageContainer = this.props.messageContainerRef.current; if (!messageContainer) { @@ -326,7 +320,6 @@ const mapStateToProps = (state: StateType) => { conversationKey: getSelectedConversationKey(state), conversation: getSelectedConversation(state), messagesProps: getSortedMessagesOfSelectedConversation(state), - showScrollButton: getShowScrollButton(state), animateQuotedMessageId: getQuotedMessageToAnimate(state), firstUnreadOnOpen: getFirstUnreadMessageId(state), }; diff --git a/ts/components/conversation/composition/CompositionBox.tsx b/ts/components/conversation/composition/CompositionBox.tsx index bf1e59fa7..48ac028c0 100644 --- a/ts/components/conversation/composition/CompositionBox.tsx +++ b/ts/components/conversation/composition/CompositionBox.tsx @@ -825,6 +825,7 @@ class CompositionBoxInner extends React.Component<Props, State> { : undefined; try { + // this does not call call removeAllStagedAttachmentsInConvers const { attachments, previews } = await this.getFiles(linkPreview); this.props.sendMessage({ body: messagePlaintext, @@ -898,11 +899,6 @@ class CompositionBoxInner extends React.Component<Props, State> { } } - window.inboxStore?.dispatch( - removeAllStagedAttachmentsInConversation({ - conversationKey: this.props.selectedConversationKey, - }) - ); return { attachments, previews }; } diff --git a/ts/components/conversation/message/message-content/MessageContent.tsx b/ts/components/conversation/message/message-content/MessageContent.tsx index bd8cbe715..559cb6602 100644 --- a/ts/components/conversation/message/message-content/MessageContent.tsx +++ b/ts/components/conversation/message/message-content/MessageContent.tsx @@ -9,6 +9,7 @@ import { getMessageContentSelectorProps, getMessageTextProps, getQuotedMessageToAnimate, + getShouldHighlightMessage, } from '../../../../state/selectors/conversations'; import { canDisplayImage, @@ -98,6 +99,7 @@ export const IsMessageVisibleContext = createContext(false); export const MessageContent = (props: Props) => { const [flashGreen, setFlashGreen] = useState(false); + const [didScroll, setDidScroll] = useState(false); const contentProps = useSelector(state => getMessageContentSelectorProps(state as any, props.messageId) ); @@ -123,20 +125,28 @@ export const MessageContent = (props: Props) => { }, [setImageBroken]); const quotedMessageToAnimate = useSelector(getQuotedMessageToAnimate); + const shouldHighlightMessage = useSelector(getShouldHighlightMessage); const isQuotedMessageToAnimate = quotedMessageToAnimate === props.messageId; useLayoutEffect(() => { if (isQuotedMessageToAnimate) { - if (!flashGreen) { + if (!flashGreen && !didScroll) { //scroll to me and flash me scrollToLoadedMessage(props.messageId, 'quote-or-search-result'); - setFlashGreen(true); + setDidScroll(true); + if (shouldHighlightMessage) { + setFlashGreen(true); + } } return; } if (flashGreen) { setFlashGreen(false); } + + if (didScroll) { + setDidScroll(false); + } return; }); diff --git a/ts/components/conversation/message/message-content/MessageQuote.tsx b/ts/components/conversation/message/message-content/MessageQuote.tsx index 0c683faf0..d5468c26e 100644 --- a/ts/components/conversation/message/message-content/MessageQuote.tsx +++ b/ts/components/conversation/message/message-content/MessageQuote.tsx @@ -71,6 +71,7 @@ export const MessageQuote = (props: Props) => { void openConversationToSpecificMessage({ conversationKey: foundInDb.get('conversationId'), messageIdToNavigateTo: foundInDb.get('id'), + shouldHighlightMessage: true, }); }, [quote, multiSelectMode, props.messageId] diff --git a/ts/components/search/MessageSearchResults.tsx b/ts/components/search/MessageSearchResults.tsx index 78e205fc5..9fc443e5c 100644 --- a/ts/components/search/MessageSearchResults.tsx +++ b/ts/components/search/MessageSearchResults.tsx @@ -140,6 +140,7 @@ export const MessageSearchResult = (props: MessageResultProps) => { void openConversationToSpecificMessage({ conversationKey: conversationId, messageIdToNavigateTo: id, + shouldHighlightMessage: true, }); }} className={classNames('module-message-search-result')} diff --git a/ts/data/data.ts b/ts/data/data.ts index 86c88e143..4b1cf634d 100644 --- a/ts/data/data.ts +++ b/ts/data/data.ts @@ -128,6 +128,7 @@ const channelsToMake = { getNextExpiringMessage, getMessagesByConversation, getLastMessagesByConversation, + getOldestMessageInConversation, getFirstUnreadMessageIdInConversation, hasConversationOutgoingMessage, getSeenMessagesByHashList, @@ -799,6 +800,26 @@ export async function getLastMessagesByConversation( return new MessageCollection(messages); } +export async function getLastMessageInConversation(conversationId: string) { + const messages = await channels.getLastMessagesByConversation(conversationId, 1); + for (const message of messages) { + message.skipTimerInit = true; + } + + const collection = new MessageCollection(messages); + return collection.length ? collection.models[0] : null; +} + +export async function getOldestMessageInConversation(conversationId: string) { + const messages = await channels.getOldestMessageInConversation(conversationId); + for (const message of messages) { + message.skipTimerInit = true; + } + + const collection = new MessageCollection(messages); + return collection.length ? collection.models[0] : null; +} + /** * @returns Returns count of all messages in the database */ diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index eee1fbcec..970e592a2 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -218,8 +218,6 @@ async function handleRegularMessage( const type = message.get('type'); await copyFromQuotedMessage(message, rawDataMessage.quote); - const now = Date.now(); - if (rawDataMessage.openGroupInvitation) { message.set({ groupInvitation: rawDataMessage.openGroupInvitation }); } diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index 82d862403..f310e2951 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -1,6 +1,11 @@ import { createAsyncThunk, createSlice, PayloadAction } from '@reduxjs/toolkit'; import { getConversationController } from '../../session/conversations'; -import { getFirstUnreadMessageIdInConversation, getMessagesByConversation } from '../../data/data'; +import { + getFirstUnreadMessageIdInConversation, + getLastMessageInConversation, + getMessagesByConversation, + getOldestMessageInConversation, +} from '../../data/data'; import { ConversationNotificationSettingType, ConversationTypeEnum, @@ -279,6 +284,7 @@ export type ConversationsStateType = { showScrollButton: boolean; animateQuotedMessageId?: string; + shouldHighlightMessage: boolean; nextMessageToPlayId?: string; mentionMembers: MentionsMembersType; }; @@ -321,7 +327,7 @@ type FetchedTopMessageResults = { conversationKey: string; messagesProps: Array<MessageModelPropsWithoutConvoProps>; oldTopMessageId: string | null; -}; +} | null; export const fetchTopMessagesForConversation = createAsyncThunk( 'messages/fetchTopByConversationKey', @@ -332,6 +338,13 @@ export const fetchTopMessagesForConversation = createAsyncThunk( conversationKey: string; oldTopMessageId: string | null; }): Promise<FetchedTopMessageResults> => { + // no need to load more top if we are already at the top + const oldestMessage = await getOldestMessageInConversation(conversationKey); + + if (!oldestMessage || oldestMessage.id === oldTopMessageId) { + window.log.info('fetchTopMessagesForConversation: we are already at the top'); + return null; + } const beforeTimestamp = Date.now(); const messagesProps = await getMessages({ conversationKey, @@ -352,7 +365,7 @@ type FetchedBottomMessageResults = { conversationKey: string; messagesProps: Array<MessageModelPropsWithoutConvoProps>; oldBottomMessageId: string | null; -}; +} | null; export const fetchBottomMessagesForConversation = createAsyncThunk( 'messages/fetchBottomByConversationKey', @@ -364,6 +377,13 @@ export const fetchBottomMessagesForConversation = createAsyncThunk( oldBottomMessageId: string | null; }): Promise<FetchedBottomMessageResults> => { const beforeTimestamp = Date.now(); + // no need to load more bottom if we are already at the bottom + const mostRecentMessage = await getLastMessageInConversation(conversationKey); + + if (!mostRecentMessage || mostRecentMessage.id === oldBottomMessageId) { + window.log.info('fetchBottomMessagesForConversation: we are already at the bottom'); + return null; + } const messagesProps = await getMessages({ conversationKey, messageId: oldBottomMessageId, @@ -395,6 +415,7 @@ export function getEmptyConversationState(): ConversationsStateType { firstUnreadMessageId: undefined, oldTopMessageId: null, oldBottomMessageId: null, + shouldHighlightMessage: false, }; } @@ -439,6 +460,9 @@ function handleMessageChanged( state: ConversationsStateType, changedMessage: MessageModelPropsWithoutConvoProps ) { + if (state.selectedConversation !== changedMessage.propsForMessage.convoId) { + return state; + } const messageInStoreIndex = state?.messages?.findIndex( m => m.propsForMessage.id === changedMessage.propsForMessage.id ); @@ -650,13 +674,6 @@ const conversationsSlice = createSlice({ return state; }, - - messageChanged( - state: ConversationsStateType, - action: PayloadAction<MessageModelPropsWithoutConvoProps> - ) { - return handleMessageChanged(state, action.payload); - }, messagesChanged( state: ConversationsStateType, action: PayloadAction<Array<MessageModelPropsWithoutConvoProps>> @@ -696,6 +713,7 @@ const conversationsSlice = createSlice({ // keep the unread visible just like in other apps. It will be shown until the user changes convo return { ...state, + shouldHighlightMessage: false, firstUnreadMessageId: undefined, }; }, @@ -728,6 +746,7 @@ const conversationsSlice = createSlice({ nextMessageToPlay: undefined, showScrollButton: false, animateQuotedMessageId: undefined, + shouldHighlightMessage: false, oldTopMessageId: null, oldBottomMessageId: null, mentionMembers: [], @@ -739,6 +758,7 @@ const conversationsSlice = createSlice({ action: PayloadAction<{ conversationKey: string; messageIdToNavigateTo: string; + shouldHighlightMessage: boolean; initialMessages: Array<MessageModelPropsWithoutConvoProps>; }> ) { @@ -750,6 +770,7 @@ const conversationsSlice = createSlice({ messages: action.payload.initialMessages, showScrollButton: true, animateQuotedMessageId: action.payload.messageIdToNavigateTo, + shouldHighlightMessage: action.payload.shouldHighlightMessage, oldTopMessageId: null, oldBottomMessageId: null, }; @@ -785,6 +806,7 @@ const conversationsSlice = createSlice({ action: PayloadAction<string | undefined> ) { state.animateQuotedMessageId = action.payload; + state.shouldHighlightMessage = Boolean(state.animateQuotedMessageId); return state; }, setNextMessageToPlayId( @@ -808,6 +830,9 @@ const conversationsSlice = createSlice({ builder.addCase( fetchTopMessagesForConversation.fulfilled, (state: ConversationsStateType, action: PayloadAction<FetchedTopMessageResults>) => { + if (!action.payload) { + return state; + } // this is called once the messages are loaded from the db for the currently selected conversation const { messagesProps, conversationKey, oldTopMessageId } = action.payload; // double check that this update is for the shown convo @@ -831,6 +856,9 @@ const conversationsSlice = createSlice({ builder.addCase( fetchBottomMessagesForConversation.fulfilled, (state: ConversationsStateType, action: PayloadAction<FetchedBottomMessageResults>) => { + if (!action.payload) { + return state; + } // this is called once the messages are loaded from the db for the currently selected conversation const { messagesProps, conversationKey, oldBottomMessageId } = action.payload; // double check that this update is for the shown convo @@ -893,7 +921,6 @@ export const { messagesAdded, messageDeleted, conversationReset, - messageChanged, messagesChanged, resetOldTopMessageId, resetOldBottomMessageId, @@ -938,8 +965,9 @@ export async function openConversationWithMessages(args: { export async function openConversationToSpecificMessage(args: { conversationKey: string; messageIdToNavigateTo: string; + shouldHighlightMessage: boolean; }) { - const { conversationKey, messageIdToNavigateTo } = args; + const { conversationKey, messageIdToNavigateTo, shouldHighlightMessage } = args; const messagesAroundThisMessage = await getMessages({ conversationKey, @@ -950,6 +978,7 @@ export async function openConversationToSpecificMessage(args: { actions.openConversationToSpecificMessage({ conversationKey, messageIdToNavigateTo, + shouldHighlightMessage, initialMessages: messagesAroundThisMessage, }) ); diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index 4c0104440..2ea9b0fa4 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -612,6 +612,12 @@ export const getQuotedMessageToAnimate = createSelector( (state: ConversationsStateType): string | undefined => state.animateQuotedMessageId || undefined ); +export const getShouldHighlightMessage = createSelector( + getConversations, + (state: ConversationsStateType): boolean => + Boolean(state.animateQuotedMessageId && state.shouldHighlightMessage) +); + export const getNextMessageToPlayId = createSelector( getConversations, (state: ConversationsStateType): string | undefined => state.nextMessageToPlayId || undefined