Skip to content

Download attachments in the chat#1585

Merged
bondydaa merged 15 commits intoExpensify:masterfrom
SameeraMadushan:sameera-download-image-from-chat
Mar 3, 2021
Merged

Download attachments in the chat#1585
bondydaa merged 15 commits intoExpensify:masterfrom
SameeraMadushan:sameera-download-image-from-chat

Conversation

@SameeraMadushan
Copy link
Copy Markdown
Contributor

@SameeraMadushan SameeraMadushan commented Feb 27, 2021

Details

This feature contains download functionality for any type of attachments (images, pdf)

The file download function is a reusable function that is laid in the src/libs/fileDownload

  • iOS and Android follows index.native.js that is developed with rn-fetch-blob
  • Web, Desktop, and mobile web follows index.js that is developed with fetch and blob
  • Filename will auto-generated with a timestamp

Fixed Issues

Fixes #1040

Tests

iOS

  • UIFileSharingEnabled and LSSupportsOpeningDocumentsInPlace enabled in info.plist to save downloaded files to document folder.
  • FIles can be accessed through Files app on the iOS device
  • Separate folder will create for attachments (Expensify.cash)
  • Since multiple types of attachment downloads there, Images won't be shown in the gallery.
  • Alert will show when the file is downloaded
  • Otherwise, it will show an error alert

Android

  • WRITE_EXTERNAL_STORAGE and READ_EXTERNAL_STORAGE enabled in the AndroidMenifest.xml
  • User will be asked to allow storage permission if already not given
  • When the user allows the permission download will start
  • Android Download Manager will show a notification in the status bar
  • Files will be saved in the storage's Downloads/Expensify folder
  • Images will be displayed in the gallery under downloads

Web, Desktop, and Mobile web

  • The user can open the attachment and click the download button
  • Then after the download window appear, the user can select a location and save the file

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Desktop

Image download
image

Pdf download
image

Web

Image download
image

Pdf download
image

iOS (Simulator)

Image download
image

Pdf download
image

Downloaded files
image

Android

Permission dialog
image

Image download
image

Pdf download
image

Downloaded files
image

@SameeraMadushan SameeraMadushan requested a review from a team as a code owner February 27, 2021 18:28
@botify botify requested review from bondydaa and removed request for a team February 27, 2021 18:28
Copy link
Copy Markdown
Contributor

@bondydaa bondydaa left a comment

Choose a reason for hiding this comment

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

Just a couple of comments but also going to have someone more familiar give a look at the native stuff.

);

// creating anchor tag to initiate download
const link = document.createElement('a');
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I was really confused here and thought "there's no way this is the best way to download something in 2021, right?" but after doing some searching uh ya I guess it still is 🤯 https://davidwalsh.name/javascript-download

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes right?
I was also searching for a better way to do that. But found this is still valid.

@bondydaa bondydaa requested a review from AndrewGable March 2, 2021 04:47
@SameeraMadushan SameeraMadushan requested a review from bondydaa March 2, 2021 17:19
bondydaa
bondydaa previously approved these changes Mar 2, 2021
Copy link
Copy Markdown
Contributor

@bondydaa bondydaa left a comment

Choose a reason for hiding this comment

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

great thanks for those updates 👍 all you @AndrewGable (or re-assign to someone who might have more native knowledge 🙏 )

@bondydaa
Copy link
Copy Markdown
Contributor

bondydaa commented Mar 2, 2021

oh looks like conflicts in the Podfile.lock and Expensicons.js

@SameeraMadushan
Copy link
Copy Markdown
Contributor Author

@bondydaa Just fix conflicts. Got a pull from the upstream master. have a look. 👍🏻

- React-Core
- RNCAsyncStorage (1.12.1):
- React-Core
- RNCPicker (1.9.11):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

hmm looks like this was just added and merged a few hours ago here https://github.com/Expensify/Expensify.cash/pull/1553/files

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sounds like you need to npx pod-install to fix? cc @marcaaron ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did pod install and RNCPicker got removed. What can be the case?
Even in the master when I do pod install, It's getting removed from pod.lock

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Okay.. since the picker is newly added, I need to do run npm install. Then only pod install.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I pulled master, ran npm install in the root then cd ios/ && pod install and looks like I'm getting the same changes so something is messed up. Asking in our Slack to try and get it figured out

image

Copy link
Copy Markdown
Contributor

@bondydaa bondydaa Mar 3, 2021

Choose a reason for hiding this comment

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

okay sorry for the delay. I'm pretty sure this https://github.com/Expensify/Expensify.cash/pull/1553/files was done wrong and what you have here is correct.

Looking at https://artsy.github.io/blog/2016/05/03/podspec-checksums/ and https://romainbrunie.medium.com/podfile-lock-87df8c744dc6 helped show me how to confirm the checksum. only thing to note is we install the pods into the repo directory instead of a global ~/.cocoapods.

Expensidev/Expensify.cash (master) $ pod ipc spec ios/Pods/Local\ Podspecs/react-native-document-picker.podspec.json | openssl sha1
b3e78a8f7fef98b5cb069f20fc35797d55e68e28

looks like another conflict in the podfile.lock 😭 so if you could pull master again, npm i && cd ios && pod install and repush.

I'll merge to once conflicts are gone. Thanks for bearing with me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@bondydaa please check now.. 😊

AndrewGable
AndrewGable previously approved these changes Mar 3, 2021
@bondydaa bondydaa merged commit 653750d into Expensify:master Mar 3, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow download of images shared in chat

3 participants