From b1616d4e1336aa08efa61eb8ce14684863e17c74 Mon Sep 17 00:00:00 2001 From: yougotwill Date: Wed, 14 Aug 2024 16:00:51 +1000 Subject: [PATCH 01/11] fix: updated DebugMessageInfo --- .../message-info/components/MessageInfo.tsx | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/ts/components/conversation/right-panel/overlay/message-info/components/MessageInfo.tsx b/ts/components/conversation/right-panel/overlay/message-info/components/MessageInfo.tsx index d977ea8b8..2cb733322 100644 --- a/ts/components/conversation/right-panel/overlay/message-info/components/MessageInfo.tsx +++ b/ts/components/conversation/right-panel/overlay/message-info/components/MessageInfo.tsx @@ -85,6 +85,8 @@ const DebugMessageInfo = ({ messageId }: { messageId: string }) => { const expirationType = useMessageExpirationType(messageId); const expirationDurationMs = useMessageExpirationDurationMs(messageId); const expirationTimestamp = useMessageExpirationTimestamp(messageId); + const timestamp = useMessageTimestamp(messageId); + const serverTimestamp = useMessageServerTimestamp(messageId); if (!isDevProd()) { return null; @@ -92,29 +94,25 @@ const DebugMessageInfo = ({ messageId }: { messageId: string }) => { return ( <> - {convoId ? ( - - ) : null} - {messageHash ? ( - - ) : null} - {serverId ? ( - - ) : null} - {expirationType ? ( - + {convoId ? : null} + {messageHash ? : null} + {serverId ? : null} + {timestamp ? : null} + {serverTimestamp ? ( + ) : null} + {expirationType ? : null} {expirationDurationMs ? ( ) : null} {expirationTimestamp ? ( ) : null} From 24b3feb39e59fd8dc17186cf5fc956496665df34 Mon Sep 17 00:00:00 2001 From: yougotwill Date: Wed, 14 Aug 2024 17:00:34 +1000 Subject: [PATCH 02/11] fix: load attachments in the messages list --- ts/components/conversation/Image.tsx | 13 ++++++++----- .../message/message-content/MessageAttachment.tsx | 6 ++++++ .../message/message-content/MessageContent.tsx | 9 +++++---- 3 files changed, 19 insertions(+), 9 deletions(-) diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index 07ee2e577..1ad2be0db 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -62,25 +62,28 @@ export const Image = (props: Props) => { width: _width, } = props; - const onErrorUrlFilterering = useCallback(() => { - if (url && onError) { - onError(); - } - }, [url, onError]); const disableDrag = useDisableDrag(); 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 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) { + onError(); + } + }, [loading, pending, url, onError]); + const width = isNumber(_width) ? `${_width}px` : _width; const height = isNumber(_height) ? `${_height}px` : _height; diff --git a/ts/components/conversation/message/message-content/MessageAttachment.tsx b/ts/components/conversation/message/message-content/MessageAttachment.tsx index 6534faed2..a151da754 100644 --- a/ts/components/conversation/message/message-content/MessageAttachment.tsx +++ b/ts/components/conversation/message/message-content/MessageAttachment.tsx @@ -31,6 +31,7 @@ import { AudioPlayerWithEncryptedFile } from '../../H5AudioPlayer'; import { ImageGrid } from '../../ImageGrid'; import { ClickToTrustSender } from './ClickToTrustSender'; import { MessageHighlighter } from './MessageHighlighter'; +import { useIsDetailMessageView } from '../../../../contexts/isDetailViewContext'; export type MessageAttachmentSelectorProps = Pick< MessageRenderingProps, @@ -67,6 +68,7 @@ const StyledGenericAttachmentContainer = styled(MessageHighlighter)<{ selected: export const MessageAttachment = (props: Props) => { const { messageId, imageBroken, handleImageError, highlight = false } = props; + const isDetailView = useIsDetailMessageView(); const dispatch = useDispatch(); const attachmentProps = useSelector((state: StateType) => @@ -140,6 +142,10 @@ export const MessageAttachment = (props: Props) => { ((isImage(attachments) && hasImage(attachments)) || (isVideo(attachments) && hasVideoScreenshot(attachments))) ) { + // we use the carousel in the detail view + if (isDetailView) { + return null; + } return ( diff --git a/ts/components/conversation/message/message-content/MessageContent.tsx b/ts/components/conversation/message/message-content/MessageContent.tsx index 1f0c87219..9f04ee5de 100644 --- a/ts/components/conversation/message/message-content/MessageContent.tsx +++ b/ts/components/conversation/message/message-content/MessageContent.tsx @@ -154,8 +154,9 @@ export const MessageContent = (props: Props) => { const toolTipTitle = moment(serverTimestamp || timestamp).format('llll'); - const isDetailViewAndSupportsAttachmentCarousel = - isDetailView && canDisplayImagePreview(attachments); + window.log.debug( + `WIP: [MessageAttachment] ${props.messageId} timestamp ${timestamp} imageBroken ${imageBroken} display ${canDisplayImagePreview(attachments)} attachments.contentType ${attachments?.[0].contentType || 'none'}` + ); return ( { )} - {!isDeleted && isDetailViewAndSupportsAttachmentCarousel && !imageBroken ? null : ( + {!isDeleted ? ( - )} + ) : null} From bdbc48590e4caf586f44a6635c9ef674d17b649e Mon Sep 17 00:00:00 2001 From: yougotwill Date: Thu, 15 Aug 2024 10:06:15 +1000 Subject: [PATCH 03/11] feat: consolidated attachment types --- ts/state/ducks/conversations.ts | 29 +++-------------- ts/types/Attachment.ts | 56 ++++++++++++++------------------- 2 files changed, 27 insertions(+), 58 deletions(-) diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index ad5064570..3f1a4a96f 100644 --- a/ts/state/ducks/conversations.ts +++ b/ts/state/ducks/conversations.ts @@ -25,6 +25,7 @@ import { PropsForCallNotification, PropsForInteractionNotification, } from './types'; +import { AttachmentType } from '../../types/Attachment'; export type MessageModelPropsWithoutConvoProps = { propsForMessage: PropsForMessageWithoutConvoProps; @@ -128,35 +129,13 @@ export type PropsForGroupInvitation = { messageId: string; }; -export type PropsForAttachment = { +export type PropsForAttachment = AttachmentType & { id: number; - contentType: string; - caption?: string; + isVoiceMessage: boolean; size: number; - width?: number; - height?: number; - duration?: string; - url: string; path: string; - fileSize: string | null; - isVoiceMessage: boolean; pending: boolean; - fileName: string; - error?: number; // if the download somhehow failed, this will be set to true and be 0-1 once saved in the db - screenshot: { - contentType: string; - width: number; - height: number; - url?: string; - path?: string; - } | null; - thumbnail: { - contentType: string; - width: number; - height: number; - url?: string; - path?: string; - } | null; + error?: number; // if the download somehow failed, this will be set to true and be 0 or 1 in the db }; export type PropsForQuote = { diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index 1154f6b64..aec9057b5 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -13,35 +13,40 @@ const MAX_HEIGHT = THUMBNAIL_SIDE; const MIN_WIDTH = THUMBNAIL_SIDE; const MIN_HEIGHT = THUMBNAIL_SIDE; -// Used for display +// Used for displaying attachments in the UI +export type AttachmentScreenshot = { + contentType: MIME.MIMEType; + height: number; + width: number; + url?: string; + path?: string; +}; + +export type AttachmentThumbnail = { + contentType: MIME.MIMEType; + height: number; + width: number; + url?: string; + path?: string; +}; export interface AttachmentType { - caption?: string; contentType: MIME.MIMEType; fileName: string; - /** Not included in protobuf, needs to be pulled from flags */ - isVoiceMessage?: boolean; /** For messages not already on disk, this will be a data url */ url: string; - videoUrl?: string; - size?: number; fileSize: string | null; pending?: boolean; + screenshot: AttachmentScreenshot | null; + thumbnail: AttachmentThumbnail | null; + caption?: string; + size?: number; width?: number; height?: number; duration?: string; - screenshot: { - height: number; - width: number; - url?: string; - contentType: MIME.MIMEType; - } | null; - thumbnail: { - height: number; - width: number; - url?: string; - contentType: MIME.MIMEType; - } | null; + videoUrl?: string; + /** Not included in protobuf, needs to be pulled from flags */ + isVoiceMessage?: boolean; } export interface AttachmentTypeWithPath extends AttachmentType { @@ -49,21 +54,6 @@ export interface AttachmentTypeWithPath extends AttachmentType { id: number; flags?: number; error?: any; - - screenshot: { - height: number; - width: number; - url?: string; - contentType: MIME.MIMEType; - path?: string; - } | null; - thumbnail: { - height: number; - width: number; - url?: string; - contentType: MIME.MIMEType; - path?: string; - } | null; } // UI-focused functions From bc86dd6c1f14812595a94f3391f7b0af0e002ebb Mon Sep 17 00:00:00 2001 From: yougotwill Date: Thu, 15 Aug 2024 10:09:56 +1000 Subject: [PATCH 04/11] feat: generic attachments for messages components --- .../message-content/MessageAttachment.tsx | 59 +++------------ .../MessageGenericAttachment.tsx | 73 +++++++++++++++++++ 2 files changed, 82 insertions(+), 50 deletions(-) create mode 100644 ts/components/conversation/message/message-content/MessageGenericAttachment.tsx diff --git a/ts/components/conversation/message/message-content/MessageAttachment.tsx b/ts/components/conversation/message/message-content/MessageAttachment.tsx index a151da754..aa680f4b8 100644 --- a/ts/components/conversation/message/message-content/MessageAttachment.tsx +++ b/ts/components/conversation/message/message-content/MessageAttachment.tsx @@ -1,4 +1,3 @@ -import classNames from 'classnames'; import { clone } from 'lodash'; import { useCallback } from 'react'; import { useDispatch, useSelector } from 'react-redux'; @@ -16,8 +15,6 @@ import { import { AttachmentType, AttachmentTypeWithPath, - canDisplayImagePreview, - getExtensionForDisplay, hasImage, hasVideoScreenshot, isAudio, @@ -26,12 +23,12 @@ import { } from '../../../../types/Attachment'; import { saveAttachmentToDisk } from '../../../../util/attachmentsUtil'; import { MediaItemType } from '../../../lightbox/LightboxGallery'; -import { Spinner } from '../../../loading'; import { AudioPlayerWithEncryptedFile } from '../../H5AudioPlayer'; import { ImageGrid } from '../../ImageGrid'; import { ClickToTrustSender } from './ClickToTrustSender'; import { MessageHighlighter } from './MessageHighlighter'; import { useIsDetailMessageView } from '../../../../contexts/isDetailViewContext'; +import { MessageGenericAttachment } from './MessageGenericAttachment'; export type MessageAttachmentSelectorProps = Pick< MessageRenderingProps, @@ -62,10 +59,6 @@ const StyledImageGridContainer = styled.div<{ justify-content: ${props => (props.messageDirection === 'incoming' ? 'flex-start' : 'flex-end')}; `; -const StyledGenericAttachmentContainer = styled(MessageHighlighter)<{ selected: boolean }>` - ${props => props.selected && 'box-shadow: var(--drop-shadow);'} -`; - export const MessageAttachment = (props: Props) => { const { messageId, imageBroken, handleImageError, highlight = false } = props; const isDetailView = useIsDetailMessageView(); @@ -130,17 +123,14 @@ export const MessageAttachment = (props: Props) => { } const firstAttachment = attachments[0]; - const displayImage = canDisplayImagePreview(attachments); if (!isTrustedForAttachmentDownload) { return ; } if ( - displayImage && - !imageBroken && - ((isImage(attachments) && hasImage(attachments)) || - (isVideo(attachments) && hasVideoScreenshot(attachments))) + (isImage(attachments) && hasImage(attachments)) || + (isVideo(attachments) && hasVideoScreenshot(attachments)) ) { // we use the carousel in the detail view if (isDetailView) { @@ -152,6 +142,8 @@ export const MessageAttachment = (props: Props) => { @@ -181,48 +173,15 @@ export const MessageAttachment = (props: Props) => { ); } - const { pending, fileName, fileSize, contentType } = firstAttachment; - const extension = getExtensionForDisplay({ contentType, fileName }); return ( - - {pending ? ( -
- -
- ) : ( -
-
- {extension ? ( -
{extension}
- ) : null} -
-
- )} -
-
- {fileName} -
-
- {fileSize} -
-
- + /> ); }; diff --git a/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx b/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx new file mode 100644 index 000000000..a567233e0 --- /dev/null +++ b/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx @@ -0,0 +1,73 @@ +import classNames from 'classnames'; +import styled from 'styled-components'; +import { PropsForAttachment } from '../../../../state/ducks/conversations'; +import { AttachmentTypeWithPath, getExtensionForDisplay } from '../../../../types/Attachment'; +import { Spinner } from '../../../loading'; +import { MessageModelType } from '../../../../models/messageType'; +import { MessageHighlighter } from './MessageHighlighter'; + +const StyledGenericAttachmentContainer = styled(MessageHighlighter)<{ + highlight: boolean; + selected: boolean; +}>` + ${props => props.selected && 'box-shadow: var(--drop-shadow);'} +`; + +export function MessageGenericAttachment({ + attachment, + direction, + highlight, + selected, + onClick, +}: { + attachment: PropsForAttachment | AttachmentTypeWithPath; + highlight: boolean; + selected: boolean; + direction?: MessageModelType; + onClick?: (e: any) => void; +}) { + // TODO add higher level pending or loading states + const { pending, fileName, fileSize, contentType } = attachment; + const extension = getExtensionForDisplay({ contentType, fileName }); + + return ( + + {pending ? ( +
+ +
+ ) : ( +
+
+ {extension ? ( +
{extension}
+ ) : null} +
+
+ )} +
+
+ {fileName} +
+
+ {fileSize} +
+
+ + ); +} From 1f1f5adb484ec322f67f7d77ca77250e4070cf7c Mon Sep 17 00:00:00 2001 From: yougotwill Date: Thu, 15 Aug 2024 17:30:25 +1000 Subject: [PATCH 05/11] fix: align file size to correct side in generic attachment preview --- stylesheets/_modules.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index 8f32136f5..de072a828 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -137,6 +137,7 @@ line-height: 16px; letter-spacing: 0.3px; margin-top: 3px; + text-align: start; white-space: nowrap; } From 45d1791cdf9ae3694ace54bdf1cf79b403cbbbbf Mon Sep 17 00:00:00 2001 From: yougotwill Date: Thu, 15 Aug 2024 17:31:36 +1000 Subject: [PATCH 06/11] feat: updated generic attachment to pass through pending prop --- .../message-content/MessageGenericAttachment.tsx | 12 +++++++----- ts/types/Attachment.ts | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx b/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx index a567233e0..4deaa1c36 100644 --- a/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx +++ b/ts/components/conversation/message/message-content/MessageGenericAttachment.tsx @@ -15,19 +15,21 @@ const StyledGenericAttachmentContainer = styled(MessageHighlighter)<{ export function MessageGenericAttachment({ attachment, - direction, - highlight, + /** comes from the attachment iself or the component if it needs to be decrypted */ + pending, selected, + highlight, + direction, onClick, }: { attachment: PropsForAttachment | AttachmentTypeWithPath; - highlight: boolean; + pending: boolean; selected: boolean; + highlight: boolean; direction?: MessageModelType; onClick?: (e: any) => void; }) { - // TODO add higher level pending or loading states - const { pending, fileName, fileSize, contentType } = attachment; + const { fileName, fileSize, contentType } = attachment; const extension = getExtensionForDisplay({ contentType, fileName }); return ( diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts index aec9057b5..436163e1d 100644 --- a/ts/types/Attachment.ts +++ b/ts/types/Attachment.ts @@ -156,7 +156,7 @@ export function isVideoAttachment(attachment?: AttachmentType): boolean { export function hasVideoScreenshot(attachments?: Array): boolean { const firstAttachment = attachments ? attachments[0] : null; - return Boolean(firstAttachment?.screenshot?.url); + return Boolean(firstAttachment?.screenshot?.url || firstAttachment?.pending); } type DimensionsType = { From 737dbd45c128ccca1fa726acc0b2214e23b4e66d Mon Sep 17 00:00:00 2001 From: yougotwill Date: Tue, 20 Aug 2024 10:57:21 +1000 Subject: [PATCH 07/11] 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. From 340361f2d9d0502d3b0f08e1f8c31474a344a682 Mon Sep 17 00:00:00 2001 From: yougotwill Date: Tue, 20 Aug 2024 11:13:55 +1000 Subject: [PATCH 08/11] fix: cleanup --- ts/components/conversation/Image.tsx | 17 +++++------------ ts/hooks/useEncryptedFileFetch.ts | 16 ++++++---------- 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index bfee52c74..b66c3ca4d 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -13,7 +13,8 @@ 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 + /** undefined if the message is not visible yet, '' if the attachment is broken */ + url: string | undefined; imageBroken?: boolean; height?: number | string; @@ -36,6 +37,7 @@ type Props = { onClickClose?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; onError?: () => void; + /** used for debugging */ timestamp?: number; }; @@ -104,27 +106,18 @@ export const Image = (props: Props) => { 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]); + }, [imageBroken, loading, mounted, onErrorUrlFilterering, pending, url, urlToLoad]); if (mounted && imageBroken) { return ( @@ -220,7 +213,7 @@ export const Image = (props: Props) => { className="module-image__close-button" /> ) : null} - {!pending && playIconOverlay ? ( + {mounted && playIconOverlay ? (
diff --git a/ts/hooks/useEncryptedFileFetch.ts b/ts/hooks/useEncryptedFileFetch.ts index 701143abb..19af3af17 100644 --- a/ts/hooks/useEncryptedFileFetch.ts +++ b/ts/hooks/useEncryptedFileFetch.ts @@ -7,11 +7,13 @@ import { import { perfEnd, perfStart } from '../session/utils/Performance'; export const useEncryptedFileFetch = ( + /** undefined if the message is not visible yet, url is '' if something is broken */ url: string | undefined, contentType: string, isAvatar: boolean, timestamp?: number ) => { + /** undefined if the attachment is not decrypted yet, '' if the attachment fails to decrypt */ const [urlToLoad, setUrlToLoad] = useState(undefined); const [loading, setLoading] = useState(true); @@ -20,10 +22,6 @@ export const useEncryptedFileFetch = ( 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); @@ -34,16 +32,14 @@ export const useEncryptedFileFetch = ( setLoading(true); try { - perfStart(`getDecryptedMediaUrl-${mediaUrl}`); + perfStart(`getDecryptedMediaUrl-${mediaUrl}-${timestamp}`); const decryptedUrl = await getDecryptedMediaUrl(mediaUrl, contentType, isAvatar); - perfEnd(`getDecryptedMediaUrl-${mediaUrl}`, `getDecryptedMediaUrl-${mediaUrl}`); - window.log.debug( - `WIP: [Image] timestamp ${timestamp} decryptedUrl ${decryptedUrl !== '' ? decryptedUrl : 'empty'}` + perfEnd( + `getDecryptedMediaUrl-${mediaUrl}-${timestamp}`, + `getDecryptedMediaUrl-${mediaUrl}-${timestamp}` ); - setUrlToLoad(decryptedUrl); } catch (error) { - window.log.error(`WIP: [Image] timestamp ${timestamp} error ${error}`); setUrlToLoad(''); } finally { setLoading(false); From 1a8d9bc27abb7d5ec4a9d50563bbceef7bb87ea2 Mon Sep 17 00:00:00 2001 From: yougotwill Date: Tue, 20 Aug 2024 11:17:41 +1000 Subject: [PATCH 09/11] fix: simplify loader --- ts/components/conversation/Image.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index b66c3ca4d..33c1ceeec 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -154,7 +154,7 @@ export const Image = (props: Props) => { }} data-attachmentindex={attachmentIndex} > - {!mounted || loading || pending ? ( + {!mounted ? (
Date: Tue, 20 Aug 2024 11:34:44 +1000 Subject: [PATCH 10/11] fix: no need for empty url check in pending init --- ts/components/conversation/Image.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index 33c1ceeec..9257dafff 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -84,7 +84,7 @@ export const Image = (props: Props) => { ); const { caption } = attachment || { caption: null }; - const [pending, setPending] = useState(attachment.pending || !url || true); + const [pending, setPending] = useState(attachment.pending || true); const [mounted, setMounted] = useState( (!loading || !pending) && urlToLoad === undefined ); From 3fd37bbb063d893f4aeb2533da2d220eae9c97c0 Mon Sep 17 00:00:00 2001 From: yougotwill Date: Wed, 21 Aug 2024 16:04:21 +1000 Subject: [PATCH 11/11] feat: use context to pass message id to image component in an attachment --- ts/components/conversation/Image.tsx | 21 ++++++++------- ts/components/conversation/ImageGrid.tsx | 20 +------------- .../message-content/MessageAttachment.tsx | 26 ++++++++++--------- ts/contexts/MessageIdContext.tsx | 14 ++++++++++ 4 files changed, 41 insertions(+), 40 deletions(-) create mode 100644 ts/contexts/MessageIdContext.tsx diff --git a/ts/components/conversation/Image.tsx b/ts/components/conversation/Image.tsx index 9257dafff..05c08f5f9 100644 --- a/ts/components/conversation/Image.tsx +++ b/ts/components/conversation/Image.tsx @@ -6,9 +6,14 @@ import { isNumber } from 'lodash'; import { useDisableDrag } from '../../hooks/useDisableDrag'; 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'; +import { useMessageIdFromContext } from '../../contexts/MessageIdContext'; +import { + useMessageDirection, + useMessageSelected, + useMessageTimestamp, +} from '../../state/selectors'; type Props = { alt: string; @@ -28,17 +33,12 @@ type Props = { playIconOverlay?: boolean; softCorners: boolean; forceSquare?: boolean; - dropShadow?: boolean; attachmentIndex?: number; - direction?: MessageModelType; highlight?: boolean; onClick?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; onClickClose?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; onError?: () => void; - - /** used for debugging */ - timestamp?: number; }; const StyledOverlay = styled.div>` @@ -66,15 +66,18 @@ export const Image = (props: Props) => { playIconOverlay, softCorners, forceSquare, - dropShadow, attachmentIndex, - direction, highlight, url, width: _width, - timestamp, } = props; + const messageId = useMessageIdFromContext(); + const dropShadow = useMessageSelected(messageId); + const direction = useMessageDirection(messageId); + /** used for debugging */ + const timestamp = useMessageTimestamp(messageId); + const disableDrag = useDisableDrag(); const { loading, urlToLoad } = useEncryptedFileFetch( url, diff --git a/ts/components/conversation/ImageGrid.tsx b/ts/components/conversation/ImageGrid.tsx index c5acd0dcf..576765a0e 100644 --- a/ts/components/conversation/ImageGrid.tsx +++ b/ts/components/conversation/ImageGrid.tsx @@ -10,11 +10,6 @@ import { } from '../../types/Attachment'; import { useIsMessageVisible } from '../../contexts/isMessageVisibleContext'; -import { - useMessageDirection, - useMessageSelected, - useMessageTimestamp, -} from '../../state/selectors'; import { THUMBNAIL_SIDE } from '../../types/attachments/VisualAttachment'; import { Image } from './Image'; @@ -24,7 +19,6 @@ type Props = { imageBroken: boolean; highlight: boolean; onClickAttachment?: (attachment: AttachmentTypeWithPath | AttachmentType) => void; - messageId?: string; }; const StyledImageGrid = styled.div<{ flexDirection: 'row' | 'column' }>` @@ -50,16 +44,11 @@ const Row = ( startIndex, onClickAttachment, totalAttachmentsCount, - 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) => { @@ -81,9 +70,6 @@ const Row = ( softCorners={true} darkOverlay={showOverlay} overlayText={showOverlay ? moreMessagesOverlayText : undefined} - dropShadow={selected} - direction={direction} - timestamp={timestamp} /> ); })} @@ -92,7 +78,7 @@ const Row = ( }; export const ImageGrid = (props: Props) => { - const { attachments, imageBroken, highlight, onError, onClickAttachment, messageId } = props; + const { attachments, imageBroken, highlight, onError, onClickAttachment } = props; if (!attachments || !attachments.length) { return null; @@ -110,7 +96,6 @@ export const ImageGrid = (props: Props) => { renderedSize={THUMBNAIL_SIDE} startIndex={0} totalAttachmentsCount={attachments.length} - messageId={messageId} /> ); @@ -129,7 +114,6 @@ export const ImageGrid = (props: Props) => { renderedSize={THUMBNAIL_SIDE} startIndex={0} totalAttachmentsCount={attachments.length} - messageId={messageId} /> ); @@ -149,7 +133,6 @@ export const ImageGrid = (props: Props) => { renderedSize={THUMBNAIL_SIDE} startIndex={0} totalAttachmentsCount={attachments.length} - messageId={messageId} /> @@ -162,7 +145,6 @@ export const ImageGrid = (props: Props) => { renderedSize={columnImageSide} startIndex={1} totalAttachmentsCount={attachments.length} - messageId={messageId} /> diff --git a/ts/components/conversation/message/message-content/MessageAttachment.tsx b/ts/components/conversation/message/message-content/MessageAttachment.tsx index 8472f79d5..00d32b1a5 100644 --- a/ts/components/conversation/message/message-content/MessageAttachment.tsx +++ b/ts/components/conversation/message/message-content/MessageAttachment.tsx @@ -27,6 +27,7 @@ import { ClickToTrustSender } from './ClickToTrustSender'; import { MessageHighlighter } from './MessageHighlighter'; import { useIsDetailMessageView } from '../../../../contexts/isDetailViewContext'; import { MessageGenericAttachment } from './MessageGenericAttachment'; +import { ContextMessageProvider } from '../../../../contexts/MessageIdContext'; export type MessageAttachmentSelectorProps = Pick< MessageRenderingProps, @@ -133,18 +134,19 @@ export const MessageAttachment = (props: Props) => { } return ( - - - - - + + + + + + + ); } diff --git a/ts/contexts/MessageIdContext.tsx b/ts/contexts/MessageIdContext.tsx new file mode 100644 index 000000000..c1f72b155 --- /dev/null +++ b/ts/contexts/MessageIdContext.tsx @@ -0,0 +1,14 @@ +import { createContext, useContext } from 'react'; + +/** + * This React context is used to share deep into a node tree the message ID we are currently rendering. + * This is to avoid passing the prop to all the subtree component + */ +const ContextMessageId = createContext(undefined); + +export const ContextMessageProvider = ContextMessageId.Provider; + +export function useMessageIdFromContext() { + const messageId = useContext(ContextMessageId); + return messageId; +}