[HOLD for payment 2022-11-16] [$500] [BUG] - Url isn't fully visible in the tooltip reported by @thesahindia

[kavimuru]

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. Navigate to any chat
2. Send a long url (e.g. https://www.expensify.com/inbox?policyID=D189E4B5582EAE8D&taskID=AddWorkEmail)
3. Hover

Expected Result:

The whole Url should be visible

Actual Result:

The Url isn't fully visible

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• Desktop App

Version Number: 1.2.10-0
Reproducible in staging?: y
Reproducible in production?: y
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:

Expensify/Expensify Issue URL:
Issue reported by: @thesahindia
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1664484641135769

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @tjferriss (AutoAssignerTriage), see https://stackoverflow.com/c/expensify/questions/4749 for more details.

[melvin-bot]

@tjferriss Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[NikkiWines]

Hmm, this is because we have an intentional maxWidth restriction for tooltips. I'd assume we wouldn't want to have that dynamically sized based on the length of the link. That could look really weird if the link is super long...

Theoretically we could do something like a scrolling tooltip but I'm not sure it's really necessary or that it's a bug that it's cut off and doesn't scroll. cc: @shawnborton for your design expertise and since you added the original maxWidth restriction here (it's used here)

[parasharrajat]

This feels like a regression. Tooltip content should wrap in next lines after max width is reached.

[NikkiWines]

Ohh, yes, you're right @parasharrajat - looks like max lines should be 3 but it's not wrapping. That still feels (to me) like it would be bulky but I think you're right that it's a regression

[NikkiWines]

Given that, marking this as external so we can get someone working on this.

[melvin-bot]

Triggered auto assignment to @bfitzexpensify (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 - @sobitneupane (External)

[melvin-bot]

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

[shawnborton]

Personally I think it this is fine as it is with the max-width, but perhaps we should truncate via ellipsis instead of just a hard cut off.

[shawnborton]

But actually, seeing your comment above:

Ohh, yes, you're right @parasharrajat - looks like max lines should be 3 but it's not wrapping. That still feels (to me) like it would be bulky but I think you're right that it's a regression

I agree with that

[NikkiWines]

@shawnborton, to be clear, you're suggesting we keep it to a single line but truncate with an elipsis?

[shawnborton]

At first I was, but then I saw your comment that this is actually a regression and we should go back to the way it as before with the text wrap

[aimane-chnaif]

This is a regression from #10504
Reverting this PR fixes this GH but safari wrapped issue (Un / pin) still remaining

[melvin-bot]

@sobitneupane, @thienlnam, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot]

@sobitneupane, @thienlnam, @bfitzexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[sobitneupane]

@fedirjh I tend to agree with your reasoning in this proposal. Few points:

• You have pointed out that "previous value of tooltipTextWidth is being used." I can verify this happening.
• "We will need to remove the whiteSpace: 'nowrap'." I agree with you here.
• "overflowWrap: 'normal' will prevent the second line from wrapping". Can you explain, why? (as this is happening only in chrome)
• Any shortcomings from removing overflowWrap: 'normal'?

Waiting for @fedirjh response.

[fedirjh]

hey there @sobitneupane

"overflowWrap: 'normal' will prevent the second line from wrapping". Can you explain, why? (as this is happening only in chrome)

it's on Firefox too , you can reproduce with this URL test : https://www.expensify.com/inbox?policyIDD189E4B5582EAE8DtaskIDAddWorkEmailpolicyIDD189E4B5582EAE8D&taskIDAddWorkEmailpolicyIDD189E4B5582EAE8DtaskIDAddWorkEmailpolicyID=D189E4B5582EAE8DtaskIDAddWorkEmailpolicyID189E4B5582EAE8DtaskIDAddWorkEmailpolicyIDD189E4B5582EAE8DtaskIDAddWorkEmailpolicyIDD189E4B5582EAE8DtaskIDAddWorkEmail

overflowWrap: 'normal' is actually overriding the default style which is overflow-wrap: break-word; , it's added in this commit 6caff3c#diff-3c27ed5ef3408ae8e44446ed4eaddb058d5088dad2c9ae4022b318dbd0c986ae

[sobitneupane]

Proposal from @fedirjh looks good to me.

He is proposing to set tooltipTextWidth to 0 before calling updateTooltipTextWidth to solve the issue of unwanted breaking of tooltips (which change with user interaction: pin:unpin, copy to clipboard:copied) into two lines.
He is also proposing to remove overflowWrap:normal which is preventing long words to break into multiple lines( causing long urls to wrap only into two lines).

cc: @thienlnam

🎀👀🎀 C+ reviewed

[thienlnam]

Let's give it a whirl 🟢

[melvin-bot]

📣 @fedirjh You have been assigned to this job by @thienlnam!
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 📖

[melvin-bot]

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

• A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• The PR that introduced the bug has been identified. Link to the PR:
• 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:
• A discussion in #contributor-plus 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:
• Payment has been made to the issue reporter (if applicable)
• Payment has been made to the contributor that fixed the issue (if applicable)
• Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot]

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

• A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• The PR that introduced the bug has been identified. Link to the PR:
• 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:
• A discussion in #contributor-plus 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:
• Payment has been made to the issue reporter (if applicable)
• Payment has been made to the contributor that fixed the issue (if applicable)
• Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot]

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

• A regression test has been added or updated so that the same bug will not reach production again. Link to the updated test here:
• The PR that introduced the bug has been identified. Link to the PR:
• 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:
• A discussion in #contributor-plus 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:
• Payment has been made to the issue reporter (if applicable)
• Payment has been made to the contributor that fixed the issue (if applicable)
• Payment has been made to the contributor+ that helped on the issue (if applicable)

[melvin-bot]

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

• [Fix]: URL isn't fully visible in the tooltip #12408

If no regressions arise, payment will be issued on 2022-11-16. 🎊

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[bfitzexpensify]

All paid out - thanks for the work here!