Skip to content

Added attachment download loading indicator#6455

Merged
Jag96 merged 12 commits into
Expensify:mainfrom
mananjadhav:fix/attachment-download-loading-indicator
Nov 30, 2021
Merged

Added attachment download loading indicator#6455
Jag96 merged 12 commits into
Expensify:mainfrom
mananjadhav:fix/attachment-download-loading-indicator

Conversation

@mananjadhav

@mananjadhav mananjadhav commented Nov 24, 2021

Copy link
Copy Markdown
Collaborator

Details

  • Adds a loading indicator for the download activity of Attachments

Fixed Issues

$ #6253

Tests

  1. Tested downloads for all platforms
  2. Tested parallel downloads for the attachments

QA Steps

  1. Go to any chat with attachments (or add a new doc/zip/xlsx, etc.)
  2. Click on the download icon
  3. You should see an indicator while the file is about to be downloaded
  4. Once the download is about to complete indicator should be replaced back with download icon.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

web-download-indicator

Mobile Web

mweb-download-indicator

Desktop

desktop-download-indicator

iOS

ios-download-indicator.mov

Android

android-download-indicator

@mananjadhav mananjadhav marked this pull request as ready for review November 26, 2021 04:12
@mananjadhav mananjadhav requested a review from a team as a code owner November 26, 2021 04:12
@botify botify requested a review from Jag96 November 26, 2021 04:12
@MelvinBot MelvinBot removed the request for review from a team November 26, 2021 04:12
@mananjadhav

Copy link
Copy Markdown
Collaborator Author

@Jag96 Any updates for me on this one?

@Jag96 Jag96 left a comment

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.

One thing I noticed on web (chrome)d, if I click the download icon and then click the spinner a few times while the file is downloading, after the first download is done the browser will attempt to download the file additional times based on how many times I clicked the spinner. Can we add a check in processDownload that doesn't call fileDownload if we are already downloading?

Comment thread src/components/AnchorForCommentsOnly/BaseAnchorForCommentsOnly/index.js Outdated
Comment thread src/components/AttachmentView.js Outdated
Comment thread src/libs/fileDownload/index.js Outdated
@mananjadhav

Copy link
Copy Markdown
Collaborator Author

@Jag96 Fixed the multiple download issue. PR updated

@Jag96 Jag96 left a comment

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.

LGTM

@Jag96 Jag96 merged commit f8d3392 into Expensify:main Nov 30, 2021
@OSBotify

Copy link
Copy Markdown
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify

OSBotify commented Dec 7, 2021

Copy link
Copy Markdown
Contributor

🚀 Deployed to staging by @Jag96 in version: 1.1.17-8 🚀

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

@OSBotify

OSBotify commented Dec 8, 2021

Copy link
Copy Markdown
Contributor

🚀 Deployed to production by @roryabraham in version: 1.1.18-3 🚀

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants