From a21751c611140f3e762a50d2bbd5eeea0a226bee Mon Sep 17 00:00:00 2001 From: audric Date: Wed, 19 Jan 2022 17:24:20 +1100 Subject: [PATCH] open convo on last unread, and can scroll up --- .prettierignore | 1 + app/sql.js | 18 +++-- stylesheets/_quote.scss | 16 +--- stylesheets/_session.scss | 3 - .../calling/DraggableCallContainer.tsx | 2 +- .../conversation/SessionMessagesList.tsx | 7 +- .../SessionMessagesListContainer.tsx | 50 ++++++++---- .../conversation/SessionRightPanel.tsx | 1 - ts/components/dialog/UserDetailsDialog.tsx | 2 +- .../ConversationListItem.tsx | 2 +- .../leftpane/overlay/OverlayMessage.tsx | 4 +- ts/data/data.ts | 7 +- ts/models/conversation.ts | 12 +-- ts/receiver/closedGroups.ts | 2 +- ts/receiver/errors.ts | 12 +-- ts/session/utils/calling/CallManager.ts | 2 +- ts/state/ducks/conversations.ts | 76 ++++++++++++++----- ts/state/ducks/search.ts | 34 --------- ts/state/selectors/conversations.ts | 4 +- ts/test/automation/open.ts | 1 - ts/window.d.ts | 2 +- 21 files changed, 132 insertions(+), 126 deletions(-) diff --git a/.prettierignore b/.prettierignore index 2ae243cc3..a57b06841 100644 --- a/.prettierignore +++ b/.prettierignore @@ -31,6 +31,7 @@ libtextsecure/libsignal-protocol.js js/util_worker.js libtextsecure/test/blanket_mocha.js mnemonic_languages/** +playwright.config.js # Managed by package manager (`yarn`/`npm`): /package.json diff --git a/app/sql.js b/app/sql.js index 556f8192e..3f40bd16f 100644 --- a/app/sql.js +++ b/app/sql.js @@ -2233,28 +2233,29 @@ function getUnreadCountByConversation(conversationId) { // be sure to update the sorting order to sort messages on redux too (sortMessages) const orderByClause = 'ORDER BY serverTimestamp DESC, serverId DESC, sent_at DESC, received_at DESC'; + function getMessagesByConversation(conversationId, { messageId = null } = {}) { - const absLimit = 30; + const absLimit = 20; // If messageId is given it means we are opening the conversation to that specific messageId, // or that we just scrolled to it by a quote click and needs to load around it. // If messageId is null, it means we are just opening the convo to the last unread message, or at the bottom const firstUnread = getFirstUnreadMessageIdInConversation(conversationId); + if (messageId || firstUnread) { const messageFound = getMessageById(messageId || firstUnread); if (messageFound && messageFound.conversationId === conversationId) { - console.warn('firstUnread', messageId, firstUnread, messageFound); - const rows = globalInstance .prepare( `WITH cte AS ( - SELECT id, json, row_number() OVER (${orderByClause}) as row_number - FROM messages + SELECT id, conversationId, json, row_number() OVER (${orderByClause}) as row_number + FROM ${MESSAGES_TABLE} WHERE conversationId = $conversationId ), current AS ( SELECT row_number FROM cte WHERE id = $messageId + ) SELECT cte.* FROM cte, current @@ -2267,7 +2268,7 @@ function getMessagesByConversation(conversationId, { messageId = null } = {}) { messageId: messageId || firstUnread, limit: absLimit, }); - console.warn('rows', rows); + return map(rows, row => jsonToObject(row.json)); } console.warn( @@ -2275,6 +2276,8 @@ function getMessagesByConversation(conversationId, { messageId = null } = {}) { ); } + const limit = 2 * absLimit; + const rows = globalInstance .prepare( ` @@ -2286,8 +2289,9 @@ function getMessagesByConversation(conversationId, { messageId = null } = {}) { ) .all({ conversationId, - limit: 2 * absLimit, + limit, }); + return map(rows, row => jsonToObject(row.json)); } diff --git a/stylesheets/_quote.scss b/stylesheets/_quote.scss index 56d5877c1..f347cf3c2 100644 --- a/stylesheets/_quote.scss +++ b/stylesheets/_quote.scss @@ -271,20 +271,6 @@ } } -// animate when user is scrolling to a quoted message -@keyframes blinker { - 0% { - border-color: rgba($session-color-green, 1); - } - 80% { - border-color: rgba($session-color-green, 1); - } - 100% { - border-color: transparent; - } -} - .flash-green-once { - border-color: $session-color-green; - animation: blinker 2s 1 normal forwards; + box-shadow: 0px 0px 6px 3px $session-color-green; } diff --git a/stylesheets/_session.scss b/stylesheets/_session.scss index ed118fa81..e6cf61bfc 100644 --- a/stylesheets/_session.scss +++ b/stylesheets/_session.scss @@ -331,9 +331,6 @@ pre { // To limit messages with things forcing them wider, like long attachment names max-width: calc(100vw - 380px - 100px); align-items: center; - border-style: solid; - border-color: transparent; - border-width: 3px; } label { user-select: none; diff --git a/ts/components/calling/DraggableCallContainer.tsx b/ts/components/calling/DraggableCallContainer.tsx index 3a0def536..1506646a1 100644 --- a/ts/components/calling/DraggableCallContainer.tsx +++ b/ts/components/calling/DraggableCallContainer.tsx @@ -100,7 +100,7 @@ export const DraggableCallContainer = () => { const openCallingConversation = () => { if (ongoingCallPubkey && ongoingCallPubkey !== selectedConversationKey) { - void openConversationWithMessages({ conversationKey: ongoingCallPubkey }); + void openConversationWithMessages({ conversationKey: ongoingCallPubkey, messageId: null }); } }; diff --git a/ts/components/conversation/SessionMessagesList.tsx b/ts/components/conversation/SessionMessagesList.tsx index cf40fc2b9..766b72415 100644 --- a/ts/components/conversation/SessionMessagesList.tsx +++ b/ts/components/conversation/SessionMessagesList.tsx @@ -45,14 +45,9 @@ export const SessionMessagesList = (props: { const newTopMessageId = messagesProps.length ? messagesProps[messagesProps.length - 1].message.props.messageId : undefined; - console.warn('useLayoutEffect ', { - oldTopMessageId, - newTopMessageId, - length: messagesProps.length, - }); if (oldTopMessageId !== newTopMessageId && oldTopMessageId && newTopMessageId) { - props.scrollAfterLoadMore(oldTopMessageId, 'center'); + props.scrollAfterLoadMore(oldTopMessageId, 'start'); } }); diff --git a/ts/components/conversation/SessionMessagesListContainer.tsx b/ts/components/conversation/SessionMessagesListContainer.tsx index ead2e6c66..7b0fd66a2 100644 --- a/ts/components/conversation/SessionMessagesListContainer.tsx +++ b/ts/components/conversation/SessionMessagesListContainer.tsx @@ -15,8 +15,10 @@ import { QuoteClickOptions } from '../../models/messageType'; import { getConversationController } from '../../session/conversations'; import { ToastUtils } from '../../session/utils'; import { + openConversationOnQuoteClick, quotedMessageToAnimate, ReduxConversationType, + resetOldTopMessageId, showScrollToBottomButton, SortedMessageModelProps, updateHaveDoneFirstScroll, @@ -122,8 +124,12 @@ class SessionMessagesListContainerInner extends React.Component { // // currentRef.scrollTop = snapShot.fakeScrollTop; // // } // } - if (!isSameConvo) { - console.info('Not same convo, resetting scrolling posiiton'); + if ( + !isSameConvo && + this.props.messagesProps.length && + this.props.messagesProps[0].propsForMessage.convoId === this.props.conversationKey + ) { + console.info('Not same convo, resetting scrolling posiiton', this.props.messagesProps.length); this.setupTimeoutResetQuotedHighlightedMessage(this.props.animateQuotedMessageId); // displayed conversation changed. We have a bit of cleaning to do here this.initialMessageLoadingPosition(); @@ -183,7 +189,9 @@ class SessionMessagesListContainerInner extends React.Component { { + this.scrollToMessage(...args, { isLoadMoreTop: true }); + }} onPageDownPressed={this.scrollPgDown} onPageUpPressed={this.scrollPgUp} onHomePressed={this.scrollTop} @@ -211,8 +219,6 @@ class SessionMessagesListContainerInner extends React.Component { return; } - console.warn('firstUnreadOnOpen', firstUnreadOnOpen); - if ( (conversation.unreadCount && conversation.unreadCount <= 0) || firstUnreadOnOpen === undefined @@ -224,7 +230,6 @@ class SessionMessagesListContainerInner extends React.Component { const firstUnreadIndex = messagesProps.findIndex( m => m.propsForMessage.id === firstUnreadOnOpen ); - console.warn('firstUnreadIndex', firstUnreadIndex); if (firstUnreadIndex === -1) { // the first unread message is not in the 30 most recent messages @@ -236,7 +241,7 @@ class SessionMessagesListContainerInner extends React.Component { const messageElementDom = document.getElementById('unread-indicator'); messageElementDom?.scrollIntoView({ behavior: 'auto', - block: 'end', + block: 'center', }); } } @@ -267,14 +272,22 @@ class SessionMessagesListContainerInner extends React.Component { } } - private scrollToMessage(messageId: string, block: ScrollLogicalPosition | undefined) { + private scrollToMessage( + messageId: string, + block: ScrollLogicalPosition | undefined, + options?: { isLoadMoreTop: boolean | undefined } + ) { const messageElementDom = document.getElementById(`msg-${messageId}`); - console.warn('scrollToMessage', messageElementDom); - debugger; + messageElementDom?.scrollIntoView({ behavior: 'auto', block, }); + + if (options?.isLoadMoreTop) { + // reset the oldTopInRedux so that a refresh/new message does not scroll us back here again + window.inboxStore?.dispatch(resetOldTopMessageId()); + } } private scrollToBottom() { @@ -328,6 +341,9 @@ class SessionMessagesListContainerInner extends React.Component { } private async scrollToQuoteMessage(options: QuoteClickOptions) { + if (!this.props.conversationKey) { + return; + } const { quoteAuthor, quoteId, referencedMessageNotFound } = options; const { messagesProps } = this.props; @@ -356,15 +372,17 @@ class SessionMessagesListContainerInner extends React.Component { // some more information then show an informative toast to the user. if (!targetMessage) { const collection = await getMessagesBySentAt(quoteId); - const found = Boolean( - collection.find((item: MessageModel) => { - const messageAuthor = item.getSource(); + const found = collection.find((item: MessageModel) => { + const messageAuthor = item.getSource(); - return Boolean(messageAuthor && quoteAuthor === messageAuthor); - }) - ); + return Boolean(messageAuthor && quoteAuthor === messageAuthor); + }); if (found) { + void openConversationOnQuoteClick({ + conversationKey: this.props.conversationKey, + messageIdToNavigateTo: found.get('id'), + }); ToastUtils.pushFoundButNotLoaded(); } else { ToastUtils.pushOriginalNoLongerAvailable(); diff --git a/ts/components/conversation/SessionRightPanel.tsx b/ts/components/conversation/SessionRightPanel.tsx index d9a116182..0393b777a 100644 --- a/ts/components/conversation/SessionRightPanel.tsx +++ b/ts/components/conversation/SessionRightPanel.tsx @@ -37,7 +37,6 @@ async function getMediaGalleryProps( documents: Array; media: Array; }> { - console.warn('getMediaGalleryProps'); // We fetch more documents than media as they don’t require to be loaded // into memory right away. Revisit this once we have infinite scrolling: const rawMedia = await getMessagesWithVisualMediaAttachments(conversationId, { diff --git a/ts/components/dialog/UserDetailsDialog.tsx b/ts/components/dialog/UserDetailsDialog.tsx index 5469daf05..c0f016b48 100644 --- a/ts/components/dialog/UserDetailsDialog.tsx +++ b/ts/components/dialog/UserDetailsDialog.tsx @@ -39,7 +39,7 @@ export const UserDetailsDialog = (props: Props) => { ConversationTypeEnum.PRIVATE ); - await openConversationWithMessages({ conversationKey: conversation.id }); + await openConversationWithMessages({ conversationKey: conversation.id, messageId: null }); closeDialog(); } diff --git a/ts/components/leftpane/conversation-list-item/ConversationListItem.tsx b/ts/components/leftpane/conversation-list-item/ConversationListItem.tsx index 879c95c40..9f592d01a 100644 --- a/ts/components/leftpane/conversation-list-item/ConversationListItem.tsx +++ b/ts/components/leftpane/conversation-list-item/ConversationListItem.tsx @@ -92,7 +92,7 @@ const ConversationListItem = (props: Props) => { async (e: React.MouseEvent) => { // mousedown is invoked sooner than onClick, but for both right and left click if (e.button === 0) { - await openConversationWithMessages({ conversationKey: conversationId }); + await openConversationWithMessages({ conversationKey: conversationId, messageId: null }); } }, [conversationId] diff --git a/ts/components/leftpane/overlay/OverlayMessage.tsx b/ts/components/leftpane/overlay/OverlayMessage.tsx index cc1846bf9..b90ff6f2d 100644 --- a/ts/components/leftpane/overlay/OverlayMessage.tsx +++ b/ts/components/leftpane/overlay/OverlayMessage.tsx @@ -47,7 +47,7 @@ export const OverlayMessage = () => { ConversationTypeEnum.PRIVATE ); - await openConversationWithMessages({ conversationKey: pubkeyorOnsTrimmed }); + await openConversationWithMessages({ conversationKey: pubkeyorOnsTrimmed, messageId: null }); closeOverlay(); } else { // this might be an ONS, validate the regex first @@ -68,7 +68,7 @@ export const OverlayMessage = () => { ConversationTypeEnum.PRIVATE ); - await openConversationWithMessages({ conversationKey: resolvedSessionID }); + await openConversationWithMessages({ conversationKey: resolvedSessionID, messageId: null }); closeOverlay(); } catch (e) { diff --git a/ts/data/data.ts b/ts/data/data.ts index 244e936e3..24ac526d1 100644 --- a/ts/data/data.ts +++ b/ts/data/data.ts @@ -754,10 +754,7 @@ export async function getUnreadCountByConversation(conversationId: string): Prom export async function getMessagesByConversation( conversationId: string, - { - skipTimerInit = false, - messageId = null, - }: { limit?: number; skipTimerInit?: false; messageId: string | null } + { skipTimerInit = false, messageId = null }: { skipTimerInit?: false; messageId: string | null } ): Promise { const messages = await channels.getMessagesByConversation(conversationId, { messageId, @@ -767,7 +764,7 @@ export async function getMessagesByConversation( message.skipTimerInit = skipTimerInit; } } - console.warn('messages length got: ', messages.length); + console.warn(`messages length got: ${messages.length} for ${conversationId}:${messageId}`); return new MessageCollection(messages); } diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index c14e5a27c..77a6d579e 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -937,12 +937,12 @@ export class ConversationModel extends Backbone.Model { if (setToExpire) { await model.setToExpire(); } - window.inboxStore?.dispatch( - conversationActions.messageAdded({ - conversationKey: this.id, - messageModelProps: model.getMessageModelProps(), - }) - ); + // window.inboxStore?.dispatch( + // conversationActions.messageAdded({ + // conversationKey: this.id, + // messageModelProps: model.getMessageModelProps(), + // }) + // ); const unreadCount = await this.getUnreadCount(); this.set({ unreadCount }); this.updateLastMessage(); diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 851cab929..e44be49e6 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -949,7 +949,7 @@ export async function createClosedGroup(groupName: string, members: Array => { - console.warn('fetchTopMessagesForConversation oldTop:', oldTopMessageId); const beforeTimestamp = Date.now(); perfStart('fetchTopMessagesForConversation'); const messagesProps = await getMessages({ @@ -683,20 +681,19 @@ const conversationsSlice = createSlice({ openConversationExternal( state: ConversationsStateType, action: PayloadAction<{ - id: string; + conversationKey: string; firstUnreadIdOnOpen: string | undefined; initialMessages: Array; - messageId?: string; }> ) { - if (state.selectedConversation === action.payload.id) { + if (state.selectedConversation === action.payload.conversationKey) { return state; } return { conversationLookup: state.conversationLookup, - selectedConversation: action.payload.id, + selectedConversation: action.payload.conversationKey, areMoreTopMessagesBeingFetched: false, messages: action.payload.initialMessages, showRightPanel: false, @@ -715,6 +712,31 @@ const conversationsSlice = createSlice({ haveDoneFirstScroll: false, }; }, + navigateInConversationToMessageId( + state: ConversationsStateType, + action: PayloadAction<{ + conversationKey: string; + messageIdToNavigateTo: string; + initialMessages: Array; + }> + ) { + if (state.selectedConversation !== action.payload.conversationKey) { + return state; + } + + return { + ...state, + areMoreTopMessagesBeingFetched: false, + messages: action.payload.initialMessages, + showScrollButton: true, + animateQuotedMessageId: action.payload.messageIdToNavigateTo, + oldTopMessageId: null, + }; + }, + resetOldTopMessageId(state: ConversationsStateType) { + state.oldTopMessageId = null; + return state; + }, updateHaveDoneFirstScroll(state: ConversationsStateType) { state.haveDoneFirstScroll = true; return state; @@ -769,7 +791,6 @@ const conversationsSlice = createSlice({ const { messagesProps, conversationKey, oldTopMessageId } = action.payload; // double check that this update is for the shown convo if (conversationKey === state.selectedConversation) { - console.warn('fullfilled', oldTopMessageId); return { ...state, oldTopMessageId, @@ -828,6 +849,7 @@ export const { conversationReset, messageChanged, messagesChanged, + resetOldTopMessageId, updateHaveDoneFirstScroll, markConversationFullyRead, // layout stuff @@ -848,28 +870,48 @@ export const { export async function openConversationWithMessages(args: { conversationKey: string; - messageId?: string; + messageId: string | null; }) { const { conversationKey, messageId } = args; - perfStart('getFirstUnreadMessageIdInConversation'); + console.warn('openConversationWithMessages', conversationKey); const firstUnreadIdOnOpen = await getFirstUnreadMessageIdInConversation(conversationKey); - perfEnd('getFirstUnreadMessageIdInConversation', 'getFirstUnreadMessageIdInConversation'); - - // preload 30 messages - perfStart('getMessages'); const initialMessages = await getMessages({ conversationKey, - messageId: null, + messageId: messageId || null, }); - perfEnd('getMessages', 'getMessages'); window.inboxStore?.dispatch( actions.openConversationExternal({ - id: conversationKey, + conversationKey, firstUnreadIdOnOpen, - messageId, initialMessages, }) ); } + +export async function openConversationOnQuoteClick(args: { + conversationKey: string; + messageIdToNavigateTo: string; +}) { + const { conversationKey, messageIdToNavigateTo } = args; + console.warn('openConversationOnQuoteClick', { conversationKey, messageIdToNavigateTo }); + + const messagesAroundThisMessage = await getMessages({ + conversationKey, + messageId: messageIdToNavigateTo, + }); + + console.warn( + 'position of navigate to message is ', + messagesAroundThisMessage.findIndex(m => m.propsForMessage.id === messageIdToNavigateTo) + ); + + window.inboxStore?.dispatch( + actions.navigateInConversationToMessageId({ + conversationKey, + messageIdToNavigateTo, + initialMessages: messagesAroundThisMessage, + }) + ); +} diff --git a/ts/state/ducks/search.ts b/ts/state/ducks/search.ts index 12b229cd8..9e93af9c0 100644 --- a/ts/state/ducks/search.ts +++ b/ts/state/ducks/search.ts @@ -298,39 +298,5 @@ export function reducer(state: SearchStateType | undefined, action: SEARCH_TYPES }; } - // if (action.type === 'CONVERSATIONS_REMOVE_ALL') { - // return getEmptyState(); - // } - - // if (action.type === openConversationExternal.name) { - // const { payload } = action; - // const { messageId } = payload; - - // if (!messageId) { - // return state; - // } - - // return { - // ...state, - // selectedMessage: messageId, - // }; - // } - - // if (action.type === 'MESSAGE_EXPIRED') { - // const { messages, messageLookup } = state; - // if (!messages.length) { - // return state; - // } - - // const { payload } = action; - // const { messageId } = payload; - - // return { - // ...state, - // messages: reject(messages, message => messageId === message.id), - // messageLookup: omit(messageLookup, ['id']), - // }; - // } - return state; } diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index beb30a19a..c9d3503a8 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -186,9 +186,11 @@ export const getSortedMessagesTypesOfSelectedConversation = createSelector( return sortedMessages.map((msg, index) => { const isFirstUnread = Boolean(firstUnreadId === msg.propsForMessage.id); const messageTimestamp = msg.propsForMessage.serverTimestamp || msg.propsForMessage.timestamp; + // do not show the date break if we are the oldest message (no previous) + // this is to smooth a bit the loading of older message (to avoid a jump once new messages are rendered) const previousMessageTimestamp = index + 1 >= sortedMessages.length - ? 0 + ? Number.MAX_SAFE_INTEGER : sortedMessages[index + 1].propsForMessage.serverTimestamp || sortedMessages[index + 1].propsForMessage.timestamp; diff --git a/ts/test/automation/open.ts b/ts/test/automation/open.ts index c50e79c2d..86eac879a 100644 --- a/ts/test/automation/open.ts +++ b/ts/test/automation/open.ts @@ -34,7 +34,6 @@ test.beforeEach(() => { throw new Error('parentFolderOfAllDataPath unset'); } const pathToRemove = path.join(parentFolderOfAllDataPath, folder); - console.warn('Removing old test data left at: ', pathToRemove); rmdirSync(pathToRemove, { recursive: true }); }); }); diff --git a/ts/window.d.ts b/ts/window.d.ts index bc3c61b99..cf2f89051 100644 --- a/ts/window.d.ts +++ b/ts/window.d.ts @@ -71,7 +71,7 @@ declare global { inboxStore?: Store; openConversationWithMessages: (args: { conversationKey: string; - messageId?: string | undefined; + messageId: string | null; }) => Promise; LokiPushNotificationServer: any; getGlobalOnlineStatus: () => boolean;