[$500] Display name fields aren't sanitized (remove blank space) after lose focus

[m-natarajan]

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. Go to https://staging.new.expensify.com/settings/profile/display-name
2. Type several blank space before name
3. Click in another input
4. You will see that the input is not changed (sanitized)
5. When you save and open again it would be sanitized

   Expected Result:

   input should remove any leading spaces (sanitization) before save to show user how it would be saved.

   Actual Result:

   The actual result is that the leading spaces are kept, and it is only removed when we save the inputs and open the edit page again, could be confuse to user.

Workaround:

unknown

Platforms:

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

• [x] Android / native
• [x] Android / Chrome
• [x] iOS / native
• [x] iOS / Safari
• [x] MacOS / Chrome / Safari
• [x] MacOS / Desktop

Version Number: 1.3.78-2 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: Any additional supporting documentation

https://github.com/Expensify/App/assets/38435837/dec1115f-bd50-4ef9-9e63-0eeafc488d63

https://github.com/Expensify/App/assets/38435837/c4a2f956-ca98-4121-9f12-3a5c560cdca0

https://github.com/Expensify/App/assets/38435837/6c3fc83a-1dc3-46a1-89e3-9b72117bcd19

https://github.com/Expensify/App/assets/38435837/008db683-4a4f-4ca9-8b3d-2167a94b664a

https://github.com/Expensify/App/assets/38435837/ffc0a9f2-02c7-4dcc-8d01-f21c48a2d5b3

https://github.com/Expensify/App/assets/38435837/1f1f9448-f4c0-41c3-a93d-a7a87aa4261c

https://github.com/Expensify/App/assets/38435837/09d6263c-c54a-4961-91ef-1bed76e5529d

Expensify/Expensify Issue URL: Issue reported by: @lcsvcn Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696436594718049

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01f6ec7bdd9f09b65b
• Upwork Job ID: 1709942839833063424
• Last Price Increase: 2023-10-19
• Automatic offers:
• shubham1206agra | Contributor | 27330339

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~01f6ec7bdd9f09b65b

[melvin-bot[bot]]

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

[m-natarajan]

Proposal by @lcsvcn

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

Display name fields aren't sanitized (remove blank space) after lose focus

What is the root cause of that problem?

The root cause of the problem is that we are not sanitizing the field at the input loses focus, that way the input looks different then when it is saved.

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

We can avoid that bug with a simple solution, by setting the onBlur variable for the TextInput field. So it could trim any whitespace and do any sanitization we want between the actual input, so any white space before or after text gets removed after user input. Below is an example on how to add to remove any space before or after the first name and last name input fields. We need to add that to both TextInput components. So that way when user finishes typing and the input loses focus, it will automatically remove any leading spaces, showing similar to how it is shown when user saves and open again. I would also recommend using onBlur because it is good for application performance, since it is called only once the input loses focus. At src/pages/settings/Profile/ProfilePage.js:

   <TextInput
       (...)
       onBlur={(input) => {
                            input.target.value = input.target.value.trim();
                        }}
    />

If we need to perform more actions in the sanitization, we could create a function to sanitize field and put in the onBlur function, similar to how I have done in the code above. But as far as I see, the only sanitization applied is to trim the text, removing any leading space.

What alternative solutions did you explore? (Optional)

N/A

[zzarcon]

Proposal

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

Display name fields aren't sanitized after lose focus

What is the root cause of that problem?

The root cause of the problem is that we allow the user to type empty spaces which later get trimmed after saving.

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

We should reuse the exiting validate method to sanitize the values by calling trimStart(). Since that method already gets called when typing on the text field as well as on blur, we will not introduce new complexity nor new event listeners and the user experience will be more similar to to the end result

src/pages/settings/Profile/DisplayNamePage.js

const validate = (values) => {
  values.firstName = values.firstName.trimStart()
  values.lastName = values.lastName.trimStart()
}

https://github.com/Expensify/App/assets/1194982/2223b88c-a6c5-46ef-a8d7-03200dabe53a

What alternative solutions did you explore? (Optional)

N/A

[melvin-bot[bot]]

📣 @zzarcon! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details. Format:

   Contributor details
   Your Expensify account email: <REPLACE EMAIL HERE>
   Upwork Profile Link: <REPLACE LINK HERE>

[zzarcon]

Contributor details Your Expensify account email: hezarco@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~01fd9d006e1408b804

[melvin-bot[bot]]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[melvin-bot[bot]]

@ntdiary Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[ntdiary]

Personally, I don't have very strong feelings about fixing this particular UX case. If we want to care about consistency between blur and save, I think we also need to consider the following:

1. Should we prompt users that leading and trailing spaces will be trimmed (or is not allowed)?
2. If only spaces are entered, should we trim it to be empty? In this case, label animation are usually triggered. Will the animation be smooth and not contradict the prompt above?
3. Should we apply this to other forms that need trimming as well?

Based on the points above, if we do want to fix this, I think it would be better to implement it in form or input(maybe) related components and then provide an prop to enable it.

cc @m-natarajan, @twisterdotcom

Additionally, @zzarcon, we have many places that use validation functions which don't mutate the original object, I think it should be avoided here too. : )

[zzarcon]

@ntdiary I agree with that, to achieve a consistent experience across the app having it built in within the input component will be the best. I just didn't know how big this issue was, also fair point with the mutation, if we want to keep validate purely for validation purposes it won't be the best place to do such thing :)

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[ntdiary]

@ntdiary I agree with that, to achieve a consistent experience across the app having it built in within the input component will be the best. I just didn't know how big this issue was, also fair point with the mutation, if we want to keep validate purely for validation purposes it won't be the best place to do such thing :)

Maybe we don't need to find and modify all the forms right now. We can implement it in the generic component first and just enable for this case. If other pages need it in the future, we can enable it for them then.

[zzarcon]

We can implement it in the generic component first and just enable for this case

I think that approach could work! I was playing with it and added a new opt-in prop to <BaseTextInput>, by default false

src/components/TextInput/baseTextInputPropTypes.js

const propTypes = {
  /** Indicate whether or not the input should trim empty spaces */
  shouldSanitizeValue: PropTypes.bool,
}

and then handled the sanitization on src/components/TextInput/BaseTextInput.js

const setValue = (value) => {
        const sanitizedValue = props.shouldSanitizeValue ? value.trimStart() : value;
}

and as you suggested, only enabled it from <DisplayNamePage>

src/pages/settings/Profile/DisplayNamePage.js

<InputWrapper
    inputID="firstName"
    name="fname"
    shouldSanitizeValue
/>

<InputWrapper
    inputID="lastName"
    name="lname"
    shouldSanitizeValue
/>

It works the same as the video I shared before, let me know what you think about it @ntdiary

[twisterdotcom]

Just waiting on @ntdiary's thoughts.

[melvin-bot[bot]]

@ntdiary @twisterdotcom this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot[bot]]

@ntdiary, @twisterdotcom Whoops! This issue is 2 days overdue. Let's get this updated quick!

[twisterdotcom]

@ntdiary do you have time for these this week? If not, I'll reassign on Monday.

[ntdiary]

Ah, @twisterdotcom, very sorry, I forgot about this issue, and still have a lot on my plate, please feel free to re-assign another C+.

[melvin-bot[bot]]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot[bot]]

@ntdiary, @twisterdotcom Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[twisterdotcom]

Looking for somebody else: https://expensify.slack.com/archives/C02NK2DQWUX/p1698069885201119

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @lcsvcn We're missing your Upwork ID to automatically send you an offer for the Reporter role. Once you apply to the Upwork job, your Upwork ID will be stored and you will be automatically hired for future jobs!

[situchan]

I am not sure why this is bug. Why should input value be trimmed on blur?

[lcsvcn]

@MelvinBot https://www.upwork.com/freelancers/~01686d9f62c3ed0541

[shubham1206agra]

According to this slack convo, we will be aiming for a general solution for trimming. I feel like it should be passive, i.e., don't trim on every input at that time, but later when we change focus or are about to submit.

Since both proposals partially have the solution I wanted, I will give them some time to update the proposals. In the meantime, anyone else, please feel free to give their proposals and ask any questions.

cc @twisterdotcom

[situchan]

@luacmartins as form expert, can you please clarify the expected behavior?

[luacmartins]

Thanks for the ping @situchan! I agree that this is not a bug. According to our form guidelines:

User input that may include optional characters (e.g. (, ), - in a phone number) should never be restricted on input, nor be modified or formatted on blur. This type of input jacking is disconcerting and makes things feel broken.

Instead we will format and clean the user input internally before using the value (e.g. making an API request where the user will never see this transformation happen). Additionally, users should always be able to copy/paste whatever characters they want into fields.

[twisterdotcom]

Nice okay, let's close.