From 3f065a7b0ebefb7751c2c5c941c34c4e2f3ff516 Mon Sep 17 00:00:00 2001 From: Audric Ackermann Date: Fri, 30 Jul 2021 16:12:36 +1000 Subject: [PATCH] improve marking message as read with hooks --- ts/components/conversation/ExpireTimer.tsx | 6 +- ts/components/conversation/Message.tsx | 88 +----------- ts/components/conversation/MessageDetail.tsx | 1 - .../conversation/ReadableMessage.tsx | 126 +++++++++++++++++- ts/components/session/SessionScrollButton.tsx | 21 ++- .../conversation/SessionMessagesTypes.tsx | 4 +- .../session/icon/SessionIconButton.tsx | 3 + ts/hooks/useAppFocused.ts | 40 ++++++ ts/hooks/useFocus.ts | 10 -- ts/models/conversation.ts | 4 +- ts/models/message.ts | 3 +- ts/models/messageType.ts | 6 +- ts/receiver/dataMessage.ts | 1 + ts/receiver/queuedJob.ts | 32 +---- ts/session/utils/WindowUtils.ts | 3 +- ts/state/selectors/conversations.ts | 2 +- 16 files changed, 198 insertions(+), 152 deletions(-) create mode 100644 ts/hooks/useAppFocused.ts delete mode 100644 ts/hooks/useFocus.ts diff --git a/ts/components/conversation/ExpireTimer.tsx b/ts/components/conversation/ExpireTimer.tsx index e3ef1819c..b03961c63 100644 --- a/ts/components/conversation/ExpireTimer.tsx +++ b/ts/components/conversation/ExpireTimer.tsx @@ -1,4 +1,4 @@ -import React, { useState } from 'react'; +import React, { useCallback, useState } from 'react'; import { getTimerBucketIcon } from '../../util/timer'; import { useInterval } from '../../hooks/useInterval'; @@ -45,12 +45,12 @@ export const ExpireTimer = (props: Props) => { const initialTimeLeft = Math.max(Math.round((expirationTimestamp - Date.now()) / 1000), 0); const [timeLeft, setTimeLeft] = useState(initialTimeLeft); - const update = () => { + const update = useCallback(() => { const newTimeLeft = Math.max(Math.round((expirationTimestamp - Date.now()) / 1000), 0); if (newTimeLeft !== timeLeft) { setTimeLeft(newTimeLeft); } - }; + }, [expirationTimestamp, timeLeft, setTimeLeft]); const updateFrequency = 500; diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx index dff9b68d6..9b5fd9ed2 100644 --- a/ts/components/conversation/Message.tsx +++ b/ts/components/conversation/Message.tsx @@ -39,31 +39,20 @@ import { getMessageById } from '../../data/data'; import { connect } from 'react-redux'; import { StateType } from '../../state/reducer'; import { - areMoreMessagesBeingFetched, - getLoadedMessagesLength, - getMostRecentMessageId, - getOldestMessageId, getQuotedMessageToAnimate, - getSelectedConversationKey, getSelectedMessageIds, - haveDoneFirstScroll, } from '../../state/selectors/conversations'; import { - fetchMessagesForConversation, - markConversationFullyRead, messageExpired, showLightBox, - showScrollToBottomButton, toggleSelectedMessageId, } from '../../state/ducks/conversations'; import { saveAttachmentToDisk } from '../../util/attachmentsUtil'; import { LightBoxOptions } from '../session/conversation/SessionConversation'; import { MessageContextMenu } from './MessageContextMenu'; import { ReadableMessage } from './ReadableMessage'; -import { isElectronWindowFocused } from '../../session/utils/WindowUtils'; import { getConversationController } from '../../session/conversations'; import { MessageMetadata } from './message/MessageMetadata'; -import { Constants } from '../../session'; // Same as MIN_WIDTH in ImageGrid.tsx const MINIMUM_LINK_PREVIEW_IMAGE_WIDTH = 200; @@ -80,12 +69,6 @@ const EXPIRED_DELAY = 600; type Props = MessageRenderingProps & { selectedMessages: Array; quotedMessageToAnimate: string | undefined; - mostRecentMessageId: string | undefined; - oldestMessageId: string | undefined; - areMoreMessagesBeingFetched: boolean; - loadedMessagesLength: number; - selectedConversationKey: string | undefined; - haveDoneFirstScroll: boolean; }; function attachmentIsAttachmentTypeWithPath(attac: any): attac is AttachmentTypeWithPath { @@ -147,7 +130,6 @@ class MessageInner extends React.PureComponent { imageBroken: false, }; this.ctxMenuID = `ctx-menu-message-${uuid()}`; - this.loadMoreMessages = _.debounce(this.loadMoreMessages, 100); } public componentDidMount() { @@ -175,7 +157,7 @@ class MessageInner extends React.PureComponent { } } - public componentDidUpdate() { + public componentDidUpdate(prevProps: Props) { this.checkExpired(); } @@ -617,9 +599,9 @@ class MessageInner extends React.PureComponent { direction, id: messageId, conversationType, - areMoreMessagesBeingFetched: fetchingMore, - isUnread, selectedMessages, + receivedAt, + isUnread, } = this.props; const { expired, expiring } = this.state; @@ -632,8 +614,6 @@ class MessageInner extends React.PureComponent { const width = this.getWidth(); const isShowingImage = this.isShowingImage(); - const isIncoming = direction === 'incoming'; - const shouldMarkReadWhenVisible = isIncoming && isUnread; const divClasses = ['session-message-wrapper']; if (selected) { @@ -648,52 +628,16 @@ class MessageInner extends React.PureComponent { divClasses.push('flash-green-once'); } - const onVisible = async (inView: boolean | Object) => { - // when the view first loads, it needs to scroll to the unread messages. - // we need to disable the inview on the first loading - if (!this.props.haveDoneFirstScroll) { - if (inView === true) { - window.log.info('onVisible but waiting for first scroll event'); - } - return; - } - // we are the bottom message - if (this.props.mostRecentMessageId === messageId && isElectronWindowFocused()) { - if (inView === true) { - window.inboxStore?.dispatch(showScrollToBottomButton(false)); - void getConversationController() - .get(this.props.selectedConversationKey as string) - ?.markRead(this.props.receivedAt || 0) - .then(() => { - window.inboxStore?.dispatch( - markConversationFullyRead(this.props.selectedConversationKey as string) - ); - }); - } else if (inView === false) { - window.inboxStore?.dispatch(showScrollToBottomButton(true)); - } - } - - if (inView === true && this.props.oldestMessageId === messageId && !fetchingMore) { - this.loadMoreMessages(); - } - if (inView === true && shouldMarkReadWhenVisible && isElectronWindowFocused()) { - const found = await getMessageById(messageId); - - if (found && Boolean(found.get('unread'))) { - // mark the message as read. - // this will trigger the expire timer. - void found.markRead(Date.now()); - } - } - }; + const isIncoming = direction === 'incoming'; return ( {this.renderAvatar()} @@ -767,18 +711,6 @@ class MessageInner extends React.PureComponent { ); } - private loadMoreMessages() { - const { loadedMessagesLength, selectedConversationKey } = this.props; - - const numMessages = loadedMessagesLength + Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT; - (window.inboxStore?.dispatch as any)( - fetchMessagesForConversation({ - conversationKey: selectedConversationKey as string, - count: numMessages, - }) - ); - } - private handleContextMenu(e: any) { e.preventDefault(); e.stopPropagation(); @@ -935,12 +867,6 @@ const mapStateToProps = (state: StateType) => { return { selectedMessages: getSelectedMessageIds(state), quotedMessageToAnimate: getQuotedMessageToAnimate(state), - mostRecentMessageId: getMostRecentMessageId(state), - oldestMessageId: getOldestMessageId(state), - areMoreMessagesBeingFetched: areMoreMessagesBeingFetched(state), - selectedConversationKey: getSelectedConversationKey(state), - loadedMessagesLength: getLoadedMessagesLength(state), - haveDoneFirstScroll: haveDoneFirstScroll(state), }; }; diff --git a/ts/components/conversation/MessageDetail.tsx b/ts/components/conversation/MessageDetail.tsx index db3a81c5f..6ec598533 100644 --- a/ts/components/conversation/MessageDetail.tsx +++ b/ts/components/conversation/MessageDetail.tsx @@ -5,7 +5,6 @@ import moment from 'moment'; import { Avatar, AvatarSize } from '../Avatar'; import { ContactName } from './ContactName'; import { Message } from './Message'; -import { MessageRenderingProps } from '../../models/messageType'; import { deleteMessagesById } from '../../interactions/conversationInteractions'; import { useSelector } from 'react-redux'; import { ContactPropsMessageDetail } from '../../state/ducks/conversations'; diff --git a/ts/components/conversation/ReadableMessage.tsx b/ts/components/conversation/ReadableMessage.tsx index 44013687c..851fc1d52 100644 --- a/ts/components/conversation/ReadableMessage.tsx +++ b/ts/components/conversation/ReadableMessage.tsx @@ -1,29 +1,141 @@ -import React from 'react'; -import { useFocus } from '../../hooks/useFocus'; +import _, { noop } from 'lodash'; +import React, { useCallback } from 'react'; import { InView } from 'react-intersection-observer'; +import { useDispatch, useSelector } from 'react-redux'; +// tslint:disable-next-line: no-submodule-imports +import useDebounce from 'react-use/lib/useDebounce'; +import { getMessageById } from '../../data/data'; +import { useAppIsFocused } from '../../hooks/useAppFocused'; +import { MessageModelType } from '../../models/messageType'; +import { Constants } from '../../session'; +import { getConversationController } from '../../session/conversations'; +import { + fetchMessagesForConversation, + markConversationFullyRead, + showScrollToBottomButton, +} from '../../state/ducks/conversations'; +import { + areMoreMessagesBeingFetched, + getHaveDoneFirstScroll, + getLoadedMessagesLength, + getMostRecentMessageId, + getOldestMessageId, + getSelectedConversationKey, +} from '../../state/selectors/conversations'; type ReadableMessageProps = { children: React.ReactNode; messageId: string; className: string; - onChange: (inView: boolean) => void; + receivedAt: number | undefined; + isUnread: boolean; + direction: MessageModelType; + onContextMenu: (e: any) => void; }; +const debouncedTriggerLoadMore = _.debounce( + (loadedMessagesLength: number, selectedConversationKey: string | undefined) => { + const numMessages = loadedMessagesLength + Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT; + (window.inboxStore?.dispatch as any)( + fetchMessagesForConversation({ + conversationKey: selectedConversationKey as string, + count: numMessages, + }) + ); + }, + 100 +); + export const ReadableMessage = (props: ReadableMessageProps) => { - const { onChange, messageId, onContextMenu, className } = props; - useFocus(onChange); + const { messageId, onContextMenu, className, receivedAt, isUnread, direction } = props; + + const isAppFocused = useAppIsFocused(); + const dispatch = useDispatch(); + // onVisible={haveDoneFirstScrollProp ? onVisible : noop} + + const selectedConversationKey = useSelector(getSelectedConversationKey); + const loadedMessagesLength = useSelector(getLoadedMessagesLength); + const haveDoneFirstScroll = useSelector(getHaveDoneFirstScroll); + const mostRecentMessageId = useSelector(getMostRecentMessageId); + const oldestMessageId = useSelector(getOldestMessageId); + const fetchingMore = useSelector(areMoreMessagesBeingFetched); + const isIncoming = direction === 'incoming'; + const shouldMarkReadWhenVisible = isIncoming && isUnread; + + const onVisible = useCallback( + async (inView: boolean | Object) => { + // when the view first loads, it needs to scroll to the unread messages. + // we need to disable the inview on the first loading + if (!haveDoneFirstScroll) { + if (inView === true) { + window.log.info('onVisible but waiting for first scroll event'); + } + return; + } + // we are the most recent message + if (mostRecentMessageId === messageId) { + // 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) + ?.markRead(receivedAt || 0) + .then(() => { + dispatch(markConversationFullyRead(selectedConversationKey as string)); + }); + } else if (inView === false) { + dispatch(showScrollToBottomButton(true)); + } + } + + if (inView === true && isAppFocused && oldestMessageId === messageId && !fetchingMore) { + debouncedTriggerLoadMore(loadedMessagesLength, selectedConversationKey); + } + + // this part is just handling the marking of the message as read if needed + if ( + (inView === true || + ((inView as any).type === 'focus' && (inView as any).returnValue === true)) && + shouldMarkReadWhenVisible && + isAppFocused + ) { + const found = await getMessageById(messageId); + + if (found && Boolean(found.get('unread'))) { + // mark the message as read. + // this will trigger the expire timer. + await found.markRead(Date.now()); + } + } + }, + [ + selectedConversationKey, + haveDoneFirstScroll, + mostRecentMessageId, + oldestMessageId, + fetchingMore, + isAppFocused, + loadedMessagesLength, + receivedAt, + shouldMarkReadWhenVisible, + messageId, + debouncedTriggerLoadMore, + ] + ); return ( + // tslint:disable-next-line: use-simple-attributes {props.children} diff --git a/ts/components/session/SessionScrollButton.tsx b/ts/components/session/SessionScrollButton.tsx index 2f6db774f..17d1cb6b0 100644 --- a/ts/components/session/SessionScrollButton.tsx +++ b/ts/components/session/SessionScrollButton.tsx @@ -22,17 +22,14 @@ export const SessionScrollButton = (props: Props) => { const show = useSelector(getShowScrollButton); return ( - <> - {show && ( - - - - )} - + + + ); }; diff --git a/ts/components/session/conversation/SessionMessagesTypes.tsx b/ts/components/session/conversation/SessionMessagesTypes.tsx index 64aae3e33..34df2635f 100644 --- a/ts/components/session/conversation/SessionMessagesTypes.tsx +++ b/ts/components/session/conversation/SessionMessagesTypes.tsx @@ -109,14 +109,14 @@ export const GenericMessageItem = (props: { }; return ( - + - + ); }; diff --git a/ts/components/session/icon/SessionIconButton.tsx b/ts/components/session/icon/SessionIconButton.tsx index 495cdce2e..aed2a8c94 100644 --- a/ts/components/session/icon/SessionIconButton.tsx +++ b/ts/components/session/icon/SessionIconButton.tsx @@ -9,6 +9,7 @@ interface SProps extends SessionIconProps { notificationCount?: number; isSelected?: boolean; theme?: DefaultTheme; + isHidden?: boolean; } export const SessionIconButton = (props: SProps) => { @@ -23,6 +24,7 @@ export const SessionIconButton = (props: SProps) => { glowDuration, glowStartDelay, noScale, + isHidden, } = props; const clickHandler = (e: any) => { if (props.onClick) { @@ -38,6 +40,7 @@ export const SessionIconButton = (props: SProps) => { className={classNames('session-icon-button', iconSize, isSelected ? 'no-opacity' : '')} role="button" onClick={clickHandler} + style={{ display: isHidden ? 'none' : 'flex' }} > { + setIsAppFocused(isElectronWindowFocused()); + }, []); + + const onFocusCallback = useCallback( + (_event, win) => { + if (win.webContents.id === 1) { + setIsAppFocused(true); + } + }, + [setIsAppFocused] + ); + + const onBlurCallback = useCallback( + (_event, win) => { + if (win.webContents.id === 1) { + setIsAppFocused(false); + } + }, + [setIsAppFocused] + ); + + useEffect(() => { + remote.app.on('browser-window-focus', onFocusCallback); + remote.app.on('browser-window-blur', onBlurCallback); + return () => { + remote.app.removeListener('browser-window-blur', onBlurCallback); + remote.app.removeListener('browser-window-focus', onFocusCallback); + }; + }); + + return isAppFocused; +} diff --git a/ts/hooks/useFocus.ts b/ts/hooks/useFocus.ts deleted file mode 100644 index 8f62de92e..000000000 --- a/ts/hooks/useFocus.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { useEffect } from 'react'; - -export const useFocus = (action: (param: any) => void) => { - useEffect(() => { - window.addEventListener('focus', action); - return () => { - window.removeEventListener('focus', action); - }; - }); -}; diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index 474ddf144..62af3f53b 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -837,7 +837,7 @@ export class ConversationModel extends Backbone.Model { source, }); - const isOutgoing = Boolean(receivedAt); + const isOutgoing = Boolean(!receivedAt); source = source || UserUtils.getOurPubKeyStrFromCache(); @@ -850,7 +850,7 @@ export class ConversationModel extends Backbone.Model { const messageAttributes = { // Even though this isn't reflected to the user, we want to place the last seen // indicator above it. We set it to 'unread' to trigger that placement. - unread: 1, + unread: isOutgoing ? 0 : 1, conversationId: this.id, // No type; 'incoming' messages are specially treated by conversation.markRead() sent_at: timestamp, diff --git a/ts/models/message.ts b/ts/models/message.ts index 52b81ee85..55a9b99cc 100644 --- a/ts/models/message.ts +++ b/ts/models/message.ts @@ -1179,9 +1179,8 @@ export class MessageModel extends Backbone.Model { } } private dispatchMessageUpdate() { - trotthledAllMessagesDispatch(); - updatesToDispatch.set(this.id, this.getProps()); + trotthledAllMessagesDispatch(); } } diff --git a/ts/models/messageType.ts b/ts/models/messageType.ts index 353215118..aa58249da 100644 --- a/ts/models/messageType.ts +++ b/ts/models/messageType.ts @@ -1,11 +1,7 @@ -import { DefaultTheme } from 'styled-components'; import _ from 'underscore'; import { v4 as uuidv4 } from 'uuid'; -import { QuotedAttachmentType } from '../components/conversation/Quote'; import { PropsForMessage } from '../state/ducks/conversations'; -import { AttachmentType, AttachmentTypeWithPath } from '../types/Attachment'; -import { Contact } from '../types/Contact'; -import { ConversationTypeEnum } from './conversation'; +import { AttachmentTypeWithPath } from '../types/Attachment'; export type MessageModelType = 'incoming' | 'outgoing'; export type MessageDeliveryStatus = 'sending' | 'sent' | 'read' | 'error'; diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index 8e970a3ee..558755092 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -101,6 +101,7 @@ async function updateProfile( ConversationTypeEnum.PRIVATE ); await conv.setLokiProfile(newProfile); + await conv.commit(); } function cleanAttachment(attachment: any) { diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index fc8b66346..e8a25a1eb 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -88,11 +88,8 @@ function contentTypeSupported(type: string): boolean { return Chrome.isImageTypeSupported(type) || Chrome.isVideoTypeSupported(type); } -async function copyFromQuotedMessage( - msg: MessageModel, - quote?: Quote, - attemptCount: number = 1 -): Promise { +// tslint:disable-next-line: cyclomatic-complexity +async function copyFromQuotedMessage(msg: MessageModel, quote?: Quote): Promise { const { upgradeMessageSchema } = window.Signal.Migrations; const { Message: TypedMessage, Errors } = window.Signal.Types; @@ -100,10 +97,10 @@ async function copyFromQuotedMessage( return; } - const { attachments, id: longId, author } = quote; + const { attachments, id: quoteId, author } = quote; const firstAttachment = attachments[0]; - const id: number = Long.isLong(longId) ? longId.toNumber() : longId; + const id: number = Long.isLong(quoteId) ? quoteId.toNumber() : quoteId; // We always look for the quote by sentAt timestamp, for opengroups, closed groups and session chats // this will return an array of sent message by id we have locally. @@ -115,17 +112,10 @@ async function copyFromQuotedMessage( }); if (!found) { - // Exponential backoff, giving up after 5 attempts: - if (attemptCount < 5) { - setTimeout(() => { - window?.log?.info(`Looking for the message id : ${id}, attempt: ${attemptCount + 1}`); - void copyFromQuotedMessage(msg, quote, attemptCount + 1); - }, attemptCount * attemptCount * 500); - } else { - window?.log?.warn(`We did not found quoted message ${id} after ${attemptCount} attempts.`); - } - + window?.log?.warn(`We did not found quoted message ${id}.`); quote.referencedMessageNotFound = true; + msg.set({ quote }); + await msg.commit(); return; } @@ -135,14 +125,6 @@ async function copyFromQuotedMessage( const queryMessage = getMessageController().register(found.id, found); quote.text = queryMessage.get('body') || ''; - if (attemptCount > 1) { - // Normally the caller would save the message, but in case we are - // called by a timer, we need to update the message manually - msg.set({ quote }); - await msg.commit(); - return; - } - if (!firstAttachment || !contentTypeSupported(firstAttachment.contentType)) { return; } diff --git a/ts/session/utils/WindowUtils.ts b/ts/session/utils/WindowUtils.ts index 95f6a0edf..b3198aeb5 100644 --- a/ts/session/utils/WindowUtils.ts +++ b/ts/session/utils/WindowUtils.ts @@ -1,4 +1,5 @@ -import { remote } from 'electron'; +import { app, BrowserWindow, remote } from 'electron'; +import { useEffect, useState } from 'react'; export function isElectronWindowFocused() { const [yourBrowserWindow] = remote.BrowserWindow.getAllWindows(); diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index 763be4bd6..a3f1423de 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -320,7 +320,7 @@ export const areMoreMessagesBeingFetched = createSelector( (state: ConversationsStateType): boolean => state.areMoreMessagesBeingFetched || false ); -export const haveDoneFirstScroll = createSelector( +export const getHaveDoneFirstScroll = createSelector( getConversations, (state: ConversationsStateType): boolean => state.haveDoneFirstScroll );