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 for payment 2024-04-17] - Avatar placeholder remains in dark mode when app is in light mode #33161

Closed
6 tasks done
kbecciv opened this issue Dec 15, 2023 · 46 comments
Closed
6 tasks done
Assignees
Labels
Design Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2

Comments

@kbecciv
Copy link

kbecciv commented Dec 15, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Version Number: v1.4.13-4
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 team
Slack conversation:

Action Performed:

Precondition: App is in light mode.

  1. Launch New Expensify app.
  2. Go offline.
  3. Tap Search.
  4. Look for user with placeholder avatar.

Expected Result:

The avatar placeholder will be displayed in light mode.

Actual Result:

The avatar placeholder remains in dark mode.

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

image

Bug6314487_1702642885688.Screen_Recording_20231215_031248_New_Expensify.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b86fc7d9a009aa8d
  • Upwork Job ID: 1737654517482057728
  • Last Price Increase: 2023-12-21
Issue OwnerCurrent Issue Owner: @grgia
@kbecciv kbecciv added the DeployBlockerCash This issue or pull request should block deployment label Dec 15, 2023
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.

Copy link

melvin-bot bot commented Dec 15, 2023

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

@Julesssss Julesssss added Daily KSv2 and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 labels Dec 15, 2023
@Julesssss
Copy link
Contributor

This isn't worth blocking the deploy, so I removed the label.

@Krishna2323

This comment was marked as off-topic.

Copy link

melvin-bot bot commented Dec 15, 2023

Triggered auto assignment to @dubielzyk-expensify (Design), see these Stack Overflow questions for more details.

@grgia
Copy link
Contributor

grgia commented Dec 15, 2023

This is just the image we use for the fallback avatar, so it's not technically a bug. @dubielzyk-expensify do you think we should use a theme-specific image here?

@grgia grgia self-assigned this Dec 15, 2023
@grgia grgia changed the title Theme - Avatar placeholder remains in dark mode when app is in light mode [Theme Switching] - Avatar placeholder remains in dark mode when app is in light mode Dec 15, 2023
@dubielzyk-expensify
Copy link
Contributor

Sure can. Do you have a link to the original so I can check the export settings?

@dubielzyk-expensify
Copy link
Contributor

Think I found this here and it seems to use outdated colors as well, so uploading both new dark and light version of our fallback avatar:

svgs.zip

fallback-avatar-dark
fallback-avatar-light

cc @Expensify/design

@shawnborton
Copy link
Contributor

Nice! What color are you using for the fallback avatar BG color? We might want to use something like the icon color as the BG here so that we never have any worry about the icon getting lost in the LHN depending on active or hover states. Actually that being said, I think our border color (same BG as buttons) would be a good BG color as it won't collide with rowHover or the active/press state?

@dubielzyk-expensify
Copy link
Contributor

It's currently the background is using borders and the icon uses icon color:
CleanShot 2023-12-19 at 09 44 57@2x

@shawnborton
Copy link
Contributor

Lovely! Let's do it then!

@dannymcclain
Copy link
Contributor

:chef-kiss:

@puneetlath puneetlath removed their assignment Dec 19, 2023
@grgia
Copy link
Contributor

grgia commented Dec 20, 2023

On it!

@grgia
Copy link
Contributor

grgia commented Dec 20, 2023

image While we're at it, should we check on these colors as well? @dubielzyk-expensify @shawnborton @dannymcclain

@grgia
Copy link
Contributor

grgia commented Dec 20, 2023

Some of these are definitely unused

@dubielzyk-expensify
Copy link
Contributor

Any updates on this one?

@joekaufmanexpensify
Copy link
Contributor

I think the next step is to continue with the draft PR.

@grgia
Copy link
Contributor

grgia commented Mar 20, 2024

Yeah going to get this one updated this week as it relates to another bug

@melvin-bot melvin-bot bot removed the Overdue label Mar 20, 2024
@grgia
Copy link
Contributor

grgia commented Mar 20, 2024

Found a much easier way to do this, going to create a new PR

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Monthly KSv2 labels Mar 22, 2024
@joekaufmanexpensify
Copy link
Contributor

PR out on staging

Copy link

melvin-bot bot commented Apr 9, 2024

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

@joekaufmanexpensify joekaufmanexpensify changed the title [Theme Switching] - Avatar placeholder remains in dark mode when app is in light mode [hold for payment 2024-04-17] - Avatar placeholder remains in dark mode when app is in light mode Apr 15, 2024
@joekaufmanexpensify
Copy link
Contributor

PR is on prod. Payment due on 2024-04-17

@joekaufmanexpensify
Copy link
Contributor

@s77rt do you think a regression test would be helpful here?

@s77rt
Copy link
Contributor

s77rt commented Apr 15, 2024

It's unlikely that this bug would resurface again. Let's not add a specific test case here. The generic one should be enough https://expensify.testrail.io/index.php?/cases/view/2990277

@melvin-bot melvin-bot bot added Monthly KSv2 and removed Weekly KSv2 labels Apr 15, 2024
Copy link

melvin-bot bot commented Apr 15, 2024

This issue has not been updated in over 15 days. @s77rt, @grgia, @joekaufmanexpensify, @dubielzyk-expensify 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!

@joekaufmanexpensify joekaufmanexpensify added Weekly KSv2 and removed Monthly KSv2 labels Apr 22, 2024
@joekaufmanexpensify
Copy link
Contributor

Sounds good. TY!

@joekaufmanexpensify
Copy link
Contributor

All set to issue payment here. Only payment is $250 to @s77rt via upwork for C+ review. (Would've normally been $500 based on when the issue was created, but this one had a regression).

@joekaufmanexpensify
Copy link
Contributor

OG upwork job is closed, so I opened a new one.

@joekaufmanexpensify
Copy link
Contributor

@s77rt offer sent for $250!

@s77rt
Copy link
Contributor

s77rt commented Apr 22, 2024

@joekaufmanexpensify Accepted! Thanks!

@joekaufmanexpensify
Copy link
Contributor

@s77rt $250 sent and contract ended!

@joekaufmanexpensify
Copy link
Contributor

Upwork job closed.

@joekaufmanexpensify
Copy link
Contributor

All set. Thanks everyone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Engineering Internal Requires API changes or must be handled by Expensify staff NewFeature Something to build that is a new item. Reviewing Has a PR in review Weekly KSv2
Projects
No open projects
Development

No branches or pull requests