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 #10894 and #9402][Image] Settings - The grey avatar displayed for a brief moment when uploading a picture #10704

Closed
kbecciv opened this issue Aug 30, 2022 · 35 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Monthly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Aug 30, 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. Login at staging.new.expensify.com
  2. Navigate to settings by clicking on the user avatar over LHN
  3. Click on Profile
  4. Click on the avatar edit icon
  5. Click on upload photo
  6. Select any image of your choice
  7. Verify it’s displayed in the avatar icon
  8. Click on save

Expected Result:

Uploaded photo should be displayed without grey color

Actual Result:

The grey avatar displayed for a brief moment when uploading a picture

Workaround:

Unknown

Platform:

Where is this issue occurring?

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

Version Number: 1.1.94.4

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): any

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

Notes/Photos/Videos: Any additional supporting documentation

Bug5711038_and_3008_profile_icon.mp4
Bug5711038_and_3008.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

@melvin-bot
Copy link

melvin-bot bot commented Aug 30, 2022

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

@Luke9389
Copy link
Contributor

Hey @jasperhuangg should I make this external? I see you worked on refactoring Avatar uploads here: https://github.com/Expensify/Web-Expensify/pull/34346

@melvin-bot melvin-bot bot added the Overdue label Sep 5, 2022
@melvin-bot
Copy link

melvin-bot bot commented Sep 6, 2022

@Luke9389 Eep! 4 days overdue now. Issues have feelings too...

@Luke9389
Copy link
Contributor

Luke9389 commented Sep 6, 2022

bump @jasperhuangg

@melvin-bot melvin-bot bot removed the Overdue label Sep 6, 2022
@jasperhuangg
Copy link
Contributor

jasperhuangg commented Sep 6, 2022

Hey! I can't seem to reproduce this.. could it be because the internet connection was poor and it was taking a while to load the image from s3? @Luke9389

Probably not best to apply the external label right now since it might involve touching PHP as well.

Screen.Recording.2022-09-06.at.12.40.28.PM.mov
RPReplay_Final1662464723.MP4

@Luke9389
Copy link
Contributor

Luke9389 commented Sep 6, 2022

I think that vid you posted of your phone is a reproduction of the issue. The avatar turned grey for a brief moment there.

@Luke9389
Copy link
Contributor

Luke9389 commented Sep 6, 2022

I'll investigate whether this can be a front end only change. My hunch is that it will be, because the network will likely never be so fast that it's unnoticeable, and we already have the image on the device, so we should be able to display the image continuously.

@jasperhuangg
Copy link
Contributor

I think that vid you posted of your phone is a reproduction of the issue. The avatar turned grey for a brief moment there.

@Luke9389 that's actually expected to happen, that's the Avatar component switching briefly between the local file and the file in the URL.

@Luke9389
Copy link
Contributor

Luke9389 commented Sep 6, 2022

I see. So I think that's the "issue" being reported here. I wonder if there is a way to eliminate that grey period. Perhaps we only switch the avatar source upon loading the next view or something. We could have a contributor look into it, but idk, do you think this is worth keeping open?

@jasperhuangg
Copy link
Contributor

jasperhuangg commented Sep 6, 2022

Not really, because that's how the behavior was when I was working on the PR, and the PR was reviewed and approved in that state. I don't think this is a bug as much as a flaw with our design. Maybe we can create a follow up issue to work on? But I think that's not as much of a priority now.

@Luke9389

@Luke9389
Copy link
Contributor

Luke9389 commented Sep 6, 2022

OK, I think before closing this I'll bring it up in Slack to see if anyone has additional thoughts on what we should do with this issue. Thanks for chiming in @jasperhuangg!

Edit: Posted about this here: https://expensify.slack.com/archives/C01GTK53T8Q/p1662470480794339

@jasperhuangg
Copy link
Contributor

OK, I think before closing this I'll bring it up in Slack to see if anyone has additional thoughts on what we should do with this issue. Thanks for chiming in @jasperhuangg!

For sure! I think @Beamanator would be a good person to ask since he wrote the DD. Thanks for including me :)

@Luke9389 Luke9389 changed the title Settings - The grey avatar displayed for a brief moment when uploading a picture [HOLD] Settings - The grey avatar displayed for a brief moment when uploading a picture Sep 6, 2022
@Luke9389
Copy link
Contributor

Luke9389 commented Sep 6, 2022

Putting a hold on this issue until there's a clear, testable resolution here: #9402

@Beamanator
Copy link
Contributor

Yeah I must have overlooked the brief flash when testing, I think the main issue we've seen with this is when the previous avatar shows for a while before the new avatar updates (I think only reproduced on Android) - but maybe that's a different issue.

I also think it makes sense to put this on hold (maybe reduce the priority too) until that other issue gets worked on, maybe the solution there will fix this issue 🤞

@Luke9389
Copy link
Contributor

This is on HOLD so it shouldn't be marked Overdue.

@melvin-bot melvin-bot bot removed the Overdue label Sep 19, 2022
@JmillsExpensify JmillsExpensify changed the title [HOLD] Settings - The grey avatar displayed for a brief moment when uploading a picture [HOLD] [Bug] Settings - The grey avatar displayed for a brief moment when uploading a picture Sep 21, 2022
@JmillsExpensify JmillsExpensify changed the title [HOLD] [Bug] Settings - The grey avatar displayed for a brief moment when uploading a picture [HOLD #9402] [Bug] Settings - The grey avatar displayed for a brief moment when uploading a picture Sep 27, 2022
@trjExpensify
Copy link
Contributor

Same, Melv.

@melvin-bot melvin-bot bot removed the Overdue label Jun 5, 2023
@melvin-bot melvin-bot bot added the Overdue label Jul 6, 2023
@trjExpensify
Copy link
Contributor

No change, server migration project in progress still.

@trjExpensify
Copy link
Contributor

Still on hold for the server migration project.

@melvin-bot melvin-bot bot added the Overdue label Sep 15, 2023
@trjExpensify
Copy link
Contributor

Still on hold for the server migration project.

@melvin-bot melvin-bot bot removed the Overdue label Sep 15, 2023
@trjExpensify
Copy link
Contributor

No change.

@melvin-bot melvin-bot bot removed the Overdue label Oct 17, 2023
@melvin-bot melvin-bot bot added the Overdue label Nov 20, 2023
@trjExpensify
Copy link
Contributor

CORS is over the line, so image caching on web is back under construction.

@melvin-bot melvin-bot bot removed the Overdue label Nov 20, 2023
@melvin-bot melvin-bot bot added the Overdue label Dec 22, 2023
@trjExpensify
Copy link
Contributor

Image caching PR was deployed and reverted. Awaiting this now before re-implementing: #32703

@melvin-bot melvin-bot bot removed the Overdue label Jan 3, 2024
@melvin-bot melvin-bot bot added the Overdue label Feb 5, 2024
@trjExpensify
Copy link
Contributor

Damn, we so unlucky Melv! The image caching PR got reverted again.

@melvin-bot melvin-bot bot removed the Overdue label Feb 5, 2024
@melvin-bot melvin-bot bot added the Overdue label Mar 7, 2024
@trjExpensify
Copy link
Contributor

No change.

@melvin-bot melvin-bot bot removed the Overdue label Mar 8, 2024
@trjExpensify
Copy link
Contributor

Alrighty.. circling back here. I can no longer reproduce it on upload of a profile picture. So I think we're good to close this out now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering Monthly KSv2
Projects
None yet
Development

No branches or pull requests

8 participants