From c8f0150aaf46f45a1182ff64d5bd6b5e99895ade Mon Sep 17 00:00:00 2001 From: audric Date: Thu, 22 Jul 2021 12:30:01 +1000 Subject: [PATCH] fix unread banner position when first unread is visible --- js/background.js | 8 ++- ts/components/ConversationListItem.tsx | 12 ++-- ts/components/MessageSearchResult.tsx | 8 ++- ts/components/UserDetailsDialog.tsx | 6 +- ts/components/conversation/Message.tsx | 22 +++---- .../conversation/ReadableMessage.tsx | 2 +- .../session/LeftPaneMessageSection.tsx | 14 ++++- .../conversation/SessionMessagesList.tsx | 61 ++++++------------- ts/models/message.ts | 11 +++- ts/models/messageType.ts | 1 - ts/receiver/closedGroups.ts | 9 ++- ts/state/ducks/conversations.ts | 11 ++-- ts/state/ducks/search.ts | 6 +- ts/state/selectors/conversations.ts | 9 --- 14 files changed, 88 insertions(+), 92 deletions(-) diff --git a/js/background.js b/js/background.js index fada3d7e8..33add739a 100644 --- a/js/background.js +++ b/js/background.js @@ -341,11 +341,15 @@ window.setMediaPermissions(!value); }; - Whisper.Notifications.on('click', (id, messageId) => { + Whisper.Notifications.on('click', async (id, messageId) => { window.showWindow(); if (id) { + const firstUnreadIdOnOpen = await window.Signal.Data.getFirstUnreadMessageIdInConversation( + id + ); + window.inboxStore.dispatch( - window.actionsCreators.openConversationExternal({ id, messageId }) + window.actionsCreators.openConversationExternal({ id, messageId, firstUnreadIdOnOpen }) ); } else { appView.openInbox({ diff --git a/ts/components/ConversationListItem.tsx b/ts/components/ConversationListItem.tsx index 98be42401..9c8eff3cb 100644 --- a/ts/components/ConversationListItem.tsx +++ b/ts/components/ConversationListItem.tsx @@ -1,4 +1,4 @@ -import React from 'react'; +import React, { useCallback } from 'react'; import classNames from 'classnames'; import { isEmpty } from 'lodash'; import { contextMenu } from 'react-contexify'; @@ -26,6 +26,7 @@ import { SessionIcon, SessionIconSize, SessionIconType } from './session/icon'; import { useDispatch, useSelector } from 'react-redux'; import { SectionType } from '../state/ducks/section'; import { getFocusedSection } from '../state/selectors/section'; +import { getFirstUnreadMessageIdInConversation } from '../data/data'; // tslint:disable-next-line: no-empty-interface export interface ConversationListItemProps extends ReduxConversationType {} @@ -240,13 +241,16 @@ const ConversationListItem = (props: Props) => { const dispatch = useDispatch(); + const openConvo = useCallback(async () => { + const firstUnreadIdOnOpen = await getFirstUnreadMessageIdInConversation(conversationId); + dispatch(openConversationExternal({ id: conversationId, firstUnreadIdOnOpen })); + }, [conversationId]); + return (
{ - dispatch(openConversationExternal({ id: conversationId })); - }} + onClick={openConvo} onContextMenu={(e: any) => { contextMenu.show({ id: triggerId, diff --git a/ts/components/MessageSearchResult.tsx b/ts/components/MessageSearchResult.tsx index 3faf761bc..1fb86349a 100644 --- a/ts/components/MessageSearchResult.tsx +++ b/ts/components/MessageSearchResult.tsx @@ -94,7 +94,13 @@ export const MessageSearchResult = (props: Props) => {
{ - dispatch(openConversationExternal({ id: conversationId, messageId })); + dispatch( + openConversationExternal({ + id: conversationId, + messageId, + firstUnreadIdOnOpen: undefined, + }) + ); }} className={classNames( 'module-message-search-result', diff --git a/ts/components/UserDetailsDialog.tsx b/ts/components/UserDetailsDialog.tsx index 51081632c..f2896f334 100644 --- a/ts/components/UserDetailsDialog.tsx +++ b/ts/components/UserDetailsDialog.tsx @@ -11,6 +11,7 @@ import { updateUserDetailsModal } from '../state/ducks/modalDialog'; import { openConversationExternal } from '../state/ducks/conversations'; // tslint:disable-next-line: no-submodule-imports import useKey from 'react-use/lib/useKey'; +import { getFirstUnreadMessageIdInConversation } from '../data/data'; type Props = { conversationId: string; authorAvatarPath?: string; @@ -33,8 +34,11 @@ export const UserDetailsDialog = (props: Props) => { convo.id, ConversationTypeEnum.PRIVATE ); + const firstUnreadIdOnOpen = await getFirstUnreadMessageIdInConversation(conversation.id); - window.inboxStore?.dispatch(openConversationExternal({ id: conversation.id })); + window.inboxStore?.dispatch( + openConversationExternal({ id: conversation.id, firstUnreadIdOnOpen }) + ); closeDialog(); } diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx index 900c5c0ca..7c3252cac 100644 --- a/ts/components/conversation/Message.tsx +++ b/ts/components/conversation/Message.tsx @@ -39,7 +39,10 @@ import { ClickToTrustSender } from './message/ClickToTrustSender'; import { getMessageById } from '../../data/data'; import { connect } from 'react-redux'; import { StateType } from '../../state/reducer'; -import { getSelectedMessageIds } from '../../state/selectors/conversations'; +import { + getQuotedMessageToAnimate, + getSelectedMessageIds, +} from '../../state/selectors/conversations'; import { messageExpired, showLightBox, @@ -64,7 +67,10 @@ interface State { const EXPIRATION_CHECK_MINIMUM = 2000; const EXPIRED_DELAY = 600; -type Props = MessageRegularProps & { selectedMessages: Array }; +type Props = MessageRegularProps & { + selectedMessages: Array; + quotedMessageToAnimate: string | undefined; +}; const onClickAttachment = async (onClickProps: { attachment: AttachmentTypeWithPath; @@ -570,14 +576,7 @@ class MessageInner extends React.PureComponent { // tslint:disable-next-line: cyclomatic-complexity public render() { - const { - direction, - id, - multiSelectMode, - conversationType, - isUnread, - selectedMessages, - } = this.props; + const { direction, id, conversationType, isUnread, selectedMessages } = this.props; const { expired, expiring } = this.state; if (expired) { @@ -601,7 +600,7 @@ class MessageInner extends React.PureComponent { divClasses.push('public-chat-message-wrapper'); } - if (this.props.isQuotedMessageToAnimate) { + if (this.props.quotedMessageToAnimate === this.props.id) { divClasses.push('flash-green-once'); } @@ -851,6 +850,7 @@ class MessageInner extends React.PureComponent { const mapStateToProps = (state: StateType) => { return { selectedMessages: getSelectedMessageIds(state), + quotedMessageToAnimate: getQuotedMessageToAnimate(state), }; }; diff --git a/ts/components/conversation/ReadableMessage.tsx b/ts/components/conversation/ReadableMessage.tsx index d56fa2c4e..11e17c457 100644 --- a/ts/components/conversation/ReadableMessage.tsx +++ b/ts/components/conversation/ReadableMessage.tsx @@ -15,7 +15,7 @@ export const ReadableMessage = (props: ReadableMessageProps) => { useFocus(onChange); return ( - + {props.children} ); diff --git a/ts/components/session/LeftPaneMessageSection.tsx b/ts/components/session/LeftPaneMessageSection.tsx index f2ea3701c..c260c704e 100644 --- a/ts/components/session/LeftPaneMessageSection.tsx +++ b/ts/components/session/LeftPaneMessageSection.tsx @@ -26,6 +26,7 @@ import autoBind from 'auto-bind'; import { onsNameRegex } from '../../session/snode_api/SNodeAPI'; import { SNodeAPI } from '../../session/snode_api'; import { clearSearch, search, updateSearchTerm } from '../../state/ducks/search'; +import { getFirstUnreadMessageIdInConversation } from '../../data/data'; export interface Props { searchTerm: string; @@ -319,7 +320,11 @@ export class LeftPaneMessageSection extends React.Component { pubkeyorOns, ConversationTypeEnum.PRIVATE ); - window.inboxStore?.dispatch(openConversationExternal({ id: pubkeyorOns })); + const firstUnreadIdOnOpen = await getFirstUnreadMessageIdInConversation(pubkeyorOns); + + window.inboxStore?.dispatch( + openConversationExternal({ id: pubkeyorOns, firstUnreadIdOnOpen }) + ); this.handleToggleOverlay(undefined); } else { // this might be an ONS, validate the regex first @@ -339,7 +344,12 @@ export class LeftPaneMessageSection extends React.Component { resolvedSessionID, ConversationTypeEnum.PRIVATE ); - window.inboxStore?.dispatch(openConversationExternal({ id: resolvedSessionID })); + + const firstUnreadIdOnOpen = await getFirstUnreadMessageIdInConversation(resolvedSessionID); + + window.inboxStore?.dispatch( + openConversationExternal({ id: resolvedSessionID, firstUnreadIdOnOpen }) + ); this.handleToggleOverlay(undefined); } catch (e) { window?.log?.warn('failed to resolve ons name', pubkeyorOns, e); diff --git a/ts/components/session/conversation/SessionMessagesList.tsx b/ts/components/session/conversation/SessionMessagesList.tsx index 7107cc54b..ebfbc24c5 100644 --- a/ts/components/session/conversation/SessionMessagesList.tsx +++ b/ts/components/session/conversation/SessionMessagesList.tsx @@ -30,7 +30,7 @@ import { PropsForDataExtractionNotification, QuoteClickOptions, } from '../../../models/messageType'; -import { getFirstUnreadMessageIdInConversation, getMessagesBySentAt } from '../../../data/data'; +import { getMessagesBySentAt } from '../../../data/data'; import autoBind from 'auto-bind'; import { ConversationTypeEnum } from '../../../models/conversation'; import { DataExtractionNotification } from '../../conversation/DataExtractionNotification'; @@ -44,9 +44,9 @@ import { getSelectedConversationKey, getShowScrollButton, isMessageSelectionMode, - getFirstUnreadMessageIndex, areMoreMessagesBeingFetched, isFirstUnreadMessageIdAbove, + getFirstUnreadMessageId, } from '../../../state/selectors/conversations'; import { isElectronWindowFocused } from '../../../session/utils/WindowUtils'; import useInterval from 'react-use/lib/useInterval'; @@ -65,8 +65,9 @@ type Props = SessionMessageListProps & { areMoreMessagesBeingFetched: boolean; }; -const UnreadIndicator = (props: { messageId: string; show: boolean }) => { - if (!props.show) { +const UnreadIndicator = (props: { messageId: string }) => { + const isFirstUnreadOnOpen = useSelector(getFirstUnreadMessageId); + if (!isFirstUnreadOnOpen || isFirstUnreadOnOpen !== props.messageId) { return null; } return ; @@ -75,12 +76,11 @@ const UnreadIndicator = (props: { messageId: string; show: boolean }) => { const GroupUpdateItem = (props: { messageId: string; groupNotificationProps: PropsForGroupUpdate; - showUnreadIndicator: boolean; }) => { return ( - + ); }; @@ -88,13 +88,12 @@ const GroupUpdateItem = (props: { const GroupInvitationItem = (props: { messageId: string; propsForGroupInvitation: PropsForGroupInvitation; - showUnreadIndicator: boolean; }) => { return ( - + ); }; @@ -102,7 +101,6 @@ const GroupInvitationItem = (props: { const DataExtractionNotificationItem = (props: { messageId: string; propsForDataExtractionNotification: PropsForDataExtractionNotification; - showUnreadIndicator: boolean; }) => { return ( @@ -111,7 +109,7 @@ const DataExtractionNotificationItem = (props: { {...props.propsForDataExtractionNotification} /> - + ); }; @@ -119,13 +117,12 @@ const DataExtractionNotificationItem = (props: { const TimerNotificationItem = (props: { messageId: string; timerProps: PropsForExpirationTimer; - showUnreadIndicator: boolean; }) => { return ( - + ); }; @@ -134,12 +131,10 @@ const GenericMessageItem = (props: { messageId: string; messageProps: SortedMessageModelProps; playableMessageIndex?: number; - showUnreadIndicator: boolean; scrollToQuoteMessage: (options: QuoteClickOptions) => Promise; playNextMessage?: (value: number) => void; }) => { const multiSelectMode = useSelector(isMessageSelectionMode); - const quotedMessageToAnimate = useSelector(getQuotedMessageToAnimate); const nextMessageToPlay = useSelector(getNextMessageToPlayIndex); const messageId = props.messageId; @@ -152,7 +147,6 @@ const GenericMessageItem = (props: { ...props.messageProps.propsForMessage, firstMessageOfSeries: props.messageProps.firstMessageOfSeries, multiSelectMode, - isQuotedMessageToAnimate: messageId === quotedMessageToAnimate, nextMessageToPlay, playNextMessage: props.playNextMessage, onQuoteClick, @@ -166,7 +160,7 @@ const GenericMessageItem = (props: { multiSelectMode={multiSelectMode} key={messageId} /> - + ); }; @@ -176,7 +170,6 @@ const MessageList = (props: { playNextMessage?: (value: number) => void; }) => { const messagesProps = useSelector(getSortedMessagesOfSelectedConversation); - const firstUnreadMessageIndex = useSelector(getFirstUnreadMessageIndex); const isAbove = useSelector(isFirstUnreadMessageIdAbove); console.warn('isAbove', isAbove); @@ -191,19 +184,12 @@ const MessageList = (props: { const groupNotificationProps = messageProps.propsForGroupNotification; - // IF we found the first unread message - // AND we are not scrolled all the way to the bottom - // THEN, show the unread banner for the current message - const showUnreadIndicator = - Boolean(firstUnreadMessageIndex) && firstUnreadMessageIndex === index; - if (groupNotificationProps) { return ( ); } @@ -214,7 +200,6 @@ const MessageList = (props: { key={messageProps.propsForMessage.id} propsForGroupInvitation={propsForGroupInvitation} messageId={messageProps.propsForMessage.id} - showUnreadIndicator={showUnreadIndicator} /> ); } @@ -225,7 +210,6 @@ const MessageList = (props: { key={messageProps.propsForMessage.id} propsForDataExtractionNotification={propsForDataExtractionNotification} messageId={messageProps.propsForMessage.id} - showUnreadIndicator={showUnreadIndicator} /> ); } @@ -236,7 +220,6 @@ const MessageList = (props: { key={messageProps.propsForMessage.id} timerProps={timerProps} messageId={messageProps.propsForMessage.id} - showUnreadIndicator={showUnreadIndicator} /> ); } @@ -255,7 +238,6 @@ const MessageList = (props: { playableMessageIndex={playableMessageIndex} messageId={messageProps.propsForMessage.id} messageProps={messageProps} - showUnreadIndicator={showUnreadIndicator} scrollToQuoteMessage={props.scrollToQuoteMessage} playNextMessage={props.playNextMessage} /> @@ -266,7 +248,6 @@ const MessageList = (props: { }; class SessionMessagesListInner extends React.Component { - private scrollOffsetBottomPx: number = Number.MAX_VALUE; private ignoreScrollEvents: boolean; private timeoutResetQuotedScroll: NodeJS.Timeout | null = null; @@ -301,7 +282,7 @@ class SessionMessagesListInner extends React.Component { ) { // displayed conversation changed. We have a bit of cleaning to do here this.ignoreScrollEvents = true; - this.setupTimeoutResetQuotedHighlightedMessage(true); + this.setupTimeoutResetQuotedHighlightedMessage(this.props.animateQuotedMessageId); this.initialMessageLoadingPosition(); } else { // if we got new message for this convo, and we are scrolled to bottom @@ -355,7 +336,7 @@ class SessionMessagesListInner extends React.Component { // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ // ~~~~~~~~~~~~~ MESSAGE HANDLING ~~~~~~~~~~~~~ // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ - private updateReadMessages() { + private updateReadMessages(forceIsOnBottom = false) { const { messagesProps, conversationKey } = this.props; if (!messagesProps || messagesProps.length === 0 || !conversationKey) { @@ -372,7 +353,7 @@ class SessionMessagesListInner extends React.Component { return; } - if (this.getScrollOffsetBottomPx() === 0 && isElectronWindowFocused()) { + if ((forceIsOnBottom || this.getScrollOffsetBottomPx() === 0) && isElectronWindowFocused()) { void conversation.markRead(messagesProps[0].propsForMessage.receivedAt || 0); } } @@ -450,10 +431,10 @@ class SessionMessagesListInner extends React.Component { window.inboxStore?.dispatch(showScrollToBottomButton(showScrollButton)); // trigger markRead if we hit the bottom - const isScrolledToBottom = bottomOfBottomMessage >= containerBottom - 5; + const isScrolledToBottom = bottomOfBottomMessage <= containerBottom - 5; if (isScrolledToBottom) { // Mark messages read - this.updateReadMessages(); + this.updateReadMessages(true); } } @@ -522,19 +503,15 @@ class SessionMessagesListInner extends React.Component { * So we need to reset the state of of the highlighted message so when the users clicks again, * the highlight is shown once again */ - private setupTimeoutResetQuotedHighlightedMessage(clearOnly = false) { + private setupTimeoutResetQuotedHighlightedMessage(messageId: string | undefined) { if (this.timeoutResetQuotedScroll) { clearTimeout(this.timeoutResetQuotedScroll); } - // only clear the timeout, do not schedule once again - if (clearOnly) { - return; - } - if (this.props.animateQuotedMessageId !== undefined) { + if (messageId !== undefined) { this.timeoutResetQuotedScroll = global.setTimeout(() => { window.inboxStore?.dispatch(quotedMessageToAnimate(undefined)); - }, 3000); + }, 2000); // should match .flash-green-once } } @@ -548,7 +525,7 @@ class SessionMessagesListInner extends React.Component { // we consider that a `smooth` set to true, means it's a quoted message, so highlight this message on the UI if (smooth) { window.inboxStore?.dispatch(quotedMessageToAnimate(messageId)); - this.setupTimeoutResetQuotedHighlightedMessage(); + this.setupTimeoutResetQuotedHighlightedMessage(messageId); } const messageContainer = this.props.messageContainerRef.current; diff --git a/ts/models/message.ts b/ts/models/message.ts index a19e25896..635ed217b 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -1087,8 +1087,17 @@ export class MessageModel extends Backbone.Model { public async markRead(readAt: number) { this.markReadNoCommit(readAt); - await this.commit(); + + const convo = this.getConversation(); + if (convo) { + const beforeUnread = convo.get('unreadCount'); + const unreadCount = await convo.getUnreadCount(); + if (beforeUnread !== unreadCount) { + convo.set({ unreadCount }); + await convo.commit(); + } + } } public markReadNoCommit(readAt: number) { diff --git a/ts/models/messageType.ts b/ts/models/messageType.ts index ea41391ef..34e5cb2cb 100644 --- a/ts/models/messageType.ts +++ b/ts/models/messageType.ts @@ -244,7 +244,6 @@ export interface MessageRegularProps { multiSelectMode: boolean; firstMessageOfSeries: boolean; isUnread: boolean; - isQuotedMessageToAnimate?: boolean; isTrustedForAttachmentDownload: boolean; onQuoteClick?: (options: QuoteClickOptions) => Promise; diff --git a/ts/receiver/closedGroups.ts b/ts/receiver/closedGroups.ts index 100e9d755..2f7305f85 100644 --- a/ts/receiver/closedGroups.ts +++ b/ts/receiver/closedGroups.ts @@ -31,10 +31,7 @@ import { forceSyncConfigurationNowIfNeeded } from '../session/utils/syncUtils'; import { getMessageController } from '../session/messages'; import { ClosedGroupEncryptionPairReplyMessage } from '../session/messages/outgoing/controlMessage/group/ClosedGroupEncryptionPairReplyMessage'; import { queueAllCachedFromSource } from './receiver'; -import { - actions as conversationActions, - openConversationExternal, -} from '../state/ducks/conversations'; +import { openConversationExternal } from '../state/ducks/conversations'; import { getSwarmPollingInstance } from '../session/snode_api'; import { MessageModel } from '../models/message'; @@ -955,7 +952,9 @@ export async function createClosedGroup(groupName: string, members: Array; - firstUnreadMessageId: string | undefined; }; export const fetchMessagesForConversation = createAsyncThunk( @@ -307,8 +306,6 @@ export const fetchMessagesForConversation = createAsyncThunk( const beforeTimestamp = Date.now(); console.time('fetchMessagesForConversation'); const messagesProps = await getMessages(conversationKey, count); - - const firstUnreadMessageId = await getFirstUnreadMessageIdInConversation(conversationKey); const afterTimestamp = Date.now(); console.timeEnd('fetchMessagesForConversation'); @@ -318,7 +315,6 @@ export const fetchMessagesForConversation = createAsyncThunk( return { conversationKey, messagesProps, - firstUnreadMessageId, }; } ); @@ -594,12 +590,14 @@ const conversationsSlice = createSlice({ state: ConversationsStateType, action: PayloadAction<{ id: string; + firstUnreadIdOnOpen: string | undefined; messageId?: string; }> ) { if (state.selectedConversation === action.payload.id) { return state; } + return { conversationLookup: state.conversationLookup, selectedConversation: action.payload.id, @@ -615,7 +613,7 @@ const conversationsSlice = createSlice({ showScrollButton: false, animateQuotedMessageId: undefined, mentionMembers: [], - firstUnreadMessageId: undefined, + firstUnreadMessageId: action.payload.firstUnreadIdOnOpen, }; }, showLightBox( @@ -662,14 +660,13 @@ const conversationsSlice = createSlice({ fetchMessagesForConversation.fulfilled, (state: ConversationsStateType, action: PayloadAction) => { // this is called once the messages are loaded from the db for the currently selected conversation - const { messagesProps, conversationKey, firstUnreadMessageId } = action.payload; + const { messagesProps, conversationKey } = action.payload; // double check that this update is for the shown convo if (conversationKey === state.selectedConversation) { return { ...state, messages: messagesProps, areMoreMessagesBeingFetched: false, - firstUnreadMessageId, }; } return state; diff --git a/ts/state/ducks/search.ts b/ts/state/ducks/search.ts index 3719a32d5..5cb677fdf 100644 --- a/ts/state/ducks/search.ts +++ b/ts/state/ducks/search.ts @@ -5,11 +5,7 @@ import { cleanSearchTerm } from '../../util/cleanSearchTerm'; import { searchConversations, searchMessages } from '../../../ts/data/data'; import { makeLookup } from '../../util/makeLookup'; -import { - openConversationExternal, - PropsForSearchResults, - ReduxConversationType, -} from './conversations'; +import { PropsForSearchResults, ReduxConversationType } from './conversations'; import { PubKey } from '../../session/types'; import { MessageModel } from '../../models/message'; import { MessageModelType } from '../../models/messageType'; diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index 616227663..f24683956 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -402,14 +402,6 @@ function sortMessages( return messagesSorted; } -export const getFirstUnreadMessageIndex = createSelector( - getSortedMessagesOfSelectedConversation, - (messageModelsProps: Array): number | undefined => { - const firstUnreadIndex = getFirstMessageUnreadIndex(messageModelsProps); - return firstUnreadIndex; - } -); - function getFirstMessageUnreadIndex(messages: Array) { if (!messages || messages.length === 0) { return -1; @@ -442,7 +434,6 @@ function getFirstMessageUnreadIndex(messages: Array) { export const getFirstUnreadMessageId = createSelector( getConversations, (state: ConversationsStateType): string | undefined => { - console.warn('getFirstUnreadMessageId', state.firstUnreadMessageId); return state.firstUnreadMessageId; } );