[HOLD for payment 2022-06-13] [$250] mWeb - Chat - Drawer does not close after tapping download file

[kbecciv]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Action Performed:

1. Go https://staging.new.expensify.com/
2. Log in with any account
3. Find the chat where you can download the file
4. Hold the finger on any type of file (mp4, png., mov)
5. Tap download

Expected Result:

Drawer is closing after tapping download file.

Actual Result:

Drawer not closing after tapping download file. Drawer closes for other options but not for the download one

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Mobile Web

Version Number: 1.1.57.12

Reproducible in staging?: Yes

Reproducible in production?: No 9new feature)

Email or phone of affected tester (no customers): any

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20220511-100037_Chrome.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

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

[sobitneupane]

Proposal
This issue is occurring in web as well. Addition of following lines of code should solve the issue.

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 46 to 54 in a2f8cf9

	onPress: (closePopover, {reportAction}) => {
	const message = _.last(lodashGet(reportAction, 'message', [{}]));
	const html = lodashGet(message, 'html', '');
	const attachmentDetails = getAttachmentDetails(html);
	const {originalFileName} = attachmentDetails;
	let {sourceURL} = attachmentDetails;
	sourceURL = addEncryptedAuthTokenToURL(sourceURL);
	fileDownload(sourceURL, originalFileName);
	},

        onPress: (closePopover, {reportAction}) => {
            const message = _.last(lodashGet(reportAction, 'message', [{}]));
            const html = lodashGet(message, 'html', '');
            const attachmentDetails = getAttachmentDetails(html);
            const {originalFileName} = attachmentDetails;
            let {sourceURL} = attachmentDetails;
            sourceURL = addEncryptedAuthTokenToURL(sourceURL);
            fileDownload(sourceURL, originalFileName);
+          if (closePopover) {
+              hideContextMenu(true, ReportActionComposeFocusManager.focus);
+          }
        },

[melvin-bot]

@yuwenmemon Huh... This is 4 days overdue. Who can take care of this?

[melvin-bot]

@yuwenmemon Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

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

[stephanieelliott]

Posted to Upwork: https://www.upwork.com/jobs/~01a4a5d881f891960a

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @rushatgabhane (Exported)

[melvin-bot]

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

[stephanieelliott]

Heads up, @neil-marcellini and @rushatgabhane we have a proposal for this already here.

[jayeshmangwani]

Proposal:

we can call hideContextMenu on fileDownload promise is finally either fulfilled or rejected,

App/src/pages/home/report/ContextMenu/ContextMenuActions.js

Lines 46 to 54 in 56213d0

	onPress: (closePopover, {reportAction}) => {
	const message = _.last(lodashGet(reportAction, 'message', [{}]));
	const html = lodashGet(message, 'html', '');
	const attachmentDetails = getAttachmentDetails(html);
	const {originalFileName} = attachmentDetails;
	let {sourceURL} = attachmentDetails;
	sourceURL = addEncryptedAuthTokenToURL(sourceURL);
	fileDownload(sourceURL, originalFileName);
	},

onPress: (closePopover, {reportAction}) => { 
     const message = _.last(lodashGet(reportAction, 'message', [{}])); 
     const html = lodashGet(message, 'html', ''); 
     const attachmentDetails = getAttachmentDetails(html); 
     const {originalFileName} = attachmentDetails; 
     let {sourceURL} = attachmentDetails; 
     sourceURL = addEncryptedAuthTokenToURL(sourceURL); 
-    fileDownload(sourceURL, originalFileName);
+    fileDownload(sourceURL, originalFileName)
+                .finally(()=> hideContextMenu(true, ReportActionComposeFocusManager.focus));

[rushatgabhane]

🎀 👀 🎀 C+ reviewed
@yuwenmemon I like @sobitneupane's proposal. I don't see why we should wait for the promise to be resolved. We prefer optimistic UI changes.

P. S. @sobitneupane do we really need to check this condition? if (closePopover). Please make sure to explain why this is or isn't required in the PR. Thanks

[neil-marcellini]

I agree with @rushatgabhane that @sobitneupane has a good proposal. Personally I think we should check if (closePopover), but I'm also curious to hear your explanation.

[sobitneupane]

closePopover is false only for MiniReportActionContextMenu which remains active as long as mouse is hovered over the message.

With the condition check, we are avoiding unnecessary call of hideContextMenu as MiniReportActionContextMenu remains active as long as message is being hovered.

[neil-marcellini]

@sobitneupane Thanks for your explanation. I will assign you to this issue and I look forward to reviewing your PR.

[melvin-bot]

📣 @sobitneupane You have been assigned to this job by @neil-marcellini!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[stephanieelliott]

Hey @sobitneupane, can you apply to the job on Upwork when you get a chance? https://www.upwork.com/jobs/~01a4a5d881f891960a

[sobitneupane]

Hey @sobitneupane, can you apply to the job on Upwork when you get a chance? https://www.upwork.com/jobs/~01a4a5d881f891960a

Applied.

[neil-marcellini]

PR is under review now.

[stephanieelliott]

PR is on staging

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.71-2 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Close Drawer or popup after pressing Download #9212

If no regressions arise, payment will be issued on 2022-06-13. 🎊

[stephanieelliott]

All paid, thank you!