[HOLD for payment 2023-04-26] [$2000] New Room Dropdown does not properly reflect multiple spaces in the New Room Modal

[alex-mechler]

Coming from #15646 (comment)

Action Performed:

1. Create a new workspace
2. Update the name to have multiple consecutive spaces
3. Click the FAB
4. Select New Room
5. Observe the workspace name in the modal/dropdown doesn't show multiple spaces

Expected Result:

The displayed name should show multiple spaces, same as elsewhere

Actual Result:

The display name shows a single space

Workaround:

N/A, its simply a display issue

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: v1.2.84-1 DEV
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): N/a
Logs: https://stackoverflow.com/c/expensify/questions/4856 N/a
Notes/Photos/Videos: Any additional supporting documentation

Expensify/Expensify Issue URL: N/a
Issue reported by: @tienifr
Slack conversation: N/a #15646 (comment)

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018f5eeb68bbcf95bc
• Upwork Job ID: 1638322484300886016
• Last Price Increase: 2023-03-28

[MelvinBot]

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

[jliexpensify]

@alex-mechler I wanted to ask about this as I'm on v1.2.87-0. This is what I see when I input multiple spaces between words:

I also don't have a DEV environment - can I actually test this?

[alex-mechler]

When #15646 is actually deployed to staging, you should see multiple spaces in the workspace name. I created this issue because we know we needed the follow up after the pr was merged.

With the next app deploy you should be able to test this on staging

[jliexpensify]

Awesome, thanks for the context - will check it out once it's deployed!

[jliexpensify]

Ok, I'm on Staging v88 and it's still happening:

[MelvinBot]

Job added to Upwork: https://www.upwork.com/jobs/~018f5eeb68bbcf95bc

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

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

[dayyad]

Proposal

Problem Statement

In the current React Native application, when a user updates the workspace name with multiple consecutive spaces and opens the modal/dropdown for creating a new room, the workspace name is displayed with only a single space instead of the intended multiple consecutive spaces.
Root Cause

The root cause of this problem is that HTML automatically collapses multiple consecutive spaces into a single space when rendering text. As a result, the workspace name with multiple spaces is not being displayed as intended in the modal/dropdown.

Proposed Solution

To solve this issue, we should wrap the workspace name text in a element and apply the white-space: pre-wrap style. This style will preserve the white space in the text, including multiple consecutive spaces.

Add a new style to the component in the file where the workspace name is displayed in the modal/dropdown

const styles = StyleSheet.create({
    // ...
    workspaceName: {
        whiteSpace: 'pre-wrap',
    },
});

Wrap the workspace name text in a <span> element and apply the new style:

<Text style={styles.workspaceName}>
    <span style={styles.workspaceName}>{this.props.workspaceName}</span>
</Text>

This solution ensures that the workspace name is displayed with multiple consecutive spaces as intended in the modal/dropdown.
Alternative Solutions (Optional)

An alternative solution could involve using non-breaking spaces ( ) to replace multiple consecutive spaces. However, this approach requires additional processing of the input text and might introduce unnecessary complexity to the codebase.

[hellohublot]

Proposal

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

New Room Dropdown does not properly reflect multiple spaces in the New Room Modal

What is the root cause of that problem?

select.option will filter consecutive spaces

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

App/src/components/Picker/Picker.js

Lines 194 to 195 in ebc03db

	// We add a text color to prevent white text on white background dropdown items on Windows
	items={_.map(this.props.items, item => ({...item, color: themeColors.pickerOptionsTextColor}))}

- // We add a text color to prevent white text on white background dropdown items on Windows
- items={_.map(this.props.items, item => ({...item, color: themeColors.pickerOptionsTextColor}))}
+ items={_.map(this.props.items, item => ({
+     ...item,
+     // We add a text color to prevent white text on white background dropdown items on Windows
+     color: themeColors. pickerOptionsTextColor,

+     // We replace normal space to unicode consecutive space to display consecutive space in browser, it's also support native parse
+     label: item.label.replace(/ /g, '\u00A0')
+ }))}

