[PAID] UX issues with Attachment View in Chat

[akshayasalvi]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Go to chat
2. Click Add Attachment and select a word docx document.
3. Upload the document
4. Now see the preview and try downloading

Expected Result:

• It should've given a better UX for showing the attachment instead of link
• Download shouldn't take you to a new tab and filename should be reader-friendly.

Actual Result:

• UX looks similar to a link than an attachment
  

• Downloading the file opens a new tab and downloads the file with a long random string
  

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:

View all open jobs on Upwork

[MelvinBot]

Triggered auto assignment to @flaviadefaria (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[akshayasalvi]

Proposed Solution

We can create a custom view for Attachment view in the chatlist. It'll have the filename as well as the download button. A sample UI would look similar to this.

We'll have to update ReportActionItemFragment, case 'COMMENT': section.

	if(fragment.text === '[Attachment]') {
		const fileName = extractFileNameFromLink(fragment.html);
		return (
			<AttachmentView fileName={fileName} rightElement={
				<Pressable onPress={() => fileDownload(fragment.html, fileName) }>
					<Icon src={download} />
				</Pressable>

			} />
		);

	} else if(fragment.html !== fragment.text) {
		// existing code...
	}

Modify the AttachmentView

	<View
            style={styles.defaultAttachmentView}
        >
            <View style={styles.mr2}>
                <Icon src={Paperclip} />
            </View>
            <Text style={[styles.textStrong]}>{props.file && props.file.name}</Text>
            {
            	props.rightElement && ({props.rightElement})
            }
        </View>

Add a param in fileDownload.js,

	export default function fileDownload(url, downloadFileName) {

		// existing code

		const attachmentName = downloadFileName ?? getAttachmentName(url);
		link.setAttribute(
            'download',
            attachmentName
        )

[MelvinBot]

Triggered auto assignment to @robertjchen (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[MelvinBot]

Triggered auto assignment to @mallenexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[mallenexpensify]

Thank's for bringing up the issue @akshayasalvi , I do agree that the text that looks like a hyperlink looks like a link. @Expensify/design can y'all take a look and provide feedback on the proposed update from

to

[michelle-thompson]

Yeah, I think that looks great!

[MelvinBot]

Triggered auto assignment to @thienlnam (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[mallenexpensify]

Job created in Upwork - https://www.upwork.com/jobs/~012a60478f2a5dfa9a
@thienlnam can you please review @akshayasalvi 's proposal above?

[thienlnam]

@akshayasalvi Thanks for the clear proposal! After taking a look here are a couple things I've noticed

We'll have to update ReportActionItemFragment, case 'COMMENT': section.

We should be handling this in BaseRenderHTML, the 'attachment' in this case is just an anchor tag. Can you outline a solution that would exist here?

App/src/components/RenderHTML/BaseRenderHTML.js

Lines 67 to 90 in 88593d4

	function AnchorRenderer({tnode, key, style}) {
	const htmlAttribs = tnode.attributes;
	// An auth token is needed to download Expensify chat attachments
	const isAttachment = Boolean(htmlAttribs['data-expensify-source']);
	return (
	<AnchorForCommentsOnly
	href={htmlAttribs.href}
	isAuthTokenRequired={isAttachment}
	// Unless otherwise specified open all links in
	// a new window. On Desktop this means that we will
	// skip the default Save As... download prompt
	// and defer to whatever browser the user has.
	// eslint-disable-next-line react/jsx-props-no-multi-spaces
	target={htmlAttribs.target || '_blank'}
	rel={htmlAttribs.rel || 'noopener noreferrer'}
	style={style}
	key={key}
	>
	<TNodeChildrenRenderer tnode={tnode} />
	</AnchorForCommentsOnly>
	);
	}

Modify the AttachmentView

Instead of adding a prop for rightElement, we should instead just include the file download icon and instead choose to conditionally render it with shouldShowDownloadIcon or something similar

Add a param in fileDownload.js,

These changes look fine for web but how will you handle this for native devices?

[akshayasalvi]

@thienlnam
I couldn't find BaseRenderHTML in the main branch. Is it a part of the working branch? I did find https://github.com/Expensify/App/blob/main/src/components/RenderHTML.js in the main branch.

Then the same changes in terms of the UI can be pulled into:
https://github.com/Expensify/App/blob/88593d4f00f38bf97c70da9dfb97b80bb9b381a4/src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js component.

These changes look fine for web but how will you handle this for native devices?

I was relying on the existing functionality of download attachments. But it seems the current App download doesn't work (atleast in the iPhone). I tried downloading an image as well as a pdf. We can use react-native-fs to add a download logic for all attachments. But do you want to keep this in the same task? This was more of the UX of the attachment view.

[akshayasalvi]

@thienlnam Is there a way to get the filename in the BaseAnchorForCommentsOnly. I've checked that it just sends HTML to the renderHTML. In Text the children, it loads an element and I don't have a way to get just the filename.

<Text
          ref={el => linkRef = el}
          style={StyleSheet.flatten(style)}
          accessibilityRole="link"
          href={href}
          hrefAttrs={{rel, target}}
         // eslint-disable-next-line react/jsx-props-no-spreading
          {...props}  
      >
      {children}
</Text>

If there isn't a direct way to get the filename then I'll figure out a way to pass it to props or something. This is how it looks right now with the UX changes.

[thienlnam]

You'd probably have to pass it down via props here, likely by extracting it from htmlAttribs. If we need to add an attribute to the html element to include the name let me know

[akshayasalvi]

Honestly, passing a name would be better. Not only for the documents but also for image. I can then just pick that and display it with htmlAttribs

[thienlnam]

Honestly, passing a name would be better. Not only for the documents but also for image. I can then just pick that and display it with htmlAttribs

Can you clarify this? Do you need a name to be passed to the html element which would show up in htmlAttribs?

[akshayasalvi]

@thienlnam I've managed to find the filename from the TextNode. Please check the draft PR. If that doesn't work well then I would required the name to be passed in the HTML such as data-filename, which I can use to pass as prop.

[thienlnam]

Sweet, @akshayasalvi Could you please apply for the upwork job here so we can get you hired before continuing work on the PR?

[akshayasalvi]

I've applied on Upwork

[mallenexpensify]

Hired @akshayasalvi in Upwork. The bonus for reporting the issue will be added at the end upon payment for the job.

[akshayasalvi]

Thank you. @thienlnam Can you review the PR now?

[thienlnam]

Just merged the PR this morning, will be going on the next staging deploy

[botify]

🚀 Deployed to staging by @sketchydroide in version: 1.0.88-3 🚀

platform	result
🤖 android 🤖	cancelled 🔪
🖥 desktop 🖥	cancelled 🔪
🍎 iOS 🍎	cancelled 🔪
🕸 web 🕸	success ✅

[botify]

🚀 Deployed to production by @roryabraham in version: 1.0.88-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

[botify]

🚀 Deployed to production by @roryabraham in version: 1.0.90-2 🚀

platform	result
🤖 android 🤖	success ✅
🖥 desktop 🖥	success ✅
🍎 iOS 🍎	failure ❌
🕸 web 🕸	success ✅

[mallenexpensify]

Paid @akshayasalvi in Upwork, included the $250 bonus for reporting this issue.
Paid ahead of 9/8 because issue was created before we standardized on paying 7 days after production. closing because code has been deployed to production.