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 2023-01-25] [$1000] mWeb - Chat - The avatar in the groupDM welcome message does not have the default grey background #13585

Closed
kbecciv opened this issue Dec 14, 2022 · 56 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@kbecciv
Copy link

kbecciv commented Dec 14, 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. Go to URL https://staging.new.expensify.com/
  2. Login with any account (applausetester+ihchat3@applause.expensifail.com/ Yulia1986Gerets)
  3. Click on FAB button/New Group
  4. Choose several users, one of them should have an avatar with a transparent background -> Click "Create group"

Expected Result:

The member with a transparent background PNG avatar in the group's welcome message should have a default grey background, as seen in other places the avatars are displayed.

Actual Result:

The member with a transparent background PNG avatar in the groupDM's welcome message does not have the default grey background

Workaround:

Unknown

Platform:

Where is this issue occurring?

  • Mobile Web

Version Number: 1.2.39.0

Reproducible in staging?: Yes

Reproducible in production?: Yes

Email or phone of affected tester (no customers): applausetester+ihchat3@applause.expensifail.com/ Yulia1986Gerets

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

Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01f4e7464efae609c6
  • Upwork Job ID: 1612490320358330368
  • Last Price Increase: 2023-01-09
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 14, 2022
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Dec 14, 2022
@slafortune
Copy link
Contributor

signing out until next week so reassigning this as to not hold it up

@slafortune slafortune removed the Bug Something is broken. Auto assigns a BugZero manager. label Dec 14, 2022
@slafortune slafortune removed their assignment Dec 14, 2022
@slafortune slafortune added the Bug Something is broken. Auto assigns a BugZero manager. label Dec 14, 2022
@danieldoglas danieldoglas self-assigned this Dec 14, 2022
@danieldoglas
Copy link
Contributor

assigning this one to me

@stephanieelliott
Copy link
Contributor

Hey @danieldoglas does that mean you'll fix this one internally?

@danieldoglas
Copy link
Contributor

I read it too fast and thought that melvin had added the same person as the one doing triaging for the bug, sorry! I was going to continue it through the normal flow, so please do what you were going to do before, and I'll be here to do the triaging of proposals!

@melvin-bot melvin-bot bot added the Overdue label Dec 19, 2022
@stephanieelliott
Copy link
Contributor

Hm, I've tested this a few times -- I think this is expected behavior, no? A PNG with a transparent background would show up as it does in the example that was shared:

image

The alternative would be... backfilling the image background? IMO we should be displaying the image exactly as the user uploaded it.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Dec 22, 2022
@danieldoglas
Copy link
Contributor

I think the expected result here would be something like this instead @stephanieelliott

image

@trjExpensify
Copy link
Contributor

Hm, I agree. I'm not sure I understand how this is a bug. "Not shown correctly" isn't really descriptive, what are we expecting exactly?

CC: @Beamanator

@danieldoglas
Copy link
Contributor

Sorry, I'm not sure if I was not clear, but the image I've uploaded have all users' images on the same size, while the images when using mobile web are different sizes. Is that how they were supposed to be?

@melvin-bot melvin-bot bot removed the Overdue label Jan 5, 2023
@trjExpensify
Copy link
Contributor

It is the same size though, isn't it? It's just the transparent background? I think the question is whether we should be filling the background in, which it looks like we are in other places, but that seems to defeat the purpose of a transparent background:

image

image

image

image

@danieldoglas
Copy link
Contributor

Ohhhhhhhhhh now I see. When I looked at the first picture in the issue, I thought Beaman's picture was actually two pictures. I'm completely wrong here, sorry about that

@Beamanator
Copy link
Contributor

Lol! Sorry for the confusion y'all :D

@trjExpensify
Copy link
Contributor

@Beamanator, to confirm, are we expecting to fill in the transparent background then? 😕

@Beamanator
Copy link
Contributor

No no I think this isn't an issue so we can close

@Beamanator
Copy link
Contributor

Wait maybe I'm confused?

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jan 10, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 10, 2023

📣 @bernhardoj You have been assigned to this job by @danieldoglas!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@stephanieelliott
Copy link
Contributor

Sent the offer to @bernhardoj, also extended the job to @eVoloshchak for C+.

@bernhardoj
Copy link
Contributor

bernhardoj commented Jan 10, 2023

PR is ready for review.

@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Daily KSv2 labels Jan 13, 2023
@melvin-bot melvin-bot bot changed the title [$1000] mWeb - Chat - The avatar in the groupDM welcome message does not have the default grey background [HOLD for payment 2023-01-20] [$1000] mWeb - Chat - The avatar in the groupDM welcome message does not have the default grey background Jan 13, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jan 13, 2023

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.53-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

If no regressions arise, payment will be issued on 2023-01-20. 🎊

After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:

  • External issue reporter
  • Contributor that fixed the issue
  • Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

  • Merged PR within 3 business days of assignment - 50% bonus
  • Merged PR more than 9 business days after assignment - 50% penalty

@melvin-bot
Copy link

melvin-bot bot commented Jan 13, 2023

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

  • [@eVoloshchak / @danieldoglas] The PR that introduced the bug has been identified. Link to the PR:
  • [@eVoloshchak / @danieldoglas] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@eVoloshchak / @danieldoglas] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@stephanieelliott] A regression test has been added or updated so that the same bug will not reach production again. Link to the GH issue for creating the test here:

@bernhardoj
Copy link
Contributor

There is a regression happened reported here. I have opened a new PR to solve that regression.

@danieldoglas @eVoloshchak

@mountiny
Copy link
Contributor

@stephanieelliott As mentioned above, could we make sure @jayeshmangwani is paid a reporting bonus for reporting the regression https://expensify.slack.com/archives/C049HHMV9SM/p1673730118353969

The reward for the contributor and C+ should be adjusted because of the regression.

Thanks for addressing it so quick @bernhardoj!

@mountiny
Copy link
Contributor

PR Merged!

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jan 19, 2023
@mallenexpensify mallenexpensify removed the Reviewing Has a PR in review label Jan 20, 2023
@melvin-bot melvin-bot bot added the Overdue label Jan 23, 2023
@eVoloshchak
Copy link
Contributor

Not overdue, PR has been deployed to production 4 days ago

@melvin-bot melvin-bot bot removed the Overdue label Jan 23, 2023
@stephanieelliott stephanieelliott changed the title [HOLD for payment 2023-01-20] [$1000] mWeb - Chat - The avatar in the groupDM welcome message does not have the default grey background [HOLD for payment 2023-01-25] [$1000] mWeb - Chat - The avatar in the groupDM welcome message does not have the default grey background Jan 23, 2023
@stephanieelliott
Copy link
Contributor

This has been paid!

@jayeshmangwani
Copy link
Contributor

@stephanieelliott reporting regression bonus is due here

@stephanieelliott
Copy link
Contributor

Oop, sorry I missed that -- sent you the offer in Upwork @jayeshmangwani!

@jayeshmangwani
Copy link
Contributor

Offer accepted @stephanieelliott , Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests