[HOLD for payment 2023-08-07] [$1000] App only displays tooltip on long reply in thread message from concierge

[kavimuru]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the app
2. Search for new user eg: [email protected]
3. Concierge will display 'first time chatting' message, hover on it and click on reply in thread
4. In LHN, hover on three dots on the reply in thread report of concierge and observe that it displays the complete message in tooltip
5. Open any other report and send long text
6. Click on reply in thread on long text
7. In LHN, hover on three dots on the reply in thread report of long text and observe that app does not display any tooltip

Expected Result:

App should display tooltip (to display full message) on hover over three dots in LHN for long text reply in thread report as it does for concierge reply in thread report

Actual Result:

App does not display tooltip (to display full message) on hover over three dots in LHN for long text reply in thread report

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.39-5
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

No.tooltip.on.long.text.3.dots.LHN.mp4

Recording.1244.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1689012283926099

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01019167e09bdf2205
• Upwork Job ID: 1678698491007807488
2023-07-11
• Automatic offers:
• | | 0
• StevenKKC | Contributor | 25729137
• dhanashree-sawant | Reporter | 25729138

[melvin-bot]

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

[StevenKKC]

Proposal

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

App only displays tooltip on long reply in thread message from concierge.

What is the root cause of that problem?

When hover over three dots in LHN, the report title is displayed by below code.

App/src/components/LHNOptionsList/OptionRowLHN.js

Lines 191 to 201 in 882026d

	<DisplayNames
	accessibilityLabel={props.translate('accessibilityHints.chatUserDisplayNames')}
	fullTitle={optionItem.text}
	displayNamesWithTooltips={optionItem.displayNamesWithTooltips}
	tooltipEnabled
	numberOfLines={1}
	textStyles={displayNameStyle}
	shouldUseFullTitle={
	optionItem.isChatRoom || optionItem.isPolicyExpenseChat || optionItem.isTaskReport || optionItem.isThread || optionItem.isMoneyRequestReport
	}
	/>

And tooltip is displayed only if displayNamesWithTooltips.length > 1.

App/src/components/DisplayNames/index.js

Lines 114 to 121 in 882026d

	{this.props.displayNamesWithTooltips.length > 1 && Boolean(this.state.isEllipsisActive) && (
	<View style={styles.displayNameTooltipEllipsis}>
	<Tooltip text={this.props.fullTitle}>
	{/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */}
	<Text>....</Text>
	</Tooltip>
	</View>
	)}

displayNamesWithTooltips is generated as below.

App/src/libs/SidebarUtils.js

Lines 299 to 300 in 882026d

	// We only create tooltips for the first 10 users or so since some reports have hundreds of users, causing performance to degrade.
	const displayNamesWithTooltips = ReportUtils.getDisplayNamesWithTooltips((participantPersonalDetailList || []).slice(0, 10), hasMultipleParticipants);

That is, displayNamesWithTooltips is generated for first 10 participants.
In the reply in thread report of concierge message, the participants includes concierge and the current user, so displayNamesWithTooltips.length > 1 will be true, and then tooltip is displayed.
In the reply in thread report of long text, the participants only include the user with whom the current user is interacting, so tooltip is not displayed.

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

From the user's point of view, "..." means that the title is long and cannot be displayed on one line, so there is an action to display additional information.
So we can remove checking displayNamesWithTooltips.length as below in order to display the tooltip.

