[$250] Distance - Recent destination list shows more than 5 destinations

[izarutskaya]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 9.0.6-0
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4709538
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to FAB > Submit expense > Distance.
3. Click Start point.
4. Select 6 different waypoints.
5. Note that the recent destination list shows more than 5 destinations.

Expected Result:

The recent destination list will show at most 5 destinations (production behavior).

Actual Result:

The recent destination list shows more than 5 destinations on staging.

Workaround:

Unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6538587_1720674896378.bandicam_2024-07-11_13-06-18-825.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018ea8c6fb29aecd8f
• Upwork Job ID: 1811456879870180805
• Last Price Increase: 2024-07-11

Issue Owner

Current Issue Owner: @Ollyws

[melvin-bot]

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

[melvin-bot]

Triggered auto assignment to @danieldoglas (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[izarutskaya]

@greg-schroeder FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors.

[izarutskaya]

We think this issue might be related to the #collect project.

[izarutskaya]

Production

[etCoderDysto]

Proposal

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

Recent destination list shows more than 5 destinations

What is the root cause of that problem?

CONST.RECENT_WAYPOINTS_NUMBER value is '20'. And we sliced the array using the value.

App/src/pages/iou/request/step/IOURequestStepWaypoint.tsx

Lines 257 to 259 in 19922f5

	selector: (waypoints) =>
	(waypoints ? waypoints.slice(0, CONST.RECENT_WAYPOINTS_NUMBER as number) : []).map((waypoint) => ({
	name: waypoint.name,

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

We should change CONST.RECENT_WAYPOINTS_NUMBER = 20 to CONST.RECENT_WAYPOINTS_NUMBER = 5 in CONST file.

What alternative solutions did you explore? (Optional)

[Julesssss]

Demoting from blocker

[melvin-bot]

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

[melvin-bot]

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

[Krishna2323]

Proposal

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

Distance - Recent destination list shows more than 5 destinations

What is the root cause of that problem?

I think we are intentionally storing 20 recent destinations and it was added in this PR . We just need to slice the predefinedPlaces (recent destination) when passing it to GooglePlacesAutocomplete's predefinedPlaces prop.

App/src/components/AddressSearch/index.tsx

Lines 325 to 330 in 9138d8b

	const filteredPredefinedPlaces = useMemo(() => {
	if (!searchValue) {
	return predefinedPlaces ?? [];
	}
	return predefinedPlaces?.filter((predefinedPlace) => isPlaceMatchForSearch(searchValue, predefinedPlace)) ?? [];
	}, [predefinedPlaces, searchValue]);

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

We should splice the predefinedPlaces to only show 5 results at a time.

    const filteredPredefinedPlaces = useMemo(() => {
        if (!searchValue) {
            return predefinedPlaces?.slice(0, 5) ?? [];
        }
        return predefinedPlaces?.filter((predefinedPlace) => isPlaceMatchForSearch(searchValue, predefinedPlace)).splice(0, 5) ?? [];
    }, [predefinedPlaces, searchValue]);

What alternative solutions did you explore? (Optional)

[arosiclair]

This is not a bug we intentionally increased the number of displayed places to 20. We may need to update regression test steps somewhere though. cc @neil-marcellini

[Krishna2323]

@arosiclair, the Save will be pushed down and we need to scroll, to see the scroll button, I don't think thats expected. The button should always be fixed.

waypoints_page_scrollables.mp4

[danieldoglas]

assigning @neil-marcellini to the issue based on @arosiclair's comment

[arosiclair]

@arosiclair, the Save will be pushed down and we need to scroll, to see the scroll button, I don't think thats expected. The button should always be fixed.

I think that's correct, but we did have a long discussion about it and I don't remember where we landed tbh. I'll leave it up to @neil-marcellini if we should change it or not.

[neil-marcellini]

Ah yeah this stuff again 😅, let me find what we decided.

This is working as expected. I sort of explained it here and we decided to go with the following approach at the end of that thread.

• There is one scrollable container on the page
• We show 20 recent waypoints because that matches the number we store and allows us to filter them when searching
  (I am open to feature requests to show a number of waypoints that will fit on the screen nicely, but use all 20 for searching. It's probably too complicated though and doesn't solve a real problem)
• The user knows there is a save button because it appears before the recent waypoints load
• As long as there are recent or suggested waypoints in the list, the save button isn't needed because selecting one will save it. We also don't need to hide the save button. That's added complexity without any benefit
• The user can always scroll down to find the save button, or hit enter when typing in a value (I think)

@izarutskaya I wasn't able to find a regression test checking that there are only 5 recent waypoints. If so, please update it to 20.