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

New Attachment Upload UX with Real-Time Previews #44889

Merged
merged 27 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
bf4c208
ReportUtils: simplify optimistic comment constructor logic
kidroca Jun 10, 2024
ccfa861
ReportUtils: extract attachment HTML generation to a separate function
kidroca Jun 10, 2024
c7fdc5a
ReportUtils: rename getAttachmentHtml to getUploadingAttachmentHtml
kidroca Jun 10, 2024
f5fcb32
Enhance attachment handling in getUploadingAttachmentHtml function
kidroca Jun 21, 2024
605990b
fix: prevent error when custom attachment source attribute is undefined
kidroca Jun 21, 2024
b2b1544
ReportUtils: improve attachment HTML generation logic
kidroca Jun 21, 2024
5d8d376
VideoRenderer: correct video source URL resolution logic
kidroca Jun 21, 2024
cce5e7e
ReportUtils: improve attachment handling for non-media files
kidroca Jul 5, 2024
feed1af
ReportUtils: prevent extra line breaks when comment text is empty
kidroca Jul 5, 2024
75756e5
ReportUtils: lint fix
kidroca Jul 8, 2024
d017246
ReportUtils: fix type errors
kidroca Jul 8, 2024
72077d4
CONST: add optimistic source attribute for attachments
kidroca Jul 10, 2024
0e34971
libs/isReportMessageAttachment: improve attachment message check logi…
kidroca Jul 10, 2024
556f795
AttachmentCommentFragment: update attachment uploading detection
kidroca Jul 10, 2024
40d07c4
ContextMenuActions: update attachment detection logic
kidroca Jul 10, 2024
b7a0608
ContextMenuActions: prettier fix
kidroca Jul 12, 2024
d28d2e0
isReportMessageAttachment: Update to pass unit tests
kidroca Jul 12, 2024
27ca9a8
Remove `uploading-attachment` custom model from BaseHTMLEngineProvider
kidroca Jul 12, 2024
feeb9c4
ReportUtils: Enhance data attribute generation
kidroca Jul 16, 2024
2903070
Merge branch 'main' into kidroca/feat/attachment-upload-ux
kidroca Jul 19, 2024
58c4237
Merge branch 'main' into kidroca/feat/attachment-upload-ux
kidroca Jul 26, 2024
715ae21
🐛 fix(AnchorForAttachmentsOnly): update download icon visibility logic
kidroca Jul 26, 2024
4008f07
🐛 fix(components/AnchorForAttachmentsOnly): add sourceID check to dow…
kidroca Jul 29, 2024
cab7749
♻️ refactor(components/Attachments): improve attachment file name ext…
kidroca Jul 29, 2024
955009b
🐛 fix(components/AttachmentModal): prevent downloading local file sou…
kidroca Jul 29, 2024
c147141
🐛 fix(report/ContextMenu): hide download option for uploading files
kidroca Jul 30, 2024
c2c7ec9
🐛 fix(libs/ReportUtils): revert translationKey assignment for attachm…
kidroca Jul 30, 2024
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
3 changes: 1 addition & 2 deletions src/CONST.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1189,9 +1189,8 @@ const CONST = {
YOUR_LOCATION_TEXT: 'Your Location',

ATTACHMENT_MESSAGE_TEXT: '[Attachment]',
// This is a placeholder for attachment which is uploading
ATTACHMENT_UPLOADING_MESSAGE_HTML: 'Uploading attachment...',
ATTACHMENT_SOURCE_ATTRIBUTE: 'data-expensify-source',
ATTACHMENT_OPTIMISTIC_SOURCE_ATTRIBUTE: 'data-optimistic-src',
ATTACHMENT_PREVIEW_ATTRIBUTE: 'src',
ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE: 'data-name',
ATTACHMENT_LOCAL_URL_PREFIX: ['blob:', 'file:'],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ function BaseAnchorForAttachmentsOnly({style, source = '', displayName = '', dow
<PressableWithoutFeedback
style={[style, isOffline && styles.cursorDefault]}
onPress={() => {
if (isDownloading || isOffline) {
if (isDownloading || isOffline || !sourceID) {
return;
}
Download.setDownload(sourceID, true);
Expand All @@ -63,7 +63,7 @@ function BaseAnchorForAttachmentsOnly({style, source = '', displayName = '', dow
<AttachmentView
source={sourceURLWithAuth}
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we introduced local file for preview attachments, we also need to check here whether we should not add an encrypted token for the local file.
More details: #54640 (comment)

file={{name: displayName}}
shouldShowDownloadIcon={!isOffline}
shouldShowDownloadIcon={!!sourceID && !isOffline}
shouldShowLoadingSpinnerIcon={isDownloading}
isUsedAsChatAttachment
/>
Expand Down
7 changes: 5 additions & 2 deletions src/components/AttachmentModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,8 @@ function AttachmentModal({
const {translate} = useLocalize();
const {isOffline} = useNetwork();

const isLocalSource = typeof sourceState === 'string' && /^file:|^blob:/.test(sourceState);

useEffect(() => {
setFile(originalFileName ? {name: originalFileName} : undefined);
}, [originalFileName]);
Expand Down Expand Up @@ -342,6 +344,7 @@ function AttachmentModal({
updatedFile = new File([updatedFile], cleanName, {type: updatedFile.type});
}
const inputSource = URL.createObjectURL(updatedFile);
updatedFile.uri = inputSource;
const inputModalType = getModalType(inputSource, updatedFile);
setIsModalOpen(true);
setSourceState(inputSource);
Expand Down Expand Up @@ -412,7 +415,7 @@ function AttachmentModal({
},
});
}
if (!isOffline && allowDownload) {
if (!isOffline && allowDownload && !isLocalSource) {
menuItems.push({
icon: Expensicons.Download,
text: translate('common.download'),
Expand All @@ -439,7 +442,7 @@ function AttachmentModal({
let shouldShowThreeDotsButton = false;
if (!isEmptyObject(report)) {
headerTitleNew = translate(isReceiptAttachment ? 'common.receipt' : 'common.attachment');
shouldShowDownloadButton = allowDownload && isDownloadButtonReadyToBeShown && !shouldShowNotFoundPage && !isReceiptAttachment && !isOffline;
shouldShowDownloadButton = allowDownload && isDownloadButtonReadyToBeShown && !shouldShowNotFoundPage && !isReceiptAttachment && !isOffline && !isLocalSource;
shouldShowThreeDotsButton = isReceiptAttachment && isModalOpen && threeDotsMenuItems.length !== 0;
}
const context = useMemo(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,11 @@ function extractAttachments(
}

uniqueSources.add(source);
const splittedUrl = attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE].split('/');
const fileName = attribs[CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE] || FileUtils.getFileName(`${source}`);
attachments.unshift({
source: tryResolveUrlFromApiRoot(attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]),
isAuthTokenRequired: !!attribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE],
file: {name: splittedUrl[splittedUrl.length - 1]},
file: {name: fileName},
duration: Number(attribs[CONST.ATTACHMENT_DURATION_ATTRIBUTE]),
isReceipt: false,
hasBeenFlagged: false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,13 +82,8 @@ function BaseHTMLEngineProvider({textSelectable = false, children, enableExperim
mixedUAStyles: {...styles.textSupporting, ...styles.textLineThrough},
contentModel: HTMLContentModel.textual,
}),
'uploading-attachment': HTMLElementModel.fromCustomModel({
tagName: 'uploading-attachment',
mixedUAStyles: {...styles.mt4},
contentModel: HTMLContentModel.block,
}),
}),
[styles.formError, styles.mb0, styles.colorMuted, styles.textLabelSupporting, styles.lh16, styles.textSupporting, styles.textLineThrough, styles.mt4, styles.mutedNormalTextLabel],
[styles.formError, styles.mb0, styles.colorMuted, styles.textLabelSupporting, styles.lh16, styles.textSupporting, styles.textLineThrough, styles.mutedNormalTextLabel],
);
/* eslint-enable @typescript-eslint/naming-convention */

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ type VideoRendererProps = CustomRendererProps<TBlock> & {

function VideoRenderer({tnode, key}: VideoRendererProps) {
const htmlAttribs = tnode.attributes;
const attrHref = htmlAttribs.href || htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] || '';
const attrHref = htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] || htmlAttribs.src || htmlAttribs.href || '';
const sourceURL = tryResolveUrlFromApiRoot(attrHref);
const fileName = FileUtils.getFileName(`${sourceURL}`);
const thumbnailUrl = tryResolveUrlFromApiRoot(htmlAttribs[CONST.ATTACHMENT_THUMBNAIL_URL_ATTRIBUTE]);
Expand Down
53 changes: 35 additions & 18 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3756,6 +3756,33 @@ function getParsedComment(text: string, parsingDetails?: ParsingDetails): string
: lodashEscape(text);
}

function getUploadingAttachmentHtml(file?: FileObject): string {
if (!file || typeof file.uri !== 'string') {
return '';
}

const dataAttributes = [
`${CONST.ATTACHMENT_OPTIMISTIC_SOURCE_ATTRIBUTE}="${file.uri}"`,
`${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="${file.uri}"`,
`${CONST.ATTACHMENT_ORIGINAL_FILENAME_ATTRIBUTE}="${file.name}"`,
'width' in file && `${CONST.ATTACHMENT_THUMBNAIL_WIDTH_ATTRIBUTE}="${file.width}"`,
'height' in file && `${CONST.ATTACHMENT_THUMBNAIL_HEIGHT_ATTRIBUTE}="${file.height}"`,
]
.filter((x) => !!x)
.join(' ');

// file.type is a known mime type like image/png, image/jpeg, video/mp4 etc.
if (file.type?.startsWith('image')) {
return `<img src="${file.uri}" alt="${file.name}" ${dataAttributes} />`;
}
if (file.type?.startsWith('video')) {
return `<video src="${file.uri}" ${dataAttributes}>${file.name}</video>`;
}

// For all other types, we present a generic download link
return `<a href="${file.uri}" ${dataAttributes}>${file.name}</a>`;
}

function getReportDescriptionText(report: Report): string {
if (!report.description) {
return '';
Expand All @@ -3781,24 +3808,14 @@ function buildOptimisticAddCommentReportAction(
reportID?: string,
): OptimisticReportAction {
const commentText = getParsedComment(text ?? '', {shouldEscapeText, reportID});
const isAttachmentOnly = file && !text;
const isTextOnly = text && !file;

let htmlForNewComment;
let textForNewComment;
if (isAttachmentOnly) {
htmlForNewComment = CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML;
textForNewComment = CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML;
} else if (isTextOnly) {
htmlForNewComment = commentText;
textForNewComment = Parser.htmlToText(htmlForNewComment);
} else {
htmlForNewComment = `${commentText}<uploading-attachment>${CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML}</uploading-attachment>`;
textForNewComment = `${Parser.htmlToText(commentText)}\n${CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML}`;
}
const attachmentHtml = getUploadingAttachmentHtml(file);

const htmlForNewComment = `${commentText}${commentText && attachmentHtml ? '<br /><br />' : ''}${attachmentHtml}`;
const textForNewComment = Parser.htmlToText(htmlForNewComment);

const isAttachmentOnly = file && !text;
const isAttachmentWithText = !!text && file !== undefined;
const accountID = actorAccountID ?? currentUserAccountID;
const accountID = actorAccountID ?? currentUserAccountID ?? -1;

// Remove HTML from text when applying optimistic offline comment
return {
Expand All @@ -3810,12 +3827,12 @@ function buildOptimisticAddCommentReportAction(
person: [
{
style: 'strong',
text: allPersonalDetails?.[accountID ?? -1]?.displayName ?? currentUserEmail,
text: allPersonalDetails?.[accountID]?.displayName ?? currentUserEmail,
type: 'TEXT',
},
],
automatic: false,
avatar: allPersonalDetails?.[accountID ?? -1]?.avatar,
avatar: allPersonalDetails?.[accountID]?.avatar,
created: DateUtils.getDBTimeWithSkew(Date.now() + createdOffset),
message: [
{
Expand Down
4 changes: 2 additions & 2 deletions src/libs/isReportMessageAttachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const attachmentRegex = new RegExp(` ${CONST.ATTACHMENT_SOURCE_ATTRIBUTE}="(.*)"
* Check whether a report action is Attachment or not.
* Ignore messages containing [Attachment] as the main content. Attachments are actions with only text as [Attachment].
*
* @param reportActionMessage report action's message as text, html and translationKey
* @param message report action's message as text, html and translationKey
*/
export default function isReportMessageAttachment(message: Message | undefined): boolean {
if (!message?.text || !message.html) {
Expand All @@ -19,7 +19,7 @@ export default function isReportMessageAttachment(message: Message | undefined):
return message.text === CONST.ATTACHMENT_MESSAGE_TEXT && message.translationKey === CONST.TRANSLATION_KEYS.ATTACHMENT;
}

const hasAttachmentHtml = message.html === CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML || attachmentRegex.test(message.html);
const hasAttachmentHtml = attachmentRegex.test(message.html);

if (!hasAttachmentHtml) {
return false;
Expand Down
7 changes: 3 additions & 4 deletions src/pages/home/report/ContextMenu/ContextMenuActions.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -515,10 +515,9 @@ const ContextMenuActions: ContextMenuAction[] = [
successIcon: Expensicons.Download,
shouldShow: (type, reportAction, isArchivedRoom, betas, menuTarget, isChronosReport, reportID, isPinnedChat, isUnreadChat, isOffline): reportAction is ReportAction => {
const isAttachment = ReportActionsUtils.isReportActionAttachment(reportAction);
const messageHtml = getActionHtml(reportAction);
return (
isAttachment && messageHtml !== CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML && !!reportAction?.reportActionID && !ReportActionsUtils.isMessageDeleted(reportAction) && !isOffline
);
const html = getActionHtml(reportAction);
const isUploading = html.includes(CONST.ATTACHMENT_OPTIMISTIC_SOURCE_ATTRIBUTE);
return isAttachment && !isUploading && !!reportAction?.reportActionID && !ReportActionsUtils.isMessageDeleted(reportAction) && !isOffline;
},
onPress: (closePopover, {reportAction}) => {
const html = getActionHtml(reportAction);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ type AttachmentCommentFragmentProps = {

function AttachmentCommentFragment({addExtraMargin, html, source, styleAsDeleted}: AttachmentCommentFragmentProps) {
const styles = useThemeStyles();
const isUploading = html === CONST.ATTACHMENT_UPLOADING_MESSAGE_HTML;
const isUploading = html.includes(CONST.ATTACHMENT_OPTIMISTIC_SOURCE_ATTRIBUTE);
const htmlContent = styleAsDeleted && isUploading ? `<del>${html}</del>` : html;

return (
Expand Down
Loading