-   {this.props.displayNamesWithTooltips.length > 1 && Boolean(this.state.isEllipsisActive) && (
+   {Boolean(this.state.isEllipsisActive) && (
        <View style={styles.displayNameTooltipEllipsis}>
            <Tooltip text={this.props.fullTitle}>
                {/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */}
                <Text>....</Text>
            </Tooltip>
        </View>
    )}

What alternative solutions did you explore? (Optional)

None.

[bernhardoj]

Proposal

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

A thread that has Concierge as the participant shows a tooltip when hovered over the ellipsis if the LHN title is very long.

What is the root cause of that problem?

The LHN use DisplayNames to show the title. If displayNamesWithTooltips length > 1 and the text is overflowing, we will show an ellipsis that has a tooltip. displayNamesWithTooltips is constructed from the report participants and the tooltip title is the same as the LHN title. In short, we show the ellipsis and tooltip if participants are more than 1 and the text can't fit in one line. This means this issue could happen on every report that has multiple participants with a very long report name.

App/src/components/DisplayNames/index.js

Lines 114 to 121 in e299de5

	{this.props.displayNamesWithTooltips.length > 1 && Boolean(this.state.isEllipsisActive) && (
	<View style={styles.displayNameTooltipEllipsis}>
	<Tooltip text={this.props.fullTitle}>
	{/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */}
	<Text>....</Text>
	</Tooltip>
	</View>
	)}

The ellipsis and tooltip are useful for a group chat where the participant list does not fit in one line.

But in the case of a thread, the LHN title is not the participant's name/email, but the thread parent action text. How? We have a shouldUseFullTitle prop, that shows the report name instead of showing the participant list as the title.

App/src/components/LHNOptionsList/OptionRowLHN.js

Lines 198 to 200 in e299de5

	shouldUseFullTitle={
	optionItem.isChatRoom || optionItem.isPolicyExpenseChat || optionItem.isTaskReport || optionItem.isThread || optionItem.isMoneyRequestReport
	}

Notice that it's not only thread but also other types of report.

It doesn't make sense for me to show the tooltip in a thread (or other types of report) when there is more than one participant, but not when we are the only participant.

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

The tooltip is designed to show the rest of the group participants, in case shouldUseFullTitle is true, don't show the tooltip.

We have 2 options:

1. Add !shouldUseFullTitle here
   

   App/src/components/DisplayNames/index.js

   Lines 114 to 120 in e299de5

   	{this.props.displayNamesWithTooltips.length > 1 && Boolean(this.state.isEllipsisActive) && (
   	<View style={styles.displayNameTooltipEllipsis}>
   	<Tooltip text={this.props.fullTitle}>
   	{/* There is some Gap for real ellipsis so we are adding 4 `.` to cover */}
   	<Text>....</Text>
   	</Tooltip>
   	</View>

OR

2. Early return here when shouldUseFullTitle is true
   

   App/src/components/DisplayNames/index.js

   Lines 68 to 78 in e299de5

   	if (!this.props.tooltipEnabled) {
   	// No need for any complex text-splitting, just return a simple Text component
   	return (
   	<Text
   	style={[...this.props.textStyles, this.props.numberOfLines === 1 ? styles.pre : styles.preWrap]}
   	numberOfLines={this.props.numberOfLines}
   	>
   	{this.props.fullTitle}
   	</Text>
   	);
   	}

We can then remove the fullTitle from here

App/src/components/DisplayNames/index.js

Lines 88 to 89 in e299de5

	{this.props.shouldUseFullTitle
	? this.props.fullTitle

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01019167e09bdf2205

[melvin-bot]

Current assignee @bfitzexpensify is eligible for the External assigner, not assigning anyone new.

[melvin-bot]

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

[Skalakid]

Proposal

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

App does not display tooltip (to display full message) on hover over three dots in LHN for long text reply in thread report

What is the root cause of that problem?

The problem is with the condition that defines if the tooltip should render:

 {this.props.displayNamesWithTooltips.length > 1 && Boolean(this.state.isEllipsisActive) && (
 ...

As we can see we check if displayNamesWithTooltips array has more than 1 item. But what if we have 1 item but with a really long display name? We don't cover this case.

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

In my opinion, we should change the mentioned condition in DisplayNames/index.js to:

 {this.props.displayNamesWithTooltips.length > 1 || Boolean(this.state.isEllipsisActive) && (
 ...

Changing && to || will make this condition more flexible and will fix the problem

[allroundexperts]

I'll be taking this over.

[thesahindia]

@bfitzexpensify, please assign @allroundexperts for the c+ role.

[melvin-bot]

📣 @allroundexperts 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Upwork job
Please accept the offer 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]

📣 @dhanashree! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[melvin-bot]

The BZ member will need to manually hire dhanashree for the Reporter role. Please store your Upwork details and apply to our Upwork job so this process is automatic in the future!

[allroundexperts]

Still reviewing the proposals.

[allroundexperts]

Okay. I guess we have proposals that are taking into consideration different expected results.

@bfitzexpensify Can we please clarify the expected behaviour here first?

Specifically, should we show the LHN tooltip only for group chats (where the title is participant names)? Or should we be showing it for all chats?

[bfitzexpensify]

I think it makes sense to have the LHN tooltip for all chats where we can't show the full message in the LHN

[bfitzexpensify]

@allroundexperts given that, do we have a proposal that fits best?

[melvin-bot]

📣 @StevenKKC 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer 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]

📣 @dhanashree-sawant 🎉 An offer has been automatically sent to your Upwork account for the Reporter role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[StevenKKC]

@allroundexperts PR is ready for review.

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

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

• fix display tooltip on long title in LHN #23533

If no regressions arise, payment will be issued on 2023-08-07. 🎊

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.

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

For reference, here are some details about the assignees on this issue:

• @allroundexperts does not require payment (Eligable for Manual Requests)
• @StevenKKC requires payment offer (Contributor)

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]

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:

• [@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
• [@allroundexperts] 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:
• [@allroundexperts] 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:
• [@allroundexperts] Determine if we should create a regression test for this bug.
• [@allroundexperts] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@bfitzexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[bfitzexpensify]

Job assigned July 25th, merged July 26th — eligible for #urgency bonus

Amounts owed here for payment:

Reporting: @dhanashree-sawant - $250 - paid out via Upwork ✅
Contributor: @StevenKKC - $1500 (contributor + #urgency bonus) - paid out via Upwork ✅
C+: @allroundexperts - $1500 (C+ & #urgency bonus) - to be paid via manual request

[bfitzexpensify]

Bump to complete BZ checklist @allroundexperts - thank you!

[NikkiWines]

@allroundexperts please complete the following checklist 🙇

[allroundexperts]

Sure, on it now!

[allroundexperts]

Checklist

1. I think there isn't a single PR to blame here. The condition we removed existed ever since we created the DisplayNames component. I think this issue surfaced after threads were added.
2. N/A
3. I'm not sure if there is anything more we can add to checklist to prevent these type of issues. I think eagle 👀 are needed to catch this.
4. Regression test might be helpful.

Regression test

1. Open any report and send a long message (more than 2 lines).
2. Start a thread from the sent message.
3. In LHN, hover on three dots on the message of the thread report.
4. Verify that the app displays a tooltip showing the entire sent message.

Do we 👍 or 👎 ?

[JmillsExpensify]

Reviewed the details for @allroundexperts. Approved for payment in NewDot based on the BZ payment summary above.

[bfitzexpensify]

Open any report and send a long message (more than 2 lines).
Start a thread from the sent message.
In LHN, hover on three dots on the message of the thread report.
Verify that the app displays a tooltip showing the entire sent message.

I like this, simple and effective. I've opened https://github.com/Expensify/Expensify/issues/308796 to add this as a regression test.

All payments now complete, as is the BZ checklist, so we're all done here, closing this out. Thanks everyone!