Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add captions to m.file, m.image, m.video, and m.audio #2059

Open
wants to merge 8 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 104 additions & 68 deletions src/app/components/RenderMessageContent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { ImageViewer } from './image-viewer';
import { PdfViewer } from './Pdf-viewer';
import { TextViewer } from './text-viewer';
import { testMatrixTo } from '../plugins/matrix-to';
import {IImageContent} from "../../types/matrix/common";

type RenderMessageContentProps = {
displayName: string;
Expand Down Expand Up @@ -67,38 +68,62 @@ export function RenderMessageContent({
</UrlPreviewHolder>
);
};
const content: IImageContent = getContent();
const renderCaption = content.filename && content.filename !== content.body;
let renderedCaption: React.ReactNode = null;
if(renderCaption) {
nexy7574 marked this conversation as resolved.
Show resolved Hide resolved
renderedCaption = (
<MNotice
nexy7574 marked this conversation as resolved.
Show resolved Hide resolved
edited={edited}
content={getContent()}
renderBody={(props) => (
<RenderBody
{...props}
highlightRegex={highlightRegex}
htmlReactParserOptions={htmlReactParserOptions}
linkifyOpts={linkifyOpts}
/>
)}
renderUrlsPreview={urlPreview ? renderUrlsPreview : undefined}
/>
)
}

const renderFile = () => (
<MFile
content={getContent()}
renderFileContent={({ body, mimeType, info, encInfo, url }) => (
<FileContent
body={body}
mimeType={mimeType}
renderAsPdfFile={() => (
<ReadPdfFile
body={body}
mimeType={mimeType}
url={url}
encInfo={encInfo}
renderViewer={(p) => <PdfViewer {...p} />}
/>
)}
renderAsTextFile={() => (
<ReadTextFile
<>
<MFile
content={getContent()}
renderFileContent={({ body, mimeType, info, encInfo, url }) => (
<FileContent
body={body}
mimeType={mimeType}
url={url}
encInfo={encInfo}
renderViewer={(p) => <TextViewer {...p} />}
/>
)}
>
<DownloadFile body={body} mimeType={mimeType} url={url} encInfo={encInfo} info={info} />
</FileContent>
)}
outlined={outlineAttachment}
/>
renderAsPdfFile={() => (
<ReadPdfFile
body={body}
mimeType={mimeType}
url={url}
encInfo={encInfo}
renderViewer={(p) => <PdfViewer {...p} />}
/>
)}
renderAsTextFile={() => (
<ReadTextFile
body={body}
mimeType={mimeType}
url={url}
encInfo={encInfo}
renderViewer={(p) => <TextViewer {...p} />}
/>
)}
>
<DownloadFile body={body} mimeType={mimeType} url={url} encInfo={encInfo} info={info} />
</FileContent>

)}
outlined={outlineAttachment}
/>
{renderedCaption}
</>
);

if (msgType === MsgType.Text) {
Expand Down Expand Up @@ -158,63 +183,74 @@ export function RenderMessageContent({

if (msgType === MsgType.Image) {
return (
<MImage
content={getContent()}
renderImageContent={(props) => (
<ImageContent
{...props}
autoPlay={mediaAutoLoad}
renderImage={(p) => <Image {...p} loading="lazy" />}
renderViewer={(p) => <ImageViewer {...p} />}
/>
)}
outlined={outlineAttachment}
/>
<>
<MImage
content={getContent()}
renderImageContent={(props) => (
<ImageContent
{...props}
autoPlay={mediaAutoLoad}
renderImage={(p) => <Image {...p} loading="lazy" />}
renderViewer={(p) => <ImageViewer {...p} />}
/>
)}
outlined={outlineAttachment}
/>
{renderedCaption}
</>
);
}

if (msgType === MsgType.Video) {
return (
<MVideo
content={getContent()}
renderAsFile={renderFile}
renderVideoContent={({ body, info, mimeType, url, encInfo }) => (
<VideoContent
body={body}
info={info}
mimeType={mimeType}
url={url}
encInfo={encInfo}
renderThumbnail={
mediaAutoLoad
? () => (
<>
<MVideo
content={getContent()}
renderAsFile={renderFile}
renderVideoContent={({ body, info, mimeType, url, encInfo }) => (
<VideoContent
body={body}
info={info}
mimeType={mimeType}
url={url}
encInfo={encInfo}
renderThumbnail={
mediaAutoLoad
? () => (
<ThumbnailContent
info={info}
renderImage={(src) => (
<Image alt={body} title={body} src={src} loading="lazy" />
)}
/>
)
: undefined
}
renderVideo={(p) => <Video {...p} />}
/>
)}
outlined={outlineAttachment}
/>
: undefined
}
renderVideo={(p) => <Video {...p} />}
/>
)}
outlined={outlineAttachment}
/>
{renderedCaption}
</>

);
}

if (msgType === MsgType.Audio) {
return (
<MAudio
content={getContent()}
renderAsFile={renderFile}
renderAudioContent={(props) => (
<AudioContent {...props} renderMediaControl={(p) => <MediaControl {...p} />} />
)}
outlined={outlineAttachment}
/>
<>
<MAudio
content={getContent()}
renderAsFile={renderFile}
renderAudioContent={(props) => (
<AudioContent {...props} renderMediaControl={(p) => <MediaControl {...p} />} />
)}
outlined={outlineAttachment}
/>
{renderedCaption}
</>

);
}

Expand Down
7 changes: 4 additions & 3 deletions src/app/components/message/MsgTypeRenderers.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ export function MNotice({ edited, content, renderBody, renderUrlsPreview }: MNot

type RenderImageContentProps = {
body: string;
filename?: string;
info?: IImageInfo & IThumbnailContent;
mimeType?: string;
url: string;
Expand Down Expand Up @@ -282,7 +283,7 @@ export function MAudio({ content, renderAsFile, renderAudioContent, outlined }:
return (
<Attachment outlined={outlined}>
<AttachmentHeader>
<FileHeader body={content.body ?? 'Audio'} mimeType={safeMimeType} />
<FileHeader body={content.filename ?? content.body ?? 'Audio'} mimeType={safeMimeType} />
</AttachmentHeader>
<AttachmentBox>
<AttachmentContent>
Expand Down Expand Up @@ -322,14 +323,14 @@ export function MFile({ content, renderFileContent, outlined }: MFileProps) {
<Attachment outlined={outlined}>
<AttachmentHeader>
<FileHeader
body={content.body ?? 'Unnamed File'}
body={content.filename ?? content.body ?? 'Unnamed File'}
mimeType={fileInfo?.mimetype ?? FALLBACK_MIMETYPE}
/>
</AttachmentHeader>
<AttachmentBox>
<AttachmentContent>
{renderFileContent({
body: content.body ?? 'File',
body: content.filename ?? content.body ?? 'File',
info: fileInfo ?? {},
mimeType: fileInfo?.mimetype ?? FALLBACK_MIMETYPE,
url: mxcUrl,
Expand Down
13 changes: 8 additions & 5 deletions src/app/features/room/RoomInput.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -227,17 +227,18 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
const contentsPromises = uploads.map(async (upload) => {
const fileItem = selectedFiles.find((f) => f.file === upload.file);
if (!fileItem) throw new Error('Broken upload');
const caption = toPlainText(editor.children).trim() || undefined;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like following changes add same caption to multiple uploaded file. Do we really want that? Can't we just send it as separate in that case?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it not be better to attach the caption to the last uploaded item, rather than as an entirely separate message?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image I think captions should be added here for each file and message input should not be used for caption as doing so will break the current functionality. For example someone wants to send an image right after writing their TWIM message.

Copy link
Author

@nexy7574 nexy7574 Dec 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As nice as that would be, would it not require adding another message composer, especially since both html and plaintext are meant to be supported? How would that work on mobile?
Apologies for the delay, my hands are quite full until Febuary 😔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah adding full message composer won't be a good idea, but maybe we can add plaintext caption as file rename option, not sure about how that would go. So if we don't want to go with sending caption from main composer yet could we please limit this PR for display only support as that alone would be really helpful.

nexy7574 marked this conversation as resolved.
Show resolved Hide resolved

if (fileItem.file.type.startsWith('image')) {
return getImageMsgContent(mx, fileItem, upload.mxc);
return getImageMsgContent(mx, fileItem, upload.mxc, caption);
}
if (fileItem.file.type.startsWith('video')) {
return getVideoMsgContent(mx, fileItem, upload.mxc);
return getVideoMsgContent(mx, fileItem, upload.mxc, caption);
}
if (fileItem.file.type.startsWith('audio')) {
return getAudioMsgContent(fileItem, upload.mxc);
return getAudioMsgContent(fileItem, upload.mxc, caption);
}
return getFileMsgContent(fileItem, upload.mxc);
return getFileMsgContent(fileItem, upload.mxc, caption);
});
handleCancelUpload(uploads);
const contents = fulfilledPromiseSettledResult(await Promise.allSettled(contentsPromises));
Expand Down Expand Up @@ -318,7 +319,9 @@ export const RoomInput = forwardRef<HTMLDivElement, RoomInputProps>(
content['m.relates_to'].is_falling_back = false;
}
}
mx.sendMessage(roomId, content);
if (!uploadBoardHandlers.current) {
mx.sendMessage(roomId, content);
}
Comment on lines -321 to +331
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It will be a good idea if we can extract the content generation process in separate function so it can be used here and for caption.

resetEditor(editor);
resetEditorHistory(editor);
setReplyDraft(undefined);
Expand Down
21 changes: 13 additions & 8 deletions src/app/features/room/msgContent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,15 +42,17 @@ const generateThumbnailContent = async (
export const getImageMsgContent = async (
mx: MatrixClient,
item: TUploadItem,
mxc: string
mxc: string,
caption?: string
): Promise<IContent> => {
const { file, originalFile, encInfo } = item;
const [imgError, imgEl] = await to(loadImageElement(getImageFileUrl(originalFile)));
if (imgError) console.warn(imgError);

const content: IContent = {
msgtype: MsgType.Image,
body: file.name,
filename: file.name,
body: caption || file.name,
};
if (imgEl) {
const blurHash = encodeBlurHash(imgEl, 512, scaleYDimension(imgEl.width, 512, imgEl.height));
Expand All @@ -74,7 +76,8 @@ export const getImageMsgContent = async (
export const getVideoMsgContent = async (
mx: MatrixClient,
item: TUploadItem,
mxc: string
mxc: string,
caption?: string
): Promise<IContent> => {
const { file, originalFile, encInfo } = item;

Expand All @@ -83,7 +86,8 @@ export const getVideoMsgContent = async (

const content: IContent = {
msgtype: MsgType.Video,
body: file.name,
filename: file.name,
body: caption || file.name,
};
if (videoEl) {
const [thumbError, thumbContent] = await to(
Expand Down Expand Up @@ -118,11 +122,12 @@ export const getVideoMsgContent = async (
return content;
};

export const getAudioMsgContent = (item: TUploadItem, mxc: string): IContent => {
export const getAudioMsgContent = (item: TUploadItem, mxc: string, caption?: string): IContent => {
const { file, encInfo } = item;
const content: IContent = {
msgtype: MsgType.Audio,
body: file.name,
filename: file.name,
body: caption || file.name,
info: {
mimetype: file.type,
size: file.size,
Expand All @@ -139,11 +144,11 @@ export const getAudioMsgContent = (item: TUploadItem, mxc: string): IContent =>
return content;
};

export const getFileMsgContent = (item: TUploadItem, mxc: string): IContent => {
export const getFileMsgContent = (item: TUploadItem, mxc: string, caption?: string): IContent => {
const { file, encInfo } = item;
const content: IContent = {
msgtype: MsgType.File,
body: file.name,
body: caption ?? file.name,
filename: file.name,
info: {
mimetype: file.type,
Expand Down
4 changes: 4 additions & 0 deletions src/types/matrix/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type IThumbnailContent = {
export type IImageContent = {
msgtype: MsgType.Image;
body?: string;
filename?: string;
url?: string;
info?: IImageInfo & IThumbnailContent;
file?: IEncryptedFile;
Expand All @@ -51,6 +52,7 @@ export type IImageContent = {
export type IVideoContent = {
msgtype: MsgType.Video;
body?: string;
filename?: string;
url?: string;
info?: IVideoInfo & IThumbnailContent;
file?: IEncryptedFile;
Expand All @@ -59,6 +61,7 @@ export type IVideoContent = {
export type IAudioContent = {
msgtype: MsgType.Audio;
body?: string;
filename?: string;
url?: string;
info?: IAudioInfo;
file?: IEncryptedFile;
Expand All @@ -67,6 +70,7 @@ export type IAudioContent = {
export type IFileContent = {
msgtype: MsgType.File;
body?: string;
filename?: string;
url?: string;
info?: IFileInfo & IThumbnailContent;
file?: IEncryptedFile;
Expand Down