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

Coming from https://github.com/Expensify/App/pull/15646#discussion_r1132003674

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 image

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 image

Expensify/Expensify Issue URL: N/a Issue reported by: @tienifr Slack conversation: N/a https://github.com/Expensify/App/pull/15646#discussion_r1132003674

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

About this issue

  • Original URL
  • State: closed
  • Created a year ago
  • Comments: 63 (47 by maintainers)

Most upvoted comments

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?

@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 https://github.com/react-native-picker/picker/pull/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

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

@sophiepintoraetz - apologies, had to reassign as I am heading OOO for the next 2 weeks! At the moment, it looks like @OlimpiaZurek has created a PR.

I’ve already paid Jakub separately, so @0xmiroslav (C+) and @tienifr (reporting) will need to be paid when this hits prod.

Daily update: I created a PR with the solution in react-native-picker/picker : https://github.com/react-native-picker/picker/pull/485

Cheers @cubuspl42 - have just sent you a separate contract as it’s a one-off. I’ll get you paid once you accept.

Thanks @alex-mechler . I will start working on it on Tuesday (as Monday is Bank Holiday in Poland). I will notify you when I submit a PR in react-native-picker/picker

@alex-mechler

Thank you for your explanation and the offered compensation!

I created a PR to clarify the rules about the “Help wanted” label.

@jliexpensify

Sure!

https://www.upwork.com/freelancers/~012b13cc109598f579

@OlimpiaZurek

You might consider (in addition to submitting the upstream PR) temporarily placing the solution here, with a comment like “Remove this logic once the upstream PR <link> is merged”. That might be enough to close this issue. Otherwise we’d have to either fork react-native-picker/picker or put the issue on hold on the upstream review.

But details aside, how are we proceeding with proposal acceptance?

We talked a bit internally about this. We definitely gave you conflicting info between here and the other issue. Since this issue didn’t have Help Wanted when you posted your proposal, we are going to leave this issue in the hands of @OlimpiaZurek. Going forward, please keep proposals to issues that still have the Help Wanted label on them.

That said, you did do work on investigating this as a result of our conflicting info, so we would like to pay you $500 for your time. @jliexpensify can get you sorted here. Sorry for any confusion, and thanks for your hard work.

I’d like to take over this issue from @koko57

Can investigate this one 😃

@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 https://github.com/Expensify/App/pull/15646#discussion_r1132003674 for the discussion on that

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?

https://github.com/Expensify/App/blob/ebc03dbb14d1689e35301278833a82716ae53034/src/components/Picker/Picker.js#L194-L195

- // 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 &nbsp;, 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

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 <span> 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.