-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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-07-24] [$1000] Web - Tooltip is not shown for a new user on invite message page #21946
Comments
Triggered auto assignment to @NicMendonca ( |
Bug0 Triage Checklist (Main S/O)
|
ProposalPlease re-state the problem that we are trying to solve in this issue.Web - Tooltip is not shown for a new user on invite message page What is the root cause of that problem?
App/src/components/MultipleAvatars.js Line 186 in 619394d
What changes do you think we should make in order to solve the problem?We don't have a data for new users in fallbackUserDetails={{
displayName: ReportUtils.getDisplayNameForParticipant(props.icons[1].id),
login: lodashGet(props.icons[1], 'name',''),
avatar: lodashGet(props.icons[1], 'source', ''),
}} We should provide fallback data in every instance of tooltip in What alternative solutions did you explore? (Optional) |
@NicMendonca Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
still reviewing this |
@priya-zha are those clickable on other platforms? Should they be clickable? 🤔 |
No. It's not about being clickable or not. Just try to hover over the avatar and notice that on hover, the tooltip isn't present for new users. |
@priya-zha my apologies, I understand now. Any the other platforms? Can you make sure you are testing on all of those? We want to make sure we are ensuring parity. I was able to also reproduce on MacOS Desktop as an example: |
I tested in on chrome (linux). Since it's related to tooltips, I could have tested it on Mac Os chrome or safari , but I don't have Mac to test them. |
I think it makes sense to have the tooltip here out of consistency's sake. That being said, I think we just merged some code that touches this area so let's make sure it's still happening (cc @puneetlath) |
I agree it makes sense to have the tooltip using the fallback details. |
Job added to Upwork: https://www.upwork.com/jobs/~01aa1190833be5a152 |
Current assignee @NicMendonca is eligible for the External assigner, not assigning anyone new. |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia ( |
@alitoshmatov's proposal looks good! 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @mountiny, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @Priya! 📣
|
The BZ member will need to manually hire priya for the Reporter role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future! |
@thesahindia Looks like icons are now empty and just have avatar for user The reason is that,
And this method is setting default value as empty. |
I think we should modify |
Makes sense to me. |
PR ready - #22693 |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.41-3 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-07-24. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
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:
|
@thesahindia will you be requesting payment via Expensify? |
@priya-zha can you please apply to the job? https://www.upwork.com/jobs/~01aa1190833be5a152 |
@alitoshmatov you've been paid! 🎉 |
bump @priya-zha @thesahindia ^^ |
yup! I think there is no need to add a regression test? And we can skip the other items of the checklist too since this couldn't have been caught. It was caused by a big PR |
@priya-zha - reporting bonus $250 |
@priya-zha just need you to accept the offer in Upwork please 🙏 |
@NicMendonca submitted the proposal. Apologies for the delay. I was out for a few days and didn't check the mail. Thank you for the offer. |
@priya-zha you've been paid! |
Everyone has been paid! @thesahindia don't forget to request payment via Expensify.
|
Reviewed the details for @thesahindia. $1,500 approved for payment in NewDot based on the BZ summary above. |
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:
Expected Result:
Tooltip should be shown for a new user on invite message page
Actual Result:
Tooltip is not shown for a new user on invite message page
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Version Number: 1.3.34-1
Reproducible in staging?: y
Reproducible in production?: y
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
Notes/Photos/Videos: Any additional supporting documentation
green-2023-06-29_12.36.12.mp4
Recording.3326.mp4
Expensify/Expensify Issue URL:
Issue reported by: @priya-zha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1688021163521579
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: