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

IOS - Attachment - Image upload remains loading #7900

Closed
kbecciv opened this issue Feb 24, 2022 · 19 comments
Closed

IOS - Attachment - Image upload remains loading #7900

kbecciv opened this issue Feb 24, 2022 · 19 comments
Assignees

Comments

@kbecciv
Copy link

kbecciv commented Feb 24, 2022

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. Click on a conversation
  2. Click on add attachment
  3. Upload an image
  4. Verify a preview is displayed
  5. Click on upload
  6. Verify the image is uploaded

Expected Result:

The image is uploaded and you can see the image in the conversation

Actual Result:

The image doesn't load and you can't see it in the conversation.

Image.from.iOS.2.MP4

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • iOS

Version Number: 1.1.40.0

Reproducible in staging?: Yes

Reproducible in production?: No

Email or phone of affected tester (no customers): veraclementina@gmail.com](mailto:veraclementina@gmail.com

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

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Feb 24, 2022
@OSBotify
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open StagingDeployCash deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

@MelvinBot
Copy link

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

@kidroca
Copy link
Contributor

kidroca commented Feb 24, 2022

maybe related to this PR that added loading to images : #7723

@mdneyazahmad
Copy link
Contributor

There are two related pr #7723 and #7569

These two pr handles two different issues.

  1. First pr adds loading indicator to the images being loaded.
  2. Second pr adds a loading placeholder when an image is being uploaded.

Could you please confirm, when this issue occurs (uploading or after uploaded)?

@roryabraham
Copy link
Contributor

Sorry for the delay, looking into this now!

@roryabraham
Copy link
Contributor

Reproduced on main, checking #7723 first

@roryabraham
Copy link
Contributor

Doesn't seem like #7723 or #7569 caused this 🤔

@roryabraham
Copy link
Contributor

Not sure, but it's possible this is just a side-effect of infra issues we had today

@mdneyazahmad
Copy link
Contributor

@roryabraham thanks for your investigation. I have seen many times when internet connection is not good it keeps loading. This is in general not related to this issue only but to the entire app. Do we have a mechanism to handle this situation and give the user a proper error message or some sort of feedback that something went wrong after a specific time?

@kidroca
Copy link
Contributor

kidroca commented Feb 28, 2022

@roryabraham @mdneyazahmad It's definitely a problem from #7723

Reverting it fixes the problem for me (tested on physical device, not sure if that matters)
See how even attachments that were sent ages ago are "loading" but as soon as I revert changes made to `ImageWithSizeCalculation.js they instantly load

RPReplay_Final1646043126.MP4.mp4

@kidroca
Copy link
Contributor

kidroca commented Feb 28, 2022

I think loading: true should be set during onLoadStart instead of hardcoding to true in the constructor
Or this should be removed this.state.isLoading && styles.dNone, as display: none seem to prevent to trigger the normal image life cycle in the first place

@mountiny
Copy link
Contributor

Thank you @kidroca, I have created a revert PR #7933 so Rory can then merge and @mdneyazahmad will look into the feedback you provided.

@roryabraham
Copy link
Contributor

Thanks @kidroca ... not sure why reverting #7723 didn't seem to resolve the problem for me, testing with my local back-end and a simulator. Going to retest now

@roryabraham
Copy link
Contributor

For some reason I am still unable to load images from my dev API, working on it...

@roryabraham
Copy link
Contributor

On my dev API, I'm actually seeing this log line in a network error where we fail to get the image size 🤔

An SSL error has occurred and a secure connection to the server cannot be made.

@mountiny
Copy link
Contributor

@roryabraham Confirmed locally that the revert branch works for me, images already sent load and new upload shows up too.

@roryabraham
Copy link
Contributor

Got my VM/simulator issues sorted, merged the revert 👍

@yuwenmemon
Copy link
Contributor

Great! Removing the blocker label then and closing this out!

@yuwenmemon yuwenmemon removed the DeployBlockerCash This issue or pull request should block deployment label Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants