-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[$500] Avatar - Avatar is blank instead of placeholder and no spinner when loading it offline #33614
Comments
👋 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:
|
Triggered auto assignment to @marcaaron ( |
Issue is not reproducible on production Bug6326350_1703616605871.production.mp4 |
👋 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:
|
I guess either related to personal details or the search page. This PR looks the most likely culprit, but have not reverted to test the theory yet. |
Oh hmm it could also be this ginormous expo image PR #30905 |
I don't feel strongly that this needs to be a blocker. When you come back online the images are restored. But will leave it to the deployer to make the final decision. |
Triggered auto assignment to @bfitzexpensify ( |
Job added to Upwork: https://www.upwork.com/jobs/~017c4e38b1b18578d0 |
Bug0 Triage Checklist (Main S/O)
|
This issue has not been updated in over 15 days. @eVoloshchak, @marcaaron, @bfitzexpensify, @dukenv0307 eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
@bfitzexpensify Please add the weekly label again, the PR is still reviewing. |
This issue has not been updated in over 15 days. @eVoloshchak, @marcaaron, @bfitzexpensify, @dukenv0307 eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
@bfitzexpensify, this was resolved by another PR (#38674), what should be our next steps? |
@bfitzexpensify, a gentle bump on the above |
Thanks for the bump @eVoloshchak. @marcaaron, do you think the work in #35639 constitutes perhaps a half-payment out here? |
Honestly, I'm not sure. While there was investigation, PR created, partial review, etc. It feels like we might have missed the mark on urgency here. Observations: The PR was marked ready for review three months ago. The alternate PR merged 3 weeks ago. Is there any explanation for why we couldn't get this merged in that time? |
We never arrived at a fix that would work for all of the cases, this one specifically. I agree we should have been faster, no payment seems fair to me personally |
Thanks for the comments. I think skipping payment for missing on urgency here is fair. Closing this one out. |
@marcaaron @bfitzexpensify Our PR #35639 fixed this issue and we completed it a long time ago. The problem is I tried to explain the confusion in this case #35639 (review) from @eVoloshchak which is not a bug. The PR #38674 fixed this issue and has the same idea as my proposal, the only difference is instead of using the current fill color of the default avatar we apply the theme color for this avatar in this PR #38674. So I think payment here for C and C+ is fair since we spent lots of effort in the PR and it's almost completed. |
@bfitzexpensify Please check my comment above when you have a chance. |
Reopened and made @bfitzexpensify the owner to get 👀, removed 'reviewing' too. |
Reviewed again. Agree that there was a substantial amount of work, but I still think it missed the mark on urgency. Given that, I think a 50% payout is fair here. Payment summary: @dukenv0307 to be paid $250 for contributor work - offer sent via Upwork |
Thanks @bfitzexpensify for helping here, I've accepted the offer. |
You're very welcome. Payment complete. Closing this out again. |
$250 approved for @eVoloshchak |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.17-1
Reproducible in staging?: y
Reproducible in production?: n
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal
Slack conversation:
Action Performed:
Expected Result:
In Step 4, users with custom avatar will show the avatar placeholder (production behavior - circular head with body with green background).
In Step 7, spinner will be shown when loading user avatar.
Actual Result:
In Step 4, the users with custom avatar are blank and gray.
In Step 7, the spinner only appears for a split second.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6326350_1703616605864.staging.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @bfitzexpensifyThe text was updated successfully, but these errors were encountered: