[$250] Android - Distance - Previous waypoints displayed for a second before new suggestions appear

[lanitochka17]

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.29-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/4923199&group_by=cases:section_id&group_order=asc&group_id=309127
Issue reported by: Applause - Internal Team

Action Performed:

1. Open the Expensify app
2. Tap on the FAB
3. Tap on "Submit Expense"
4. Tap on "Distance"
5. Tap on "Start" point
6. Write one letter of the searched direction
7. Check the suggestions behaviour
8. Write another letter
9. Check the suggestions behaviour again
10. Tap on "Stop" point
11. Repeat steps 6-9

Expected Result:

When the user starts writing a new waypoint, all the related suggestions should be displayed automatically

Actual Result:

When user starts writing a new waypoint, with each new letter, previously used directions appear for a second before all the related suggestions are displayed

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

Add any screenshot/video evidence

Bug6592945_1725458793387.Distance_Suggestions.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021832156095103318812
• Upwork Job ID: 1832156095103318812
• Last Price Increase: 2024-09-13
• Automatic offers:
• nyomanjyotisa | Contributor | 104055859

Issue Owner

Current Issue Owner: @ntdiary

[melvin-bot]

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

[lanitochka17]

@puneetlath 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

[FitseTLT]

Edited by proposal-police: This proposal was edited at 2024-09-05 13:31:17 UTC.

Proposal

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

Android - Distance - Previous waypoints displayed for a second before new suggestions appear

What is the root cause of that problem?

We are displaying the predefined places even in online case here until the search suggestion result comes from google place auto complete

App/src/components/AddressSearch/index.tsx

Lines 325 to 329 in 6760f96

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

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

We need to show the predefined places only for offline case so change it to

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

What alternative solutions did you explore? (Optional)

We can also do

App/src/components/AddressSearch/index.tsx

Line 375 in 6760f96

predefinedPlaces={filteredPredefinedPlaces}

                        predefinedPlaces={isOffline ? filteredPredefinedPlaces : []}

or we can set predefinedPlacesAlwaysVisible to true if we want to always show the predefined places

[nyomanjyotisa]

Edited by proposal-police: This proposal was edited at 2024-09-12 00:31:47 UTC.

Proposal

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

Distance - Previous waypoints displayed for a second before new suggestions appear

What is the root cause of that problem?

if the searchValue is not empty we display the filtered predefinedPlaces in both online and offline

App/src/components/AddressSearch/index.tsx

Lines 325 to 330 in 2af4d0a

	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?

If searchValue exist and is online and also is typing, don't show any predefinedPlaces. And if searchValue is empty we still need to display the predefinedPlaces even in online mode

And we only need to rerender filteredPredefinedPlaces once searchValue changed or on onInputChange to prevent this issue

Create new const

const [shouldHidePredefinedPlaces, setShouldHidePredefinedPlaces] = useState(false);

Change this code to the following

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

Then add the following code on onInputChange here

if(isOffline){
    setShouldHidePredefinedPlaces(false)
}else{
    setShouldHidePredefinedPlaces(true)
}

or just this

setShouldHidePredefinedPlaces(!isOffline)

What alternative solutions did you explore? (Optional)

Create a new const

const [shouldHidePredefinedPlaces, setShouldHidePredefinedPlaces] = useState(false);

If searchValue exist and is online and also shouldHidePredefinedPlaces, don't show any predefinedPlaces. And if searchValue is empty we still need to display the predefinedPlaces even in online mode

    const filteredPredefinedPlaces = useMemo(() => {
        if (!searchValue) {
            return predefinedPlaces ?? [];
        } else if (!isOffline && shouldHidePredefinedPlaces) {
            return [];
        }

        return predefinedPlaces?.filter((predefinedPlace) => isPlaceMatchForSearch(searchValue, predefinedPlace)) ?? [];
    }, [predefinedPlaces, searchValue]);

onInputChange here we should set setShouldHidePredefinedPlaces to true

We should setShouldHidePredefinedPlaces to false if isOffline changed

    useEffect(() => {
        setShouldHidePredefinedPlaces(false)
    }, [isOffline]);

[Shahidullah-Muffakir]

Proposal

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

When the user starts typing a new location in the app, they briefly see old locations they've used before, but then those disappear and get replaced by new suggested locations from the map service. This can be confusing for the user because:

1. The old locations flash on the screen for just a second before going away, so the user doesn't have time to actually select them.
2. It's not clear why those old locations are being shown at all, since the user can't use them.
   That is why I think we should hide them.

What is the root cause of that problem?

The app is always showing predefined locations (previously used by the user) regardless of whether the user is offline or online. This behavior is not ideal when the user is online and actively typing in a new location.

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

When the user is online, only show the predefined locations if they haven't started typing anything in the input box yet. As soon as the user starts typing, stop showing the previously used locations and instead display the recommended locations fetched from the map provider's live API.
However, if the user is offline, always show the predefined locations whether they have typed anything or not, since these are the only locations the user can select from in offline mode.

App/src/components/AddressSearch/index.tsx

Line 325 in 2af4d0a

const filteredPredefinedPlaces = useMemo(() => {

if (!isOffline && searchValue) {
  return [];
}

This will ensure that when the user is online and has started typing, an empty array is returned, preventing the previously used locations from being displayed.

Screen.20Recording.202024-09-05.20at.207.mp4

[bernhardoj]

Proposal

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

Saved waypoints are shown briefly when searching address.

What is the root cause of that problem?

When we search, the search keyword state is updated which recalculate the predefined places.

App/src/components/AddressSearch/index.tsx

Lines 325 to 330 in 8fc5e68

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

When the predefined places are updated, the auto complete component will update the list with the new predefined places and pass an empty result to it.
https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/ef107387c6846924423b832a242eee83cb07eed4/GooglePlacesAutocomplete.js#L176-L179

Because the result is empty, the predefined places are shown.
https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/ef107387c6846924423b832a242eee83cb07eed4/GooglePlacesAutocomplete.js#L91-L102

After the search is completed, the list is updated again with the new result.

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

We need to pass the result when updating the list.
https://github.com/FaridSafi/react-native-google-places-autocomplete/blob/ef107387c6846924423b832a242eee83cb07eed4/GooglePlacesAutocomplete.js#L176-L179

There is already a variable that stores the last result here, but it's not a ref, so the value is always gone after re-render, so we need to convert it to ref too.

useEffect(() => {
  // Update dataSource if props.predefinedPlaces changed
  setDataSource(buildRowsFromResults(_results.current));
}, [buildRowsFromResults, props.predefinedPlaces]);

What alternative solutions did you explore? (Optional)

Never show saved waypoints (predefined places) when searching.

App/src/components/AddressSearch/index.tsx

Lines 325 to 330 in 8fc5e68

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

[melvin-bot]

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

[melvin-bot]

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

[ntdiary]

We need to show the predefined places only for offline case

Hi, @FitseTLT, curious is there any doc supporting this requirement? I feel it would also be good to display predefined places if the searchValue is empty? 🤔

[FitseTLT]

We need to show the predefined places only for offline case

Hi, @FitseTLT, curious is there any doc supporting this requirement? I feel it would also be good to display predefined places if the searchValue is empty? 🤔

Maybe let's ask @puneetlath for Internal thoughts 👍

[ntdiary]

Hi, @lanitochka17, @puneetlath, I have a question about the expect result, should the related recent destinations be displayed in the suggestion list? Could you please confirm this point? (e.g., The Home place in my screenshot)

[puneetlath]

I think we should show them when the composer is empty and when offline. I think once you start typing, it's fine for us to just use whatever results the maps API thinks is the most relevant to your search. Which I would assume can also include the pre-defined places.

[Shahidullah-Muffakir]

I think we should show them when the composer is empty and when offline. I think once you start typing, it's fine for us to just use whatever results the maps API thinks is the most relevant to your search. Which I would assume can also include the pre-defined places.

@puneetlath Based on your input, I believe my proposal aligns well with the requirements you've outlined, particularly regarding showing suggestions when the composer is empty (whether online or offline) and when offline (at all times)
My Proposal: #48643 (comment)
Thank you.

[ntdiary]

I think we should show them when the composer is empty and when offline. I think once you start typing, it's fine for us to just use whatever results the maps API thinks is the most relevant to your search. Which I would assume can also include the pre-defined places.

Based on this input, @nyomanjyotisa's solution is the first to meet the requirements. Additionally, there's a minor edge case with this solution: if we start typing while offline, the filtered predefined places will be shown, but when switching back online, no suggestions will appear. In the production app, the filtered predefined places will still be displayed.

test.mp4

[puneetlath]

Hmm. Is there any way to have that not be the case?

[nyomanjyotisa]

I think we should show them when the composer is empty and when offline. I think once you start typing, it's fine for us to just use whatever results the maps API thinks is the most relevant to your search. Which I would assume can also include the pre-defined places.

Based on this input, @nyomanjyotisa's solution is the first to meet the requirements. Additionally, there's a minor edge case with this solution: if we start typing while offline, the filtered predefined places will be shown, but when switching back online, no suggestions will appear. In the production app, the filtered predefined places will still be displayed.

test.mp4

Updated my proposal to address this issue

-1-New-Expensify.24.mp4

[ntdiary]

a new edge issue:
if we go offline after start typing, the updated solution will show a filtered predefined places, while in production, our app will still show the search results. 🤔

test.mp4

[nyomanjyotisa]

Proposal updated to address the original Issue, this issue, and also this issue

New-Expensify.30.mp4

[ntdiary]

another case:
After clearing the search value, the predefined places are not displayed 😂:

clear-case.mp4

[melvin-bot]

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

[nyomanjyotisa]

another case: After clearing the search value, the predefined places are not displayed 😂:

clear-case.mp4

Predefined places are displayed after clearing the search value, make sure you try the main solution on my proposal

New-Expensify.31.mp4

[ntdiary]

Predefined places are displayed after clearing the search value, make sure you try the main solution on #48643 (comment)

I reinstalled the react-native-google-places-autocomplete package, and this edge case seems to have disappeared. However, from your video, I can still see that the predefined places flash for a moment before the loading appears. 🤔

[melvin-bot]

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

[melvin-bot]

@puneetlath @ntdiary 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]

@puneetlath, @ntdiary Huh... This is 4 days overdue. Who can take care of this?

[puneetlath]

@ntdiary what's next here?

[ntdiary]

@ntdiary what's next here?

After re-investigating, I found that the previously mentioned flash was caused by the hiding of currentLocation, which is unrelated to @nyomanjyotisa's solution.

App/src/components/AddressSearch/index.tsx

Lines 304 to 311 in 00d2e32

	{/* This will show current location button in list if there are some recent destinations */}
	{shouldShowCurrentLocationButton && (
	<CurrentLocationButton
	onPress={getCurrentLocation}
	isDisabled={isOffline}
	/>
	)}
	{!value && <Text style={[styles.textLabel, styles.colorMuted, styles.pv2, styles.ph3, styles.overflowAuto]}>{translate('common.recentDestinations')}</Text>}

This means their proposal is still reasonable, it's fine to move forward with their proposal! :)

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot]

📣 @nyomanjyotisa 🎉 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]

This issue has not been updated in over 15 days. @puneetlath, @ntdiary, @nyomanjyotisa eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[puneetlath]

What's the status with this? Is it ready to pay out?

[ntdiary]

The PR was deployed to production 3 weeks ago, but the automation here has failed, I will fill out the bug zero checklist. :)

[ntdiary]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist will need to be completed before the issue can be closed:

• [@ntdiary] The PR that introduced the bug has been identified. Link to the PR: Reapply - Add offline search functionality for addresses #43011
• [@ntdiary] 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: Reapply - Add offline search functionality for addresses #43011 (comment)
• [@ntdiary] 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: No need
• [@ntdiary] Determine if we should create a regression test for this bug. Yes
• [@ntdiary] 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. Show as below
• [@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Regression steps:

1. Open the Expensify app
2. Tap on the FAB
3. Tap on "Submit Expense"
4. Tap on "Distance"
5. Tap on "Start" point
6. If your address suggestion list is empty, please search for an address and save it. Repeat this step a few times to save multiple addresses.
7. Tap on "Stop" point.
8. Enter one letter from any of those previously saved addresses.
9. Verify the previous filtered addresses won't display for a second before the new suggestions appear.

[puneetlath]

Payment summary:

• C - $250 - @nyomanjyotisa - paid via Upwork
• C+ - $250 - @ntdiary - to be paid via NewDot

Thanks everyone!

[JmillsExpensify]

$250 approved for @ntdiary

[ntdiary]

Hi, @JmillsExpensify, app keeps prompting me that the Merchant is duplicated, not sure if this will affect approval. 😂

[JmillsExpensify]

Just ignore it, though I've already approved this payment above.