[PAY 15th SEP][$250] Member names in member selection views are not bold or using correct text color and styles

[m-natarajan]

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:
Reproducible in staging?: Needs reproduction
Reproducible in production?: Needs reproduction
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: @shawnborton
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1724943353451769

Action Performed:

1. Go to staging.new.expensify.com
2. Initiate an IOU

Expected Result:

Member names in member selection views are bold and using correct text color and styles

Actual Result:

Member names in member selection views are not bold or using correct text color and styles

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

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0159f84c9f700169cc
• Upwork Job ID: 1830998979182407156
• Last Price Increase: 2024-09-03

Issue Owner

Current Issue Owner: @jayeshmangwani

[melvin-bot]

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

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[FitseTLT]

@shawnborton the option will be bold when the report linked to it is unread. This is intentional. If I have understood the issue. 🤷

[bernhardoj]

Proposal

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

The money request participant title isn't using bold style.

What is the root cause of that problem?

The title text will be bold if the isBold property of the item is not false.

App/src/components/SelectionList/InviteMemberListItem.tsx

Line 101 in 117b961

item.isBold !== false && styles.sidebarLinkTextBold,

The isBold logic was first added in #37306 and we want all list by default has a bold text, except for LHN and chat finder page where it will be bold based on the unread status.

Previously, isBold was only added to ChatFinderPage list, but now, we added it to all lists by adding it in getOptions.

App/src/libs/OptionsListUtils.ts

Line 1968 in 117b961

reportOption.isBold = shouldUseBoldText(reportOption);

So, all user lists will follow the LHN unread logic to bold the title, including personal details which don't have an unread status, so it will always be false.

App/src/libs/OptionsListUtils.ts

Line 1992 in 117b961

personalDetailOption.isBold = shouldUseBoldText(personalDetailOption);

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

We can add a new param to getOptions called shouldBoldTitleByDefault which has a default value of true. Then, pass it as false from getSearchOptions.

The new isBold condition will be.

reportOption.isBold = shouldBoldTitleByDefault || shouldUseBoldText(reportOption);

We can do the same for the personal details, but since shouldUseBoldText is always false for personal detail, we can just simplify the condition to:

App/src/libs/OptionsListUtils.ts

Line 1992 in 117b961

personalDetailOption.isBold = shouldUseBoldText(personalDetailOption);

personalDetailOption.isBold = shouldBoldTitleByDefault;

Or just remove it so personal detail will always be in bold just like before in #37306, even in chat finder page.

[shawnborton]

the option will be bold when the report linked to it is unread. This is intentional. If I have understood the issue. 🤷

I think this is a bug. We only want the read/unread styles to be applied when you use the Find chat page. Otherwise any time we just list out members/contacts, the names should be bold and in our normal text color. cc @Expensify/design @trjExpensify @JmillsExpensify for the gut check there.

[dannymcclain]

@shawnborton definitely agree.

[trjExpensify]

We only want the read/unread styles to be applied when you use the Find chat page

why do I have a recollection that we decided to not use bold for unread at all on this page, did we walk back from that? 🤔

[shawnborton]

I feel like I recall a similar conversation but I had the same exact opinion/feedback as I do today and I could have sworn we implemented it correctly, hence why I was pretty convinced this was a regression.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0159f84c9f700169cc

[melvin-bot]

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

[jayeshmangwani]

@bernhardoj since we determine the bold text status from the shouldUseBoldText fucntion's output, what do you think about making default bold text changes to the shouldUseBoldText function?

[bernhardoj]

Hmm, I think it's a bit weird to see the function accepting a boolean just to return the boolean.

function shouldUseBoldText(report, shouldBoldTitleByDefault) {
    return shouldBoldTitleByDefault || (report.isUnread === true && report.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.MUTE);
}

shouldUseBoldText(report, shouldBoldTitleByDefault);

I prefer to keep shouldBoldTitleByDefault outside the function.

[jayeshmangwani]

I prefer to keep shouldBoldTitleByDefault outside the function.

Thanks for the explanation. I was thinking about using it in the function itself, but I also think we can keep the default bold parameter outside.

[jayeshmangwani]

We can go with @bernhardoj 's Proposal of keeping bold text default to true and making it false for search options using the new parameter.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[bernhardoj]

PR is ready

cc: @jayeshmangwani

[bfitzexpensify]

I am heading out of office until September 21st, so assigning a buddy to watch over this in my absence.

Current status: PR in review

[melvin-bot]

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

[melvin-bot]

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

[jayeshmangwani]

@jliexpensify Automation failed here; PR was deployed to production 4 days ago with the following checklist #48664

[jliexpensify]

Thanks @jayeshmangwani - will note it down!

[jliexpensify]

Payment Summary

• C: @bernhardoj $250 (ND payment)
• C+: @jayeshmangwani $250 (ND payment)

Is a checklist needed?

[jayeshmangwani]

Is a checklist needed?

Yes, I'll complete the checklist today

[bernhardoj]

Requested in ND.

[jayeshmangwani]

• [@jayeshmangwani] The PR that introduced the bug has been identified. Link to the PR: When we created the bold logic for the chat list in this PR Only bold user display name when it's unread in search page #37306, we missed the case where the name titles should be bold for all reports in the submit expense flow. In that PR, we only considered the search page. So, I would say this is not a regression but rather an enhancement on top of the existing logic for displaying bold member names
• [@jayeshmangwani] 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: N/A
• [@jayeshmangwani] 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: This is purely a visual change, so I don't think a Slack discussion or a new checklist is necessary.
• [@jayeshmangwani] Determine if we should create a regression test for this bug. Yes
• [@jayeshmangwani] 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.

Regression Test Proposal

1. Open App, Press FAB and select Submit Expense.
2. Enter any amount and proceed to the next step.
3. Verify that all member names in the member selection view are displayed in bold.

Do we agree 👍 or 👎

[jliexpensify]

Cheers, closing!

[jayeshmangwani]

Requested as per #48364 (comment)

[JmillsExpensify]

$250 approved for @bernhardoj

[garrettmknight]

$250 approved for @jayeshmangwani