We replace it with  , of course we need to use its unicode character \u00A0
If necessary, it can be written into CONST.js or expensify-common

I tried ['\n', '\t', '\r', ';', '<', '&'], but I have not found that other characters need to be replaced.
It works fine on the mobile native platform as it's a unicode space

What alternative solutions did you explore? (Optional)

not yet

[0xmiros]

2 proposals in review

[alex-mechler]

@hellohublot That was a solution we tried in the original issue. It did work, but we removed it because it felt rather hacky. We would like to use a less hacky solution if possible. See #15646 (comment) for the discussion on that

[0xmiros]

@dayyad thanks for your proposal but root cause is not correct. Solution is also not considerable.

@hellohublot same feedback as @alex-mechler

Looking for proposals with non-hacky solution...

[OlimpiaZurek]

I've added a temporary solution to the upstream PR here: Expensify/react-native-picker-select#9

@alex-mechler Let me know if this is enough to close this issue or should we wait for the PR at @react-native-picker/picker to be merged. Thanks!

[alex-mechler]

Requested some changes to the temporary solution. Once we get that merged, can you also make the required changes in the App repo? Then we can close this out

[alex-mechler]

I've merged the temporary solution PR. Can you spin up a App PR to update the package we are using?

[OlimpiaZurek]

@alex-mechler Thank you. I just opened a PR #17456 .

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

• update react native picker select #17456

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

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:

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

[sophiepintoraetz]

Summarising payments here:
@tienifr : $250 for reporting
@OlimpiaZurek / @0xmiroslav : I'll need your help in summarising what's due here, given there was a period of time between contributor assignment and when the PR was merged?)

(also @alex-mechler if you have any thoughts on this one, seeing as I'm taking over from JLi)

Also, want to make sure there's nothing happening with this PR here that is still open?

[alex-mechler]

@OlimpiaZurek / @0xmiroslav : I'll need your help in summarising what's due here, given there was a period of time between contributor assignment and when the PR was merged?)

@OlimpiaZurek is from CallStack, so they will be paid through that https://stackoverflowteams.com/c/expensify/questions/15868/15869#15869

Also, want to make sure there's nothing happening with this react-native-picker/picker#485 that is still open?

That's an upstream PR where we are held on the maintainer of that library reviewing that pr. We have fixed the issue in Expensify's code, so we should not be waiting on that

[sophiepintoraetz]

Cool - Do we have a shortlist somewhere of who is with Callstack? I'll still need help with @0xmiroslav's payment though, @alex-mechler 🙏

[0xmiros]

https://docs.google.com/document/d/1BvohU05MTaHnjOD_vwJv_aqDAirv-ChkyRnKCAvOVyQ/edit#heading=h.et1hcenskp8
@sophiepintoraetz you can find process here - Step 5.
Also, slack discussion: https://expensify.slack.com/archives/C02NK2DQWUX/p1679431620069529?thread_ts=1679408974.051529&cid=C02NK2DQWUX

[sophiepintoraetz]

Okay so @0xmiroslav - you're being compensated $2000? What I'm questioning is the bonus timeframe - are we saying this does not apply because a Callstack engineer created a PR?
Based on Agency-employee submits PR and C+ is assigned to review. C+ is compensated for the job amount.

[0xmiros]

I haven't found any discussion related to timeline bonus/penalty when agency assigned, but I believe it's flatten fee regardless of PR merge timeline.
@mallenexpensify can you please help here?

[mallenexpensify]

Agency-jobs are eligible for timeline bonuses. The only part that's different than other issues is we don't pay agency-contributors via Upwork, most-everything else should be the same (well... the other difference is agency-contributors don't need to submit a proposal before being hired either (they do before submitting a PR though)).

[sophiepintoraetz]

Okay I'm going to leave the payment as is for now, (no bonus but no penalty either) as it's not clear when the first PR was merged vs contributor assigned.

[sophiepintoraetz]

Offer sent, @0xmiroslav!

[0xmiros]

@sophiepintoraetz accepted offer. thanks