From 792c23da87e4d5bcced4bf881976693893cbd2d2 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 21 Jan 2022 14:39:24 +1100 Subject: [PATCH] cleanup scroll to unread of quote or search result on open --- stylesheets/_quote.scss | 3 + .../conversation/SessionLastSeenIndicator.tsx | 28 +++- .../conversation/SessionMessagesList.tsx | 2 +- .../SessionMessagesListContainer.tsx | 142 +++++++++--------- .../message-content/MessageContent.tsx | 4 +- .../message/message-content/MessageQuote.tsx | 7 - .../message/message-item/ReadableMessage.tsx | 34 ++++- ts/hooks/useParamSelector.ts | 2 +- ts/models/conversation.ts | 16 +- ts/receiver/errors.ts | 15 +- ts/state/ducks/conversations.ts | 52 +++---- 11 files changed, 168 insertions(+), 137 deletions(-) diff --git a/stylesheets/_quote.scss b/stylesheets/_quote.scss index 0d40a2d7e..44b947cc7 100644 --- a/stylesheets/_quote.scss +++ b/stylesheets/_quote.scss @@ -275,6 +275,9 @@ $session-highlight-message-shadow: 0px 0px 10px 1px $session-color-green; @keyframes remove-box-shadow { 0% { + box-shadow: none; + } + 10% { box-shadow: $session-highlight-message-shadow; } 75% { diff --git a/ts/components/conversation/SessionLastSeenIndicator.tsx b/ts/components/conversation/SessionLastSeenIndicator.tsx index 486cb3c02..3502f889a 100644 --- a/ts/components/conversation/SessionLastSeenIndicator.tsx +++ b/ts/components/conversation/SessionLastSeenIndicator.tsx @@ -1,5 +1,8 @@ -import React from 'react'; +import React, { useContext, useLayoutEffect, useState } from 'react'; +import { useSelector } from 'react-redux'; import styled from 'styled-components'; +import { getQuotedMessageToAnimate } from '../../state/selectors/conversations'; +import { ScrollToLoadedMessageContext } from './SessionMessagesListContainer'; const LastSeenBarContainer = styled.div` padding-bottom: 35px; @@ -26,14 +29,29 @@ const LastSeenText = styled.div` color: var(--color-last-seen-indicator-text); `; -export const SessionLastSeenIndicator = () => { - const { i18n } = window; - const text = i18n('unreadMessages'); +export const SessionLastSeenIndicator = (props: { messageId: string }) => { // if this unread-indicator is not unique it's going to cause issues + const [didScroll, setDidScroll] = useState(false); + const quotedMessageToAnimate = useSelector(getQuotedMessageToAnimate); + + const scrollToLoadedMessage = useContext(ScrollToLoadedMessageContext); + + // if this unread-indicator is rendered, + // we want to scroll here only if the conversation was not opened to a specific message + + useLayoutEffect(() => { + if (!quotedMessageToAnimate && !didScroll) { + scrollToLoadedMessage(props.messageId, 'unread-indicator'); + setDidScroll(true); + } else if (quotedMessageToAnimate) { + setDidScroll(true); + } + }); + return ( - {text} + {window.i18n('unreadMessages')} ); diff --git a/ts/components/conversation/SessionMessagesList.tsx b/ts/components/conversation/SessionMessagesList.tsx index e196e9241..5bf1944cf 100644 --- a/ts/components/conversation/SessionMessagesList.tsx +++ b/ts/components/conversation/SessionMessagesList.tsx @@ -85,7 +85,7 @@ export const SessionMessagesList = (props: { {messagesProps.map(messageProps => { const messageId = messageProps.message.props.messageId; const unreadIndicator = messageProps.showUnreadIndicator ? ( - + ) : null; const dateBreak = diff --git a/ts/components/conversation/SessionMessagesListContainer.tsx b/ts/components/conversation/SessionMessagesListContainer.tsx index ed6272ac2..f61c15f36 100644 --- a/ts/components/conversation/SessionMessagesListContainer.tsx +++ b/ts/components/conversation/SessionMessagesListContainer.tsx @@ -15,7 +15,6 @@ import { ReduxConversationType, resetOldBottomMessageId, resetOldTopMessageId, - showScrollToBottomButton, SortedMessageModelProps, } from '../../state/ducks/conversations'; import { StateType } from '../../state/reducer'; @@ -34,9 +33,18 @@ export type SessionMessageListProps = { messageContainerRef: React.RefObject; }; +export const messageContainerDomID = 'messages-container'; + +export type ScrollToLoadedReasons = + | 'quote-or-search-result' + | 'go-to-bottom' + | 'unread-indicator' + | 'load-more-top' + | 'load-more-bottom'; + export const ScrollToLoadedMessageContext = React.createContext( // tslint:disable-next-line: no-empty - (_loadedMessageIdToScrollTo: string) => {} + (_loadedMessageIdToScrollTo: string, _reason: ScrollToLoadedReasons) => {} ); const SessionUnreadAboveIndicator = styled.div` @@ -85,10 +93,6 @@ class SessionMessagesListContainerInner extends React.Component { // ~~~~~~~~~~~~~~~~ LIFECYCLES ~~~~~~~~~~~~~~~~ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - public componentDidMount() { - this.initialMessageLoadingPosition(); - } - public componentWillUnmount() { if (this.timeoutResetQuotedScroll) { global.clearTimeout(this.timeoutResetQuotedScroll); @@ -108,7 +112,6 @@ class SessionMessagesListContainerInner extends React.Component { ) { this.setupTimeoutResetQuotedHighlightedMessage(this.props.animateQuotedMessageId); // displayed conversation changed. We have a bit of cleaning to do here - this.initialMessageLoadingPosition(); } } @@ -129,6 +132,7 @@ class SessionMessagesListContainerInner extends React.Component { return (
{ key="typing-bubble" /> - + { - const isLoadMoreTop = type === 'load-more-top'; - const isLoadMoreBottom = type === 'load-more-bottom'; - this.scrollToMessage(messageIdToScrollTo, isLoadMoreTop ? 'start' : 'end', { - isLoadMoreTop, - isLoadMoreBottom, - }); + this.scrollToMessage(messageIdToScrollTo, type); }} onPageDownPressed={this.scrollPgDown} onPageUpPressed={this.scrollPgUp} @@ -175,43 +174,6 @@ class SessionMessagesListContainerInner extends React.Component { contextMenu.hideAll(); } - /** - * Position the list to the middle of the loaded list if the conversation has unread messages and we have some messages loaded - */ - private initialMessageLoadingPosition() { - const { messagesProps, conversation, firstUnreadOnOpen } = this.props; - if (!conversation || !messagesProps.length) { - return; - } - - if ( - (conversation.unreadCount && conversation.unreadCount <= 0) || - firstUnreadOnOpen === undefined - ) { - this.scrollToMostRecentMessage(); - } else { - // just assume that this need to be shown by default - window.inboxStore?.dispatch(showScrollToBottomButton(true)); - const firstUnreadIndex = messagesProps.findIndex( - m => m.propsForMessage.id === firstUnreadOnOpen - ); - - if (firstUnreadIndex === -1) { - // the first unread message is not in the 30 most recent messages - // just scroll to the middle as we don't have enough loaded message nevertheless - const middle = Math.floor(messagesProps.length / 2); - const idToStringTo = messagesProps[middle].propsForMessage.id; - this.scrollToMessage(idToStringTo, 'center'); - } else { - const messageElementDom = document.getElementById('unread-indicator'); - messageElementDom?.scrollIntoView({ - behavior: 'auto', - block: 'center', - }); - } - } - } - /** * Could not find a better name, but when we click on a quoted message, * the UI takes us there and highlights it. @@ -233,27 +195,57 @@ class SessionMessagesListContainerInner extends React.Component { } } - private scrollToMessage( - messageId: string, - block: ScrollLogicalPosition | undefined, - options?: { isLoadMoreTop: boolean | undefined; isLoadMoreBottom: boolean | undefined } - ) { + private scrollToMessage(messageId: string, reason: ScrollToLoadedReasons) { const messageElementDom = document.getElementById(`msg-${messageId}`); + // annoyingly, useLayoutEffect, which is calling this function, is run before ref are set on a react component. + // so the only way to scroll in the container at this time, is with the DOM itself + const messageContainerDom = document.getElementById(messageContainerDomID); - messageElementDom?.scrollIntoView({ - behavior: 'auto', - block, - }); + // * if quote or search result we want to scroll to start AND do a -50px + // * if scroll-to-unread we want to scroll end AND do a +200px to be really at the end + // * if load-more-top or bottom we want to center + + switch (reason) { + case 'load-more-bottom': + messageElementDom?.scrollIntoView({ + behavior: 'auto', + block: 'end', + }); + // reset the oldBottomInRedux so that a refresh/new message does not scroll us back here again + window.inboxStore?.dispatch(resetOldBottomMessageId()); + break; + case 'load-more-top': + messageElementDom?.scrollIntoView({ + behavior: 'auto', + block: 'start', + }); + // reset the oldTopInRedux so that a refresh/new message does not scroll us back here again + window.inboxStore?.dispatch(resetOldTopMessageId()); + break; + case 'quote-or-search-result': + messageElementDom?.scrollIntoView({ + behavior: 'auto', + block: 'start', + }); + messageContainerDom?.scrollBy({ top: -50 }); - this.props.messageContainerRef.current?.scrollBy({ top: -50 }); + break; + case 'go-to-bottom': + messageElementDom?.scrollIntoView({ + behavior: 'auto', + block: 'end', + }); + messageContainerDom?.scrollBy({ top: 200 }); - if (options?.isLoadMoreTop) { - // reset the oldTopInRedux so that a refresh/new message does not scroll us back here again - window.inboxStore?.dispatch(resetOldTopMessageId()); - } - if (options?.isLoadMoreBottom) { - // reset the oldBottomInRedux so that a refresh/new message does not scroll us back here again - window.inboxStore?.dispatch(resetOldBottomMessageId()); + break; + case 'unread-indicator': + messageElementDom?.scrollIntoView({ + behavior: 'auto', + block: 'center', + }); + messageContainerDom?.scrollBy({ top: -50 }); + break; + default: } } @@ -307,8 +299,8 @@ class SessionMessagesListContainerInner extends React.Component { messageContainer.scrollTo(0, 0); } - private scrollToQuoteMessage(loadedQuoteMessageToScrollTo: string) { - if (!this.props.conversationKey || !loadedQuoteMessageToScrollTo) { + private scrollToLoadedMessage(loadedMessageToScrollTo: string, reason: ScrollToLoadedReasons) { + if (!this.props.conversationKey || !loadedMessageToScrollTo) { return; } @@ -316,14 +308,16 @@ class SessionMessagesListContainerInner extends React.Component { // If there's no message already in memory, we won't be scrolling. So we'll gather // some more information then show an informative toast to the user. - if (!messagesProps.find(m => m.propsForMessage.id === loadedQuoteMessageToScrollTo)) { + if (!messagesProps.find(m => m.propsForMessage.id === loadedMessageToScrollTo)) { throw new Error('this message is not loaded'); } - this.scrollToMessage(loadedQuoteMessageToScrollTo, 'start'); + this.scrollToMessage(loadedMessageToScrollTo, reason); // Highlight this message on the UI - window.inboxStore?.dispatch(quotedMessageToAnimate(loadedQuoteMessageToScrollTo)); - this.setupTimeoutResetQuotedHighlightedMessage(loadedQuoteMessageToScrollTo); + if (reason === 'quote-or-search-result') { + window.inboxStore?.dispatch(quotedMessageToAnimate(loadedMessageToScrollTo)); + this.setupTimeoutResetQuotedHighlightedMessage(loadedMessageToScrollTo); + } } } diff --git a/ts/components/conversation/message/message-content/MessageContent.tsx b/ts/components/conversation/message/message-content/MessageContent.tsx index fb849759e..bd8cbe715 100644 --- a/ts/components/conversation/message/message-content/MessageContent.tsx +++ b/ts/components/conversation/message/message-content/MessageContent.tsx @@ -103,7 +103,7 @@ export const MessageContent = (props: Props) => { ); const [isMessageVisible, setMessageIsVisible] = useState(false); - const scrollToMessage = useContext(ScrollToLoadedMessageContext); + const scrollToLoadedMessage = useContext(ScrollToLoadedMessageContext); const [imageBroken, setImageBroken] = useState(false); @@ -129,7 +129,7 @@ export const MessageContent = (props: Props) => { if (isQuotedMessageToAnimate) { if (!flashGreen) { //scroll to me and flash me - scrollToMessage(props.messageId); + scrollToLoadedMessage(props.messageId, 'quote-or-search-result'); setFlashGreen(true); } return; diff --git a/ts/components/conversation/message/message-content/MessageQuote.tsx b/ts/components/conversation/message/message-content/MessageQuote.tsx index 1c8b115db..0c683faf0 100644 --- a/ts/components/conversation/message/message-content/MessageQuote.tsx +++ b/ts/components/conversation/message/message-content/MessageQuote.tsx @@ -27,7 +27,6 @@ export const MessageQuote = (props: Props) => { const multiSelectMode = useSelector(isMessageSelectionMode); const isMessageDetailViewMode = useSelector(isMessageDetailView); - // const scrollToLoadedMessage = useContext(ScrollToLoadedMessageContext); const quote = selected ? selected.quote : undefined; const direction = selected ? selected.direction : undefined; @@ -73,12 +72,6 @@ export const MessageQuote = (props: Props) => { conversationKey: foundInDb.get('conversationId'), messageIdToNavigateTo: foundInDb.get('id'), }); - - // scrollToLoadedMessage?.({ - // quoteAuthor: sender, - // quoteId, - // referencedMessageNotFound: referencedMessageNotFound || false, - // }); }, [quote, multiSelectMode, props.messageId] ); diff --git a/ts/components/conversation/message/message-item/ReadableMessage.tsx b/ts/components/conversation/message/message-item/ReadableMessage.tsx index 1be9b4869..a47aa12bb 100644 --- a/ts/components/conversation/message/message-item/ReadableMessage.tsx +++ b/ts/components/conversation/message/message-item/ReadableMessage.tsx @@ -1,5 +1,5 @@ import _, { noop } from 'lodash'; -import React, { useCallback } from 'react'; +import React, { useCallback, useContext, useLayoutEffect, useState } from 'react'; import { InView } from 'react-intersection-observer'; import { useDispatch, useSelector } from 'react-redux'; import { getMessageById } from '../../../../data/data'; @@ -13,13 +13,16 @@ import { import { areMoreBottomMessagesBeingFetched, areMoreTopMessagesBeingFetched, + getFirstUnreadMessageId, getLoadedMessagesLength, getMostRecentMessageId, getOldestMessageId, + getQuotedMessageToAnimate, getSelectedConversationKey, getYoungestMessageId, } from '../../../../state/selectors/conversations'; import { getIsAppFocused } from '../../../../state/selectors/section'; +import { ScrollToLoadedMessageContext } from '../../SessionMessagesListContainer'; type ReadableMessageProps = { children: React.ReactNode; @@ -67,21 +70,44 @@ export const ReadableMessage = (props: ReadableMessageProps) => { const youngestMessageId = useSelector(getYoungestMessageId); const fetchingTopMore = useSelector(areMoreTopMessagesBeingFetched); const fetchingBottomMore = useSelector(areMoreBottomMessagesBeingFetched); + const conversationHasUnread = Boolean(useSelector(getFirstUnreadMessageId)); const shouldMarkReadWhenVisible = isUnread; + const [didScroll, setDidScroll] = useState(false); + const quotedMessageToAnimate = useSelector(getQuotedMessageToAnimate); + + const scrollToLoadedMessage = useContext(ScrollToLoadedMessageContext); + + // if this unread-indicator is rendered, + // we want to scroll here only if the conversation was not opened to a specific message + + useLayoutEffect(() => { + if ( + props.messageId === youngestMessageId && + !quotedMessageToAnimate && + !didScroll && + !conversationHasUnread + ) { + scrollToLoadedMessage(props.messageId, 'go-to-bottom'); + setDidScroll(true); + } else if (quotedMessageToAnimate) { + setDidScroll(true); + } + }); + const onVisible = useCallback( // tslint:disable-next-line: cyclomatic-complexity async (inView: boolean | Object) => { // we are the most recent message - if (mostRecentMessageId === messageId) { + if (mostRecentMessageId === messageId && selectedConversationKey) { // make sure the app is focused, because we mark message as read here if (inView === true && isAppFocused) { dispatch(showScrollToBottomButton(false)); void getConversationController() - .get(selectedConversationKey as string) + .get(selectedConversationKey) ?.markRead(receivedAt || 0) .then(() => { - dispatch(markConversationFullyRead(selectedConversationKey as string)); + dispatch(markConversationFullyRead(selectedConversationKey)); }); } else if (inView === false) { dispatch(showScrollToBottomButton(true)); diff --git a/ts/hooks/useParamSelector.ts b/ts/hooks/useParamSelector.ts index 0ea86226d..0d550fa7a 100644 --- a/ts/hooks/useParamSelector.ts +++ b/ts/hooks/useParamSelector.ts @@ -101,7 +101,7 @@ export function useWeAreAdmin(convoId?: string) { export function useExpireTimer(convoId?: string) { const convoProps = useConversationPropsById(convoId); - return Boolean(convoProps && convoProps.expireTimer); + return convoProps && convoProps.expireTimer; } export function useIsPinned(convoId?: string) { diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index 77a6d579e..45681bc45 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -930,19 +930,21 @@ export class ConversationModel extends Backbone.Model { await this.setIsApproved(true); } - // no need to trigger a UI update now, we trigger a messageAdded just below + // no need to trigger a UI update now, we trigger a messagesAdded just below const messageId = await model.commit(false); model.set({ id: messageId }); if (setToExpire) { await model.setToExpire(); } - // window.inboxStore?.dispatch( - // conversationActions.messageAdded({ - // conversationKey: this.id, - // messageModelProps: model.getMessageModelProps(), - // }) - // ); + window.inboxStore?.dispatch( + conversationActions.messagesAdded([ + { + conversationKey: this.id, + messageModelProps: model.getMessageModelProps(), + }, + ]) + ); const unreadCount = await this.getUnreadCount(); this.set({ unreadCount }); this.updateLastMessage(); diff --git a/ts/receiver/errors.ts b/ts/receiver/errors.ts index a7c113dc4..7ac5941ad 100644 --- a/ts/receiver/errors.ts +++ b/ts/receiver/errors.ts @@ -3,6 +3,7 @@ import { toNumber } from 'lodash'; import { getConversationController } from '../session/conversations'; import { ConversationTypeEnum } from '../models/conversation'; import { toLogFormat } from '../types/attachments/Errors'; +import { messagesAdded } from '../state/ducks/conversations'; export async function onError(ev: any) { const { error } = ev; @@ -33,12 +34,14 @@ export async function onError(ev: any) { conversation.updateLastMessage(); await conversation.notify(message); - // window.inboxStore?.dispatch( - // conversationActions.messageAdded({ - // conversationKey: conversation.id, - // messageModelProps: message.getMessageModelProps(), - // }) - // ); + window.inboxStore?.dispatch( + messagesAdded([ + { + conversationKey: conversation.id, + messageModelProps: message.getMessageModelProps(), + }, + ]) + ); if (ev.confirm) { ev.confirm(); diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index f129b9bd2..82d862403 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -406,31 +406,33 @@ function handleMessageAdded( } ) { const { messages } = state; + const { conversationKey, messageModelProps: addedMessageProps } = payload; - if (conversationKey === state.selectedConversation) { - 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), - ]; + if (conversationKey !== state.selectedConversation) { + return state; + } - return { - ...state, - messages: editedMessages, - }; - } + 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: [...messages, addedMessageProps], // sorting happens in the selector + messages: editedMessages, }; } - return state; + + return { + ...state, + messages: [...messages, addedMessageProps], // sorting happens in the selector + }; } function handleMessageChanged( @@ -630,15 +632,6 @@ const conversationsSlice = createSlice({ return getEmptyConversationState(); }, - messageAdded( - state: ConversationsStateType, - action: PayloadAction<{ - conversationKey: string; - messageModelProps: MessageModelPropsWithoutConvoProps; - }> - ) { - return handleMessageAdded(state, action.payload); - }, messagesAdded( state: ConversationsStateType, action: PayloadAction< @@ -741,7 +734,7 @@ const conversationsSlice = createSlice({ firstUnreadMessageId: action.payload.firstUnreadIdOnOpen, }; }, - navigateInConversationToMessageId( + openConversationToSpecificMessage( state: ConversationsStateType, action: PayloadAction<{ conversationKey: string; @@ -897,7 +890,6 @@ export const { conversationRemoved, removeAllConversations, messageExpired, - messageAdded, messagesAdded, messageDeleted, conversationReset, @@ -955,7 +947,7 @@ export async function openConversationToSpecificMessage(args: { }); window.inboxStore?.dispatch( - actions.navigateInConversationToMessageId({ + actions.openConversationToSpecificMessage({ conversationKey, messageIdToNavigateTo, initialMessages: messagesAroundThisMessage,