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

[HOLD] No loading animation or preview for persisted attachments #7934

Closed
thienlnam opened this issue Feb 28, 2022 · 11 comments
Closed

[HOLD] No loading animation or preview for persisted attachments #7934

thienlnam opened this issue Feb 28, 2022 · 11 comments
Assignees
Labels
Improvement Item broken or needs improvement. Monthly KSv2

Comments

@thienlnam
Copy link
Contributor

thienlnam commented Feb 28, 2022

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


Creating a new ticket for: #3867 (comment)

Action Performed:

  1. Be offline
  2. Select an attachment to be uploaded (preferable larger so it takes more time to upload)
  3. Quit the app
  4. Open the app again
  5. Open the chat where the attachment was uploaded
  6. Observe it's not there, and there's no indication something is uploading
  7. The attachment only appears after the upload is over

Expected Result:

There should be an indication or preview of the attachment being uploaded

Actual Result:

No preview or loading animation while the attachment was being uploaded

Workaround:

The attachment will still be uploaded, the user will just not know that it's in the process of being uploaded

Platform:

Where is this issue occurring?

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

Version Number:
Reproducible in staging?: Yes
Reproducible in production?: Yes
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation
Expensify/Expensify Issue URL:
Issue reported by: @kidroca
Slack conversation:

View all open jobs on GitHub

@thienlnam thienlnam added AutoAssignerTriage Auto assign issues for triage to an available triage team member Daily KSv2 labels Feb 28, 2022
@MelvinBot
Copy link

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

@MelvinBot MelvinBot removed the AutoAssignerTriage Auto assign issues for triage to an available triage team member label Feb 28, 2022
@thienlnam thienlnam added the External Added to denote the issue can be worked on by a contributor label Feb 28, 2022
@MelvinBot
Copy link

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

@thienlnam thienlnam added Improvement Item broken or needs improvement. Weekly KSv2 and removed Daily KSv2 labels Feb 28, 2022
@thienlnam
Copy link
Contributor Author

thienlnam commented Feb 28, 2022

Sorry for the tags and pings, I'm breaking down #3867 (comment) into separate tickets so they can be investigated and be known as existing bugs. This will still need an upwork job created for it

@thienlnam thienlnam assigned thienlnam and puneetlath and unassigned puneetlath Feb 28, 2022
@thienlnam
Copy link
Contributor Author

@kidroca Proposal:

I think we should try to cross reference optimistic actions with requests in the network queue
This way when we clean up optimistic actions we keep the ones that still have pending request

We already put optimisticReportActionID on the request so we can distinct it

App/src/libs/actions/Report.js

Lines 1169 to 1175 in 425e718

API.Report_AddComment({
reportID,
file,
reportComment: commentText,
clientID: optimisticReportActionID,
persist: true,
})

We can make a lib or expose a function that return any pending Report_AddComment actions from network queue or persisted storage
Report.removeOptimisticActions would use that and skip removing optimistic actions that have pending Report_AddComment

@kidroca
Copy link
Contributor

kidroca commented Feb 28, 2022

BTW the next design of the network write queue would first persist requests to Onyx
We can assume that as long as the request with optimisticReportActionID is still in Onyx the attachment is still uploading

So for any optimistic action we can make a cross check to Onyx and keep the optimistic action (to represent loading state) if it can be cross referenced to data in Onyx

Note: the new network write queue is yet to be implemented

@thienlnam
Copy link
Contributor Author

Sounds good, going to put this on hold until that new write queue is completed but will keep this issue up so people are aware of this known bug

@thienlnam thienlnam changed the title No loading animation or preview for persisted attachments [HOLD] No loading animation or preview for persisted attachments Feb 28, 2022
@thienlnam thienlnam added Monthly KSv2 and removed Weekly KSv2 labels Feb 28, 2022
@puneetlath
Copy link
Contributor

I'm going to unassign since I'm going on sabbatical soon. Please re-apply the External label when it's off hold to get someone assigned.

@puneetlath puneetlath removed their assignment Feb 28, 2022
@puneetlath puneetlath removed the External Added to denote the issue can be worked on by a contributor label Feb 28, 2022
@thienlnam
Copy link
Contributor Author

I know we've added batching but the new write queue doesn't seem to be done yet correct? @kidroca

@MelvinBot MelvinBot removed the Overdue label Apr 4, 2022
@kidroca
Copy link
Contributor

kidroca commented Apr 4, 2022

Yes, that's correct
I think these are the latest new about the write queue: https://expensify.slack.com/archives/C01GTK53T8Q/p1648658394841419
There's a design doc link, which is not accessible by everybody

I think the write queue changes would be applied somewhere along the way

@melvin-bot melvin-bot bot added the Overdue label May 5, 2022
@thienlnam
Copy link
Contributor Author

Still holding

@melvin-bot melvin-bot bot removed the Overdue label May 6, 2022
@melvin-bot melvin-bot bot added the Overdue label Jun 7, 2022
@thienlnam
Copy link
Contributor Author

Going to close this for now until we decide we want to add it in the future

@melvin-bot melvin-bot bot removed the Overdue label Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Item broken or needs improvement. Monthly KSv2
Projects
None yet
Development

No branches or pull requests

5 participants