[HOLD for payment 2023-04-10] [HOLD for payment 2023-03-31] [$1000] Display name with white spaces are not removed

[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. Open the app
2. Navigate to Settings > Profile
3. Click on Display Name
4. In the First Name field enter a string separated by a bunch of spaces (eg : Daraksha Test.)
5. Save

Expected Result:

The whitespaces between 2 words should be displayed on Web

Actual Result:

On Web the extra spaces are removed. On Android and iOS apps the extra spaces remain.

Workaround:

unknown

Platforms:

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

• Web
• Desktop

Version Number: 1.2.76-3
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:

Expensify/Expensify Issue URL:
Issue reported by: @daraksha-dk
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1677157852205049
View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0127e10cabcbea08ee
• Upwork Job ID: 1629241476360613888
• Last Price Increase: 2023-02-24

[MelvinBot]

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

[MelvinBot]

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

[sonialiap]

Reproducible

1. Open the NewDot app
2. Navigate to Settings > Profile
3. Click on Display Name
4. In the First Name field enter a string separated by a bunch of spaces (eg : Sonia Test.)

Outcome:

On Android the spaces will show
On Web the spaces are removed

Android:

iOS:

Web NewDot and OldDot:

[MelvinBot]

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

[alex-mechler]

Since this doesn't happen for web / olddot, its something on the front end. Marking as external

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~0127e10cabcbea08ee

https://www.upwork.com/jobs/~016b9d862d161980f9

2023-05-09 https://www.upwork.com/jobs/~01665b3ccb31fd7e4f

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

Current assignee @alex-mechler is eligible for the External assigner, not assigning anyone new.

[allroundexperts]

Proposal

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

Extra spaces inside display name show up on native platforms.

What is the root cause of that problem?

At the point where we're saving the name in our frontend, currently we are using values.firstName.trim(). The simple trim function only works on removing the spaces at the start or end of a string. We need to use regex here such that it removes all the whitespaces except one, between words.

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

We should trim the name properly here.
Something like this will work:

values.firstName.replace(/\s+/g, " ").trim(),
values.lastName.replace(/\s+/g, " ").trim(),

If we want to just use regex and discard trim function, something like this should work:

values.firstName.replace(/^\s+|\s+$/g, "");
values.lastName.replace(/^\s+|\s+$/g, "");

What alternative solutions did you explore? (Optional)

We can also block the user from proceeding by showing him an error. This would probably be an overkill. Silently trimming the name works best I guess.

We can do the same from line breaks as well but since this is a single line input, I don't think that is required.

[Santhosh-Sellavel]

We should trim the name properly here.

How can you elaborate? @allroundexperts

[allroundexperts]

@Santhosh-Sellavel At the point where we're saving the name in our frontend, currently we are using values.firstName.trim(). The simple trim function only works on removing the spaces at the start or end of a string. We need to use regex here such that it removes all the whitespaces except one, between words.

[tienifr]

Proposal

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

The whitespace collapsing behavior in display name is inconsistent across web and Android/iOS.

First I want to clarify the expected behavior of this issue. I believe we should show the display name as is rather than removing the white spaces in between. This aligns with the behaviors of popular apps like Slack, WhatsApp. They also do not apply "smart" modification to the user's name. We already have consensus on not applying "smart" modification to the user (message) input here so I think we can apply the same for display name as well.

What is the root cause of that problem?

The root cause is in the components to render the display name like here

App/src/pages/home/report/ReportActionItemFragment.js

Line 90 in 50434b6

<Text

, we're inheriting the white-space: no-wrap from parent components, so in the Web it will collapse the consequent whitespaces into 1 whitespace.

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

To fix this, we need to apply whiteSpace: whiteSpace.preWrap, (or pre) anywhere we show the display name so that the name will be shown as is without modification.
For example, to fix the display name in the message header we can add whiteSpace: whiteSpace.preWrap, (or pre) in here

App/src/style/StyleSheet.js

Line 253 in bc4fa2b

chatItemMessageHeaderSender: {

What alternative solutions did you explore? (Optional)

If we want the display to align with web then we can add logic to make white-space: no-wrap work in native so that it will collapse the whitespace in native as well. Meaning we store the display name as is (without applying "smart" modification) but will display it consistently across web & native.

[tienifr]

I attached a screenshot of how Slack handles it below for reference (WhatsApp is the same):

[allroundexperts]

This behaviour isn't consistent with Slack app as well. At some places, it shows the display name like @tienifr has posted. But at other places, it trims the extra whitespaces. I've attached the screenshots.

When the profile is opened by clicking your name on the top, then again it is trimming the whitespaces.

On mobile though, slack shows with whitespaces everywhere. The behaviour seems to be different on desktop and web version of Slack.

I don't think that we can compare this with WhatsApp because the former allows almost any character to be used in display name eg ~~~!a is a valid name in WhatsApp.

On Slack, using any special character in display name results in an error.

If we decide to remove the extra whitespaces, then using white-space might not be a good approach. This is because we'll have to use white-space everywhere we are displaying the name. In future, it will be highly likely that someone will miss this property and the behaviour will become inconsistent again.

[s77rt]

Proposal

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

On web, multiple white spaces are replaced by one.

What is the root cause of that problem?

This is a bug in RNW where it applies style whiteSpace: 'nowrap' here.

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

Instead of nowrap use pre i.e. whiteSpace: 'pre'.

What alternative solutions did you explore? (Optional)

• If we want to prevent users from having multiple white spaces in their names, we must do so in the backend.

[Santhosh-Sellavel]

As of now, we can clearly see the bug is on the web which doesn't show the space which should be there.

@alex-mechler I'm unsure what's the expected behavior here. Let me know your thoughts here.

[MelvinBot]

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

• fix 15438 maintain white space in display name #15646

If no regressions arise, payment will be issued on 2023-03-31. 🎊

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

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

[MelvinBot]

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:

• [@alex-mechler] The PR that introduced the bug has been identified. Link to the PR: Last change for this was 2 years ago, when we were at the very early stages of RNC. This has likely existed well before then
• [@alex-mechler] 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
• [@alex-mechler] 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: https://expensify.slack.com/archives/C049HHMV9SM/p1680188592197909
• [@sonialiap] Determine if we should create a regression test for this bug.
• [@Santhosh-Sellavel] 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.
• [@sonialiap] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[sobitneupane]

#16524 is a regression caused by PR for this issue.

cc: @tienifr @Santhosh-Sellavel @alex-mechler

[Santhosh-Sellavel]

@alex-mechler please have a look at this when you have time.

[Santhosh-Sellavel]

@alex-mechler Checklist here #15438 (comment) doesn't makes sense

[@alex-mechler] The PR that introduced the bug has been identified. Link to the PR: #15646

This PR Linked is the PR for this BUG. The actual Bug here is not caused by any PR it was something upstream.

[alex-mechler]

Oh whoops, was filling out the BZ checklist and moving a bit too fast. Thanks for the ping. Will fix it up

[MelvinBot]

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

[MelvinBot]

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

• fix/16524: "Most recent" description in Spanish not in multiple lines and its truncated at the end #16657

If no regressions arise, payment will be issued on 2023-04-10. 🎊

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

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

[alex-mechler]

BZ checklist was here #15438 (comment), that one is probably from one of the regressions

[alex-mechler]

@sonialiap Can you finish off the BZ checklist here, and make sure this was paid out? #15438 (comment)

[sonialiap]

My apologies for the delay in payment! I was out of office. On it now

@daraksha-dk offer sent for reporting the issue - paid
@tienifr offer sent for fixing the issue - paid
@Santhosh-Sellavel offer sent for C+ reviewing the issue (minus regression) - paid

[daraksha-dk]

@sonialiap Sorry but I'm unable to find any offer for this issue on Upwork. Can you please re-check. Thanks!

[alex-mechler]

@sonialiap Is this all paid out?

[sonialiap]

@daraksha-dk offer re-sent

This is your profile, correct?

[daraksha-dk]

@sonialiap yes, it's mine and I've accepted the offer now. Thank you.

[sonialiap]

@daraksha-dk paid ✔️