From 737dbd45c128ccca1fa726acc0b2214e23b4e66d Mon Sep 17 00:00:00 2001 From: yougotwill Date: Tue, 20 Aug 2024 10:57:21 +1000 Subject: [PATCH] fix: show loading state until image is decrypted and can be mounted --- ts/components/conversation/Image.tsx | 83 +++++++++++++++---- ts/components/conversation/ImageGrid.tsx | 41 ++++++--- .../message-content/MessageAttachment.tsx | 9 +- .../message-content/MessageContent.tsx | 8 +- ts/hooks/useEncryptedFileFetch.ts | 75 ++++++++++------- .../crypto/DecryptedAttachmentsManager.ts | 4 +- 6 files changed, 148 insertions(+), 72 deletions(-) diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index 1ad2be0db..bfee52c74 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -1,17 +1,20 @@ import classNames from 'classnames'; -import { useCallback } from 'react'; +import { useCallback, useEffect, useState } from 'react'; import styled from 'styled-components'; import { isNumber } from 'lodash'; import { useDisableDrag } from '../../hooks/useDisableDrag'; -import { useEncryptedFileFetch } from '../../hooks/useEncryptedFileFetch'; import { AttachmentType, AttachmentTypeWithPath } from '../../types/Attachment'; import { Spinner } from '../loading'; +import { MessageModelType } from '../../models/messageType'; +import { MessageGenericAttachment } from './message/message-content/MessageGenericAttachment'; +import { useEncryptedFileFetch } from '../../hooks/useEncryptedFileFetch'; type Props = { alt: string; attachment: AttachmentTypeWithPath | AttachmentType; url: string | undefined; // url is undefined if the message is not visible yet + imageBroken?: boolean; height?: number | string; width?: number | string; @@ -26,10 +29,14 @@ type Props = { forceSquare?: boolean; dropShadow?: boolean; attachmentIndex?: number; + direction?: MessageModelType; + highlight?: boolean; onClick?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; onClickClose?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; onError?: () => void; + + timestamp?: number; }; const StyledOverlay = styled.div>` @@ -46,6 +53,7 @@ export const Image = (props: Props) => { const { alt, attachment, + imageBroken, closeButton, darkOverlay, height: _height, @@ -58,35 +66,78 @@ export const Image = (props: Props) => { forceSquare, dropShadow, attachmentIndex, + direction, + highlight, url, width: _width, + timestamp, } = props; const disableDrag = useDisableDrag(); + const { loading, urlToLoad } = useEncryptedFileFetch( + url, + attachment.contentType, + false, + timestamp + ); const { caption } = attachment || { caption: null }; - let { pending } = attachment || { pending: true }; - - if (!url) { - // force pending to true if the url is undefined, so we show a loader while decrypting the attachemtn - pending = true; - } + const [pending, setPending] = useState(attachment.pending || !url || true); + const [mounted, setMounted] = useState( + (!loading || !pending) && urlToLoad === undefined + ); const canClick = onClick && !pending; const role = canClick ? 'button' : undefined; - const { loading, urlToLoad } = useEncryptedFileFetch(url || '', attachment.contentType, false); - // data will be url if loading is finished and '' if not - const srcData = !loading ? urlToLoad : ''; const onErrorUrlFilterering = useCallback(() => { - if (!loading && !pending && !url && onError) { + if (mounted && url && urlToLoad === '' && onError) { onError(); + setPending(false); } - }, [loading, pending, url, onError]); + }, [mounted, onError, url, urlToLoad]); const width = isNumber(_width) ? `${_width}px` : _width; const height = isNumber(_height) ? `${_height}px` : _height; + useEffect(() => { + if (mounted && url === '') { + setPending(false); + onErrorUrlFilterering(); + window.log.debug( + `WIP: [Image] timestamp ${timestamp} fail url ${url !== '' ? url : 'empty'} urlToLoad ${urlToLoad !== '' ? urlToLoad : 'empty'} loading ${loading} pending ${pending} imageBroken ${imageBroken}` + ); + } + + if (mounted && imageBroken && urlToLoad === '') { + setPending(false); + onErrorUrlFilterering(); + window.log.debug( + `WIP: [Image] timestamp ${timestamp} fail url ${url !== '' ? url : 'empty'} urlToLoad ${urlToLoad !== '' ? urlToLoad : 'empty'} loading ${loading} pending ${pending} imageBroken ${imageBroken}` + ); + } + + if (url) { + setPending(false); + setMounted(!loading && !pending); + window.log.debug( + `WIP: [Image] timestamp ${timestamp} success url ${url !== '' ? url : 'empty'} urlToLoad ${urlToLoad !== '' ? urlToLoad : 'empty'} loading ${loading} pending ${pending} imageBroken ${imageBroken}` + ); + } + }, [imageBroken, loading, mounted, onErrorUrlFilterering, pending, timestamp, url, urlToLoad]); + + if (mounted && imageBroken) { + return ( + + ); + } + return (
{ }} data-attachmentindex={attachmentIndex} > - {pending || loading ? ( + {!mounted || loading || pending ? (
{ width: forceSquare ? width : '', height: forceSquare ? height : '', }} - src={srcData} + src={urlToLoad} onDragStart={disableDrag} /> )} @@ -169,7 +220,7 @@ export const Image = (props: Props) => { className="module-image__close-button" /> ) : null} - {!(pending || loading) && playIconOverlay ? ( + {!pending && playIconOverlay ? (
diff --git a/ts/components/conversation/ImageGrid.tsx b/ts/components/conversation/ImageGrid.tsx index 7e6120b91..c5acd0dcf 100644 --- a/ts/components/conversation/ImageGrid.tsx +++ b/ts/components/conversation/ImageGrid.tsx @@ -10,13 +10,19 @@ import { } from '../../types/Attachment'; import { useIsMessageVisible } from '../../contexts/isMessageVisibleContext'; -import { useMessageSelected } from '../../state/selectors'; +import { + useMessageDirection, + useMessageSelected, + useMessageTimestamp, +} from '../../state/selectors'; import { THUMBNAIL_SIDE } from '../../types/attachments/VisualAttachment'; import { Image } from './Image'; type Props = { attachments: Array; onError: () => void; + imageBroken: boolean; + highlight: boolean; onClickAttachment?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; messageId?: string; }; @@ -33,22 +39,27 @@ const Row = ( renderedSize: number; startIndex: number; totalAttachmentsCount: number; - selected: boolean; } ) => { const { attachments, + imageBroken, + highlight, onError, renderedSize, startIndex, onClickAttachment, totalAttachmentsCount, - selected, + messageId, } = props; const isMessageVisible = useIsMessageVisible(); const moreMessagesOverlay = totalAttachmentsCount > 3; const moreMessagesOverlayText = moreMessagesOverlay ? `+${totalAttachmentsCount - 3}` : undefined; + const selected = useMessageSelected(messageId); + const direction = useMessageDirection(messageId); + const timestamp = useMessageTimestamp(messageId); + return ( <> {attachments.map((attachment, index) => { @@ -64,11 +75,15 @@ const Row = ( url={isMessageVisible ? getThumbnailUrl(attachment) : undefined} attachmentIndex={startIndex + index} onClick={onClickAttachment} + imageBroken={imageBroken} + highlight={highlight} onError={onError} softCorners={true} darkOverlay={showOverlay} overlayText={showOverlay ? moreMessagesOverlayText : undefined} dropShadow={selected} + direction={direction} + timestamp={timestamp} /> ); })} @@ -77,9 +92,7 @@ const Row = ( }; export const ImageGrid = (props: Props) => { - const { attachments, onError, onClickAttachment, messageId } = props; - - const selected = useMessageSelected(messageId); + const { attachments, imageBroken, highlight, onError, onClickAttachment, messageId } = props; if (!attachments || !attachments.length) { return null; @@ -90,12 +103,14 @@ export const ImageGrid = (props: Props) => { ); @@ -107,12 +122,14 @@ export const ImageGrid = (props: Props) => { ); @@ -125,23 +142,27 @@ export const ImageGrid = (props: Props) => { diff --git a/ts/components/conversation/message/message-content/MessageAttachment.tsx b/ts/components/conversation/message/message-content/MessageAttachment.tsx index aa680f4b8..8472f79d5 100644 --- a/ts/components/conversation/message/message-content/MessageAttachment.tsx +++ b/ts/components/conversation/message/message-content/MessageAttachment.tsx @@ -15,8 +15,6 @@ import { import { AttachmentType, AttachmentTypeWithPath, - hasImage, - hasVideoScreenshot, isAudio, isImage, isVideo, @@ -128,14 +126,12 @@ export const MessageAttachment = (props: Props) => { return ; } - if ( - (isImage(attachments) && hasImage(attachments)) || - (isVideo(attachments) && hasVideoScreenshot(attachments)) - ) { + if (isImage(attachments) || isVideo(attachments)) { // we use the carousel in the detail view if (isDetailView) { return null; } + return ( @@ -177,6 +173,7 @@ export const MessageAttachment = (props: Props) => { return ( { return null; } - const { direction, text, timestamp, serverTimestamp, previews, quote, attachments } = - contentProps; + const { direction, text, timestamp, serverTimestamp, previews, quote } = contentProps; const hasContentBeforeAttachment = !isEmpty(previews) || !isEmpty(quote) || !isEmpty(text); const toolTipTitle = moment(serverTimestamp || timestamp).format('llll'); - window.log.debug( - `WIP: [MessageAttachment] ${props.messageId} timestamp ${timestamp} imageBroken ${imageBroken} display ${canDisplayImagePreview(attachments)} attachments.contentType ${attachments?.[0].contentType || 'none'}` - ); - return ( { - const [urlToLoad, setUrlToLoad] = useState(''); - const [loading, setLoading] = useState(false); - - const mountedRef = useRef(true); +export const useEncryptedFileFetch = ( + url: string | undefined, + contentType: string, + isAvatar: boolean, + timestamp?: number +) => { + const [urlToLoad, setUrlToLoad] = useState(undefined); + const [loading, setLoading] = useState(true); + + const alreadyDecrypted = url ? getAlreadyDecryptedMediaUrl(url) : ''; + + const fetchUrl = useCallback( + async (mediaUrl: string | undefined) => { + if (alreadyDecrypted || !mediaUrl) { + window.log.debug( + `WIP: [Image] timestamp ${timestamp} alreadyDecrypted ${alreadyDecrypted !== '' ? alreadyDecrypted : 'empty'} mediaUrl ${mediaUrl !== '' ? mediaUrl : 'empty'}` + ); + + if (alreadyDecrypted) { + setUrlToLoad(alreadyDecrypted); + setLoading(false); + } + return; + } - const alreadyDecrypted = getAlreadyDecryptedMediaUrl(url); + setLoading(true); - useEffect(() => { - async function fetchUrl() { - perfStart(`getDecryptedMediaUrl-${url}`); - const decryptedUrl = await getDecryptedMediaUrl(url, contentType, isAvatar); - perfEnd(`getDecryptedMediaUrl-${url}`, `getDecryptedMediaUrl-${url}`); + try { + perfStart(`getDecryptedMediaUrl-${mediaUrl}`); + const decryptedUrl = await getDecryptedMediaUrl(mediaUrl, contentType, isAvatar); + perfEnd(`getDecryptedMediaUrl-${mediaUrl}`, `getDecryptedMediaUrl-${mediaUrl}`); + window.log.debug( + `WIP: [Image] timestamp ${timestamp} decryptedUrl ${decryptedUrl !== '' ? decryptedUrl : 'empty'}` + ); - if (mountedRef.current) { setUrlToLoad(decryptedUrl); + } catch (error) { + window.log.error(`WIP: [Image] timestamp ${timestamp} error ${error}`); + setUrlToLoad(''); + } finally { setLoading(false); } - } - if (alreadyDecrypted) { - return; - } - setLoading(true); - mountedRef.current = true; - void fetchUrl(); - - // eslint-disable-next-line consistent-return - return () => { - mountedRef.current = false; - }; - }, [url, alreadyDecrypted, contentType, isAvatar]); - - if (alreadyDecrypted) { - return { urlToLoad: alreadyDecrypted, loading: false }; - } + }, + [alreadyDecrypted, contentType, isAvatar, timestamp] + ); + + useEffect(() => { + void fetchUrl(url); + }, [fetchUrl, url]); + return { urlToLoad, loading }; }; diff --git a/ts/session/crypto/DecryptedAttachmentsManager.ts b/ts/session/crypto/DecryptedAttachmentsManager.ts index b8567a00e..abf63ad60 100644 --- a/ts/session/crypto/DecryptedAttachmentsManager.ts +++ b/ts/session/crypto/DecryptedAttachmentsManager.ts @@ -10,7 +10,6 @@ * */ import path from 'path'; -import { reject } from 'lodash'; import * as fse from 'fs-extra'; @@ -114,7 +113,7 @@ export const getDecryptedMediaUrl = async ( urlToDecryptingPromise.set( url, - new Promise(async resolve => { + new Promise(async (resolve, reject) => { // window.log.debug('about to read and decrypt file :', url, path.isAbsolute(url)); try { const absUrl = path.isAbsolute(url) ? url : getAbsoluteAttachmentPath(url); @@ -149,7 +148,6 @@ export const getDecryptedMediaUrl = async ( } }) ); - return urlToDecryptingPromise.get(url) as Promise; } // Not sure what we got here. Just return the file.