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

SA429 Cannot save generic files #1391

Merged
merged 7 commits into from
Jan 15, 2024

Conversation

alansley
Copy link

@alansley alansley commented Jan 15, 2024

Contributor checklist

  • I have tested my contribution on these devices:
  • Device A, Samsung S9+ on Android 9 / API 28
  • Virtual device W, Pixel 3a on Android 14 / API 34
  • My contribution is fully baked and ready to be merged as is
  • I ensure that all the open issues my contribution fixes are mentioned in the commit message of my first commit using the Fixes #1234 syntax

Description

Attempting to download an attachment before the attachment has finished transferring from the network resulted in a toast with an error message being displayed ("Error while saving attachment to storage"). To fix this issue a check is performed about whether an attachment is still downloading or not, and if so and the user attempts to download the file before it's finished a toast says "Please wait until attachment has finished downloading".

Further, a spinner has been added to the message VisibleMessageContentView which replaces the generic file icon and animates a spinner during download, then when the attachment download has completed the generic 'file' icon replaces the spinner.

All commits in this PR relate to fixing the same issue.

@@ -132,6 +132,9 @@ class VisibleMessageContentView : ConstraintLayout {
}

if (message is MmsMessageRecord) {

//if (message.isMediaPending) displaySpinnerDuringAttachmentDownload()
Copy link

Choose a reason for hiding this comment

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

This one might be left over from before

Copy link
Author

Choose a reason for hiding this comment

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

Addressed!

@@ -179,6 +182,12 @@ class VisibleMessageContentView : ConstraintLayout {
hideBody = true
// Document attachment
if (contactIsTrusted || message.isOutgoing) {
// Show the progress spinner if the attachment is downloading, otherwise show
// the document icon (and always remove the other, whichever one that is)
binding.documentView.documentViewProgress.visibility = if (message.isMediaPending) { VISIBLE } else { GONE }
Copy link

Choose a reason for hiding this comment

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

we can probably push this one into the DocumentView.bind method, also check out this helper if that makes it easier :)

Copy link
Author

Choose a reason for hiding this comment

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

Done & tested - all works nicely

@@ -120,6 +119,8 @@ class VisibleMessageView : LinearLayout {
}
// endregion

var messagesWeHaveBound = mutableListOf<Long>()
Copy link

Choose a reason for hiding this comment

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

Is this one also a leftover?

Copy link
Author

Choose a reason for hiding this comment

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

Oops

@0x330a 0x330a merged commit 2fff9ec into oxen-io:dev Jan 15, 2024
@0x330a 0x330a mentioned this pull request Jan 15, 2024
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.

2 participants