[$500] Chat - Link in end of line displays tooltip over text and not on link

[lanitochka17]

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. Open the app
2. Open any report
3. Type in some text and at the end of line, add link with long text eg: Test test test test test test test test test test
4. Send the text, text should have text and link in first line and link should be continued in second line too
5. Hover on link and observe that tooltip is displayed over text

Expected Result:

App should display tooltip over link on hover

Actual Result:

App displays tooltip over text on link hover when link is multiline and occupies the end of line with text in start

Workaround:

Unknown

Platforms:

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

• Android / native
• Android / Chrome
• iOS / native
• iOS / Safari
• MacOS / Chrome / Safari
• MacOS / Desktop

Version Number: 1.3.70-5

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

link.in.end.text.displays.link.over.text.mp4

Recording.29.2.mp4

Expensify/Expensify Issue URL:

Issue reported by: @dhanashree-sawant

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1694604089052599

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01aa77f02fd5409216
• Upwork Job ID: 1703100827931217920
• Last Price Increase: 2023-09-16
• Automatic offers:
• s77rt | Reviewer | 26768759
• Dhanashree-Sawant | Reporter | 26768761

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01aa77f02fd5409216

[melvin-bot]

Triggered auto assignment to @sophiepintoraetz (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[melvin-bot]

Triggered auto assignment to @isabelastisser (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt (External)

[ewanmellor]

Proposal

Please re-state the problem that we are trying to solve in this issue.

When a link is wrapped onto multiple lines in a chat window, and the user hovers over the link, the tooltip appears in the wrong place.

What is the root cause of that problem?

The problem is that the Tooltip component places the tooltip over the bounding box of the element that is being hovered. For a link that has wrapped onto two lines, the bounding box encloses the entire two lines, and so it looks like the tooltip is being placed over the paragraph of text instead of the link.

What changes do you think we should make in order to solve the problem?

From the user's perspective, you want the tooltip to target the link, but since the link is now in two parts (one at the end of the first line, one at the start of the second line) you want the tooltip to target the center of whichever part of the link the user is hovering over.

It is possible to get the browser to give you the bounding box of each part of the text. However, Tooltip uses @react-ng/bounds-observer to get the element bounding box, and doesn't do anything to position the tooltip when the user hovers. We would need to extend Tooltip to reposition the tooltip in the onHoverIn handler, depending on which part of the link the user is hovering over.

Algorithm outline for onHoverIn handler:

1. Use Element. getClientRects to get the bounding boxes for each partial target.
2. Use the mouse position from the onHoverIn event to determine which partial target the user is hovering over.
3. Set the tooltip position to center over that partial target.
4. Fall back to the current algorithm if necessary, though getClientRects is well supported so it should not be a problem.

This algorithm will add a bit of cost to the onHoverIn event, so I'd add a prop to Tooltip to make it opt-in.

[s77rt]

@ewanmellor Thanks for the proposal. Your RCA is correct. In our App we seem to always position the tooltip on the center of the target regardless of the mouse position. I noticed that Slack do things a little different, they position the tooltip on the center of the partial target, the partial target being the part of the target that the mouse is at. Your suggested solution seems to help towards that approach but it's not much clear. Can you please elaborate your proposal on how the algorithm would work?

[ewanmellor]

@s77rt Yes, that's what I meant.

1. Use Element. getClientRects to get the bounding boxes for each partial target.
2. Use the mouse position from the onHoverIn event to determine which partial target the user is hovering over.
3. Set the tooltip position to center over that partial target.
4. Fall back to the current algorithm if necessary, though getClientRects is well supported so it should not be a problem.

[s77rt]

@ewanmellor Thanks for the update. That makes sense, can you please update your proposal to include that?

BTW, do you have a proof of concept or did you manage to get this working? not really necessary, just it would help avoid deadends

[ewanmellor]

@s77rt I haven't written the code yet, no. I am waiting for approval before I spend the time.

[ewanmellor]

@s77rt Proposal updated: #27585 (comment)

[s77rt]

@ewanmellor Thanks for the update!

🎀 👀 🎀 C+ reviewed
Link to proposal

[melvin-bot]

Triggered auto assignment to @pecanoro, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[pecanoro]

Assigning @ewanmellor, let's see if we can fix this one!

[sophiepintoraetz]

Hey @kadiealexander, just need some help babysitting this issue while I'm OOO till 15 Oct:

• We had a contributor assigned and then they silently ghosted the issue, so we're looking for "proposals" but really, anyone could implement the proposal that was approved.
• I've asked Callstack to see if they have capacity 🤞 - in which case, only @s77rt would need payment, as I have already paid the bug reporter @dhanashree-sawant.
• If they don't have capacity, I was going to try Software mansion next!

[teneeto]

Hi, I'm Eto from Callstack - expert contributor group - and I would like to take a look at this issue.

[melvin-bot]

@pecanoro @s77rt @teneeto @sophiepintoraetz @kadiealexander this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[s77rt]

Not overdue. We are working on it Melvin

[teneeto]

Please Let's continue Review of the Solution to this Issue with new Information.

• New PR: fix chat-link in end of line displays tooltip over text and not on link #29134
• Leading PR: Fix inconsistent margins for reacted emojis to different sort of comments #22010

Note: It would be nice to assign PR to same reviewer, then continue discussions on the new PR.

[s77rt]

Note: We had a regression #29446.

[s77rt]

The PR fixing this is merged and deployed to production. Melvin forgot to remove the Reviewing label here

[s77rt]

And another one 😅 #29678

[pecanoro]

@s77rt Do you know why it created an additional issue for payment instead of updating this one? Probably the issue wasn't linked properly in the PR description.

[pecanoro]

@sophiepintoraetz We only need to pay the reporting bonus to @dhanashree-sawant since @s77rt was paid in the other issue.

[sophiepintoraetz]

I've already paid @dhanashree-sawant for this issue, so if that's all that we're waiting on, we can close this issue?

[pecanoro]

Yes! We can close it then, thank you!