[HOLD for payment 2024-05-02] [$250] Group chat - No tooltip when hovering over group member name in the chat header

[izarutskaya]

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

---

Version Number: 1.4.63-0
Reproducible in staging?: Y
Reproducible in production?: N
Found when executing PR : #40134
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Start chat.
3. Create a group chat.
4. Hover over group member name in the header.

Expected Result:

Tooltip will display.

Actual Result:

No tooltip when hovering over group member name in the group chat header.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6453372_1713429586098.20240418_163647.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01c4e26e9f3b200297
• Upwork Job ID: 1781107500795551744
• Last Price Increase: 2024-04-18
• Automatic offers:
• s77rt | Contributor | 0
• Nodebrute | Contributor | 0

[melvin-bot]

Triggered auto assignment to @bondydaa (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[melvin-bot]

Triggered auto assignment to @zanyrenney (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[izarutskaya]

@zanyrenney I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think this issue might be related to the #collect project.

[Nodebrute]

Proposal

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

No tooltip when hovering over group member name in the chat header

What is the root cause of that problem?

It's because of the shouldUseFullTitle

App/src/pages/home/HeaderView.tsx

Line 267 in 9b839f4

shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport || isGroupChat}

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

We can add below code and then use it instead of isGroupChat

 const groupChatTooltip = report?.reportName !== ''

App/src/pages/home/HeaderView.tsx

Line 267 in 9033ccb

shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport || isGroupChat}

     shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport || groupChatTooltip}

We have the option to rename the variable.

How is my proposal different?
My approach to this problem differs from the other person's. Their solution may not be effective in situations where group chats are created offline. Even if groups are created in online mode, the tooltip will not be instantly available.

Screen.Recording.2024-04-19.at.1.00.49.AM.mov

What alternative solutions did you explore? (Optional)

We should remove the ||isGroupChat from here

App/src/pages/home/HeaderView.tsx

Line 267 in 9b839f4

shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport || isGroupChat}

Result

Screen.Recording.2024-04-18.at.7.41.59.PM.mov

[allgandalf]

regression from #39757

[zanyrenney]

Thanks, yep adding external

[zanyrenney]

and adding to wave collect.

[zanyrenney]

@trjExpensify what release? I think May 1.

[allgandalf]

Proposal

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

We are not showing tooltip for report header for the very first time we create a group chat

What is the root cause of that problem?

When we create a group chat, isGroupChat Is set to true, now this will make the header a report title rather than a hoverable participant email title, because we set the following condition and isGroupChat will always be true over here which will set shouldUseFullTitle to always true:

App/src/pages/home/HeaderView.tsx

Lines 261 to 267 in 9b839f4

	<DisplayNames
	fullTitle={title}
	displayNamesWithTooltips={displayNamesWithTooltips}
	tooltipEnabled
	numberOfLines={1}
	textStyles={[styles.headerText, styles.pre]}
	shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport || isGroupChat}

This causes a regression in DisplayNames where we will always set shouldUseFullTitle to true and execute the below block of code and return :

App/src/components/DisplayNames/index.tsx

Lines 22 to 32 in 25bf0f8

	if (shouldUseFullTitle) {
	return (
	<DisplayNamesWithToolTip
	shouldUseFullTitle
	fullTitle={title}
	textStyles={textStyles}
	numberOfLines={numberOfLines}
	renderAdditionalText={renderAdditionalText}
	/>
	);
	}

The above code doesn't let us have a hoverable tooltip, because the code to show tooltip is below which is never executed for group chats:

App/src/components/DisplayNames/index.tsx

Lines 34 to 41 in 25bf0f8

	return (
	<DisplayNamesWithToolTip
	fullTitle={title}
	displayNamesWithTooltips={displayNamesWithTooltips}
	textStyles={textStyles}
	numberOfLines={numberOfLines}
	renderAdditionalText={renderAdditionalText}
	/>

Summary:

We should not show tooltips when we set group name, that is the reason why we added isGroupChat to shouldUseFullTitle but the regression it caused is that for the first time we create a group report header is set to the participants email (Which should be hoverable and contain a tooltip to display user profile). So our intent is right over here, were just need to modify the condition to set shouldUseFullTitle to true.

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

We should not directly add isGroupChat to shouldUseFullTitle instead we should create a variable shouldShowFullTitleforGroupChat which will check if reportName inside parentReport doesn't contain a Empty string, if reportName is not empty them only we should set shouldShowFullTitleforGroupChat to true, or else set it to false (This is the case where we show tooltip):

const shouldShowFullTitleforGroupChat = parentReport?.reportName !== '';


.
.
.
.
<DisplayNames
        fullTitle={title}
        displayNamesWithTooltips={displayNamesWithTooltips}
        tooltipEnabled
        numberOfLines={1}
        textStyles={[styles.headerText, styles.pre]}
        shouldUseFullTitle={isChatRoom || isPolicyExpenseChat || isChatThread || isTaskReport || shouldShowFullTitleforGroupChat}

This is because when we create a group chat parentReport.reportName is empty string and it takes a value when we set the group name:

Result Video:

Screen.Recording.2024-04-19.at.12.04.59.AM.mov

[trjExpensify]

@trjExpensify what release? I think May 1.

@zanyrenney Group Chats is #vip-split.

[marcaaron]

Removing the blocker.

[marcaaron]

@bondydaa let me know if you want me to take this off your hands as it's related to Group Chats.

[Nodebrute]

PROPOSAL UPDATED

[melvin-bot]

📣 @Nodebrute 🎉 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 📖

[marcaaron]

The bots got confused here

C+: @s77rt
Contributor: @Nodebrute

[allgandalf]

@marcaaron , they proposed differently before my solution

[allgandalf]

@marcaaron , they don't have an RCA in their proposal and also they added their current solution after I proposed mine :)

First they proposed to remove the isGroupChat check altogether :) And later when I posed my proposal ,they wrote it as it is (Without RCA) by just changing Parent Report to report. I feel it is unfair as I found the RCA and the solution first and they just modified their proposal based on my proposal :)

[marcaaron]

Sorry, assigned you to this one. Let's get it!

[allgandalf]

But isn't my proposal correct over here @marcaaron ? the other contributor used all of my details in their solution :), you can check the edit timelines as well

I really feel unfair over here as I put a lot of time curation the proposal :)

you can even double check with @s77rt on this one :)

[marcaaron]

If you feel like you have a valid claim then email [email protected]. Based on the complexity of the issue I selected the first proposal. We also have this in the guidelines:

[Nodebrute]

Hey, I've thoroughly tested my proposal multiple times to ensure it covers all cases. My proposal is distinct, and I've outlined the reasons why? Thank you for accepting the proposal. The PR will be ready soon.

[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.4.65-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• enable tooltip for group title #40528

If no regressions arise, payment will be issued on 2024-05-02. 🎊

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

• @s77rt requires payment automatic offer (Contributor)
• @Nodebrute requires payment automatic offer (Contributor)

[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:

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] 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:
• [@s77rt] 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:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] 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.
• [@zanyrenney] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

• The PR that introduced the bug has been identified: [Group Chats] Add remaining features #39757
• The offending PR has been commented on: Not needed. Both author and reviewer are assigned here
• A discussion in #expensify-bugs has been started: Not needed, unique bug
• Determine if we should create a regression test for this bug: No

[zanyrenney]

Payment Summary

@s77rt paid $250 via upwork.
@Nodebrute requires payment [automatic offer] paid $250 via upwork.

[s77rt]

Payment due here is $125 for a regression. I have refunded 50%