[HOLD for payment 2023-05-03] [Regression] [$250] Web - Personal Details - Using spaces and a letter then removing it still shows the small dropdown box with an empty record

[kbecciv]

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 the Sittings > Profile > Personal details
2. Enter some spaces on Address line 1
3. Then use a letter and remove it

Expected Result:

The dropdown box should not be displayed if you have removed the character entered.

Actual Result:

Using spaces and a letter then removing it still shows the small dropdown box with an empty record

Workaround:

Unknown

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: 1.3.1.3

Reproducible in staging?: Yes

Reproducible in production?: Yes

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

Recording.2524.mp4

2023-04-19.00-39-37.mp4

Expensify/Expensify Issue URL:

Issue reported by:@ayazhussain79

Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1681848102529159

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01aa2953586b4b6c29
• Upwork Job ID: 1649125546977239040
• Last Price Increase: 2023-04-24

[MelvinBot]

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

[MelvinBot]

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

[MelvinBot]

Current assignee @maddylewis 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]

Triggered auto assignment to @youssef-lr (External), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[PrashantMangukiya]

Proposal

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

Web Personal Details - Using spaces and a letter then removing it still shows the small dropdown box with an empty record.

What is the root cause of that problem?

When we type something within address field at that time it is using onInputChange from AddressSearch.js

App/src/components/AddressSearch.js

Lines 246 to 257 in cfca7c7

	onInputChange: (text) => {
	if (props.inputID) {
	props.onInputChange(text);
	} else {
	props.onInputChange({street: text});
	}
	// If the text is empty, we set displayListViewBorder to false to prevent UI flickering
	if (_.isEmpty(text)) {
	setDisplayListViewBorder(false);
	}
	},

We can see at line 254-256 we are calling _.isEmpty(text) and then then hiding border setDisplayListViewBorder(false); if text is empty.

So here when we enter empty space at that time _.isEmpty() will return false because _.isEmpty will check for string length property, so it will consider string as not empty due to space so it will not hide border. So this is the root cause of the problem.

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

We have to update code as shown below at line 254 i.e. Use text.trim() before checking _.isEmpty()

  // If the text is empty, we set displayListViewBorder to false to prevent UI flickering
  if (_.isEmpty(text.trim())) {      // *** Trim before checking empty ***
      setDisplayListViewBorder(false);
  }

So this will solve the issue as shown in result video.

What alternative solutions did you explore? (Optional)

None

Results

Web.mov

[hungvu193]

Proposal

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

Using spaces and a letter then removing it still shows the small dropdown box with an empty record

What is the root cause of that problem?

Inside onInputChange of AddressSearch, we are using underscrore function isEmpty in order to show the view border of address list. In this case, the text included spaces so isEmpty returned true which caused the issue.

App/src/components/AddressSearch.js

Lines 254 to 256 in cfca7c7

	if (_.isEmpty(text)) {
	setDisplayListViewBorder(false);
	}

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

In order to fix the issue, we can just trim the space, before comparing, we can avoid using isEmpty since it added few unnecessary checks. So the onInputChange function will look like:

                        onInputChange: (text) => {
                            if (props.inputID) {
                                props.onInputChange(text);
                            } else {
                                props.onInputChange({street: text});
                            }

                            // If the text is empty, we set displayListViewBorder to false to prevent UI flickering
                            if (!text.trim().length) {
                                setDisplayListViewBorder(false);
                            }
                        },

What alternative solutions did you explore? (Optional)

None

[bernhardoj]

Proposal

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

The address autocomplete empty list is shown after making the search empty with spaces only.

What is the root cause of that problem?

This is a regression from this PR #17403 by adding padding vertical 6 to the list view. Currently, we completely hide the list view when the text is empty, a list item is pressed, or the list height is <= 2. This is to solved this issue #5810. I think it's expected to not trim the text because address autocomplete has a "debounce" on the search keyword and if we trim it, we will see a blank space for a while as can be seen at @PrashantMangukiya video. Why? Because we hide all the list item (scale 0), but inside GooglePlacesAutocomplete, it still have the previous list and height before update to the new list after a "delay".

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

I guess the PR owner is the one who should fix this regression, but here is my solution.

As mentioned before, the list view will be completely hidden if the height is <= 2, but this isn't valid anymore because we add padding vertical 6, which means we should add 12 to the calculation.

App/src/components/AddressSearch.js

Lines 279 to 282 in 4f7cfdb

	// We use the height of the element to determine if we should hide the border of the listView dropdown
	// to prevent a lingering border when there are no address suggestions.
	// The height of the empty element is 2px (1px height for each top and bottom borders)
	setDisplayListViewBorder(event.nativeEvent.layout.height > 2);

[aman-atg]

@bernhardoj Thank you for pointing out the regression from #17403. Since, this is under regression period I'll be making a PR shortly.

cc: @s77rt @mountiny

[s77rt]

Oh that's an interesting case. Sorry for missing that. But who starts a search with spaces 😅? @ayazhussain79 is a real tester 😁.

@mountiny Would you please assign me and @aman-atg to handle this.

[MelvinBot]

📣 @aman-atg You have been assigned to this job by @mountiny!
Please apply to this job in Upwork 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 📖

[MelvinBot]

📣 @s77rt You have been assigned to this job by @mountiny!
Please apply to this job in Upwork 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 📖

[MelvinBot]

Upwork job price has been updated to $250

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

• fix 17664: use googleEmptyListViewHeight variable with correct value #17832

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

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:

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

[s77rt]

• The PR that introduced the bug has been identified: add vertical spacing to avoid outline getting cut off in Address Search input #17403
• The offending PR has been commented on: add vertical spacing to avoid outline getting cut off in Address Search input #17403 (comment)
• A discussion in #expensify-bugs has been: https://expensify.slack.com/archives/C049HHMV9SM/p1682501033915429
• Determine if we should create a regression test for this bug: Yes

---

Regression Test Proposal

1. Open App
2. Go to the Settings > Profile > Personal details
3. Enter a few spaces on the 'Address line 1' field
4. Type some letters e.g. ka
5. Verify a dropdown list is shown
6. Delete the letters (but keep the spaces)
7. Verify the dropdown list is no longer shown

[maddylewis]

Payments:

• reporting bonus - @ayazhussain79 ($250)
• contributor - @aman-atg ($250)
• C+ - @s77rt ($250)

just confirming the amounts before processing payment. lmk if that looks correct.

[s77rt]

@maddylewis The payment is only due to @ayazhussain79

[maddylewis]

thanks for confirming! ill get the offer sent to @ayazhussain79 shortly.

[ayazhussain79]

@maddylewis offer accepted Thank you

[s77rt]

I think we are good to close this one?

[mountiny]

thanks everyone