-
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
[Payment Due 2023-12-21][$500] Web – Chat – Tooltip is out if hover the cursor on the mentioned user you have no conversation #31720
Comments
Job added to Upwork: https://www.upwork.com/jobs/~01fd8d4190c3d5d7a0 |
Triggered auto assignment to @laurenreidexpensify ( |
Bug0 Triage Checklist (Main S/O)
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @ntdiary ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Tooltip is out if hover the cursor on the mentioned user you have no conversation What is the root cause of that problem?Because the personal details for that new account was not in Onyx yet and we did not pass the required App/src/components/UserDetailsTooltip/BaseUserDetailsTooltip.web.js Lines 83 to 85 in b30b555
What changes do you think we should make in order to solve the problem?
const icons = OptionsListUtils.getAvatarsForAccountIDs([accountID], props.personalDetails, {[displayNameOrLogin]: accountID})
<UserDetailsTooltip
accountID={icons}
icon={icons[0]}
fallbackUserDetails={{
displayName: icons[0].name,
}}
> What alternative solutions did you explore? (Optional)We should check all usages of |
ProposalPlease re-state the problem that we are trying to solve in this issue.If we mention a user whom we never had a chat with before, hovering over it won't show a tooltip. What is the root cause of that problem?This is a regression from #30342. For the tooltip to show, we need to have either App/src/components/UserDetailsTooltip/BaseUserDetailsTooltip.web.js Lines 83 to 96 in 856b6e7
For a user mention, we have set a fallback display name (personal detail) that should be used if we don't have the user App/src/components/HTMLEngineProvider/HTMLRenderers/MentionUserRenderer.js Lines 75 to 80 in 856b6e7
App/src/components/UserDetailsTooltip/BaseUserDetailsTooltip.web.js Lines 22 to 23 in 856b6e7
However, after #30342, the Notice that previously, we get the display name from What changes do you think we should make in order to solve the problem?We should fall back to
|
Contributor details When tagging a non-existent user, the userinfo tool tip fails to render. This is caused by the tooltip expecting user details to exist - a hard dependency. Solution: When retrieving user data to populate the tooltip, users for whom no user data is returned ought to fall back to placeholder user info. This would solve the issue. |
📣 @mingybopeep! 📣
|
@tienifr , I don't think the I lean toward @bernhardoj's proposal, we can set the
cc @puneetlath |
@ntdiary just so I follow, are we waiting on @puneetlath's comment on the above before moving ahead? Thanks |
@laurenreidexpensify, let's wait for two days, it would be great if @puneetlath could provide some clarification. If they don't have time, we can move forward with the default contact information. :) |
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸 |
@ntdiary, @laurenreidexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@ntdiary let's move ahead 👍 |
Will check the latest code and provide final feedback tomorrow. :) |
@ntdiary @laurenreidexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
So far, I think we can move forward with @bernhardoj's proposal, it's okay to show the displayName, this is also consistent with the details page. :) 🎀 👀 🎀 C+ reviewed |
Triggered auto assignment to @bondydaa, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
📣 @ntdiary 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
📣 @bernhardoj 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
the only thing I want make sure is that |
ReportUtils.getDisplayNameForParticipant returns the user display name if found. The fallback we add will show the login. PR is ready cc: @ntdiary |
Sorry for the delay, but I agree with the logic. |
This has been on prod since 14 Dec #32632 (comment) |
@bondydaa, @ntdiary, @bernhardoj, @laurenreidexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Payment Summary: Contributor; $500 @bernhardoj paid in Upwork @ntdiary can you please complete a checklist and confirm if we need a regression test? Thanks |
Regression test steps:
Considering this is an edge regression case for PR #30342 and a suggestion in Slack regarding regression tests, I personally think it's fine to create a regression test for this issue. :) |
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: 1.4.2.0
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
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:
Action Performed:
Expected Result:
The user detail displayed inside the tooltip.
Actual Result:
Tooltip is out if hover the cursor on the mentioned user you have no conversation yet.
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6287369_1700668851348.Tooltip_is_out.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: