LG-8711: Do not eagerly reset foundAddress onSearch

[allthesignals]

🎫 Ticket

Link to the relevant ticket.

🛠 Summary of changes

We’re only rendering results when an address is geocoded and usps locations are returned:
{locationResults && foundAddress && (<InPersonLocations ... /> )}

What’s happening is that when I search for the same address again, it eagerly resets foundAddress to null, expecting this hook to run later:

  useDidUpdateEffect(() => {
    onFoundLocations(locationResults);

    foundAddress && onFoundAddress(foundAddress);
  }, [locationResults]);

But, because the returned locationResults are identical to the previous, that hook never fires, and locationResults && foundAddress evaluates false.

I tried to prevent multiple queries of the same address (yes, the bug appears when you search the same address twice/click search twice), but that isDisabled prop for SpinnerButton isn’t really behaving like I thought it would (see context).

This change solves the immediate problem by not resetting foundAddress onSearch, however, I am not confident it doesn't create another problem elsewhere.

📜 Testing Plan

Provide a checklist of steps to confirm the changes.

• Add new test to catch regression
• Test locally with USPS credentials?

👀 Screenshots

If relevant, include a screenshot or screen capture of the changes.

Before:

before.mov

After:

after.mov

[eileen-nava]

This works well. I do want to flag that now, when a user searches for two different addresses back to back, the search results won’t clear after the first search. I included a video to demonstrate this.

SearchResultsPersistButUpdateSimultaneously.mov

I’m okay with this, because the address and location results still update at the same time. However, I’d like to check in with product/design about it. Thoughts? cc: @sumiat @kellular

[eileen-nava]

We could potentially

1. merge this change, along with a test
2. make a story to restore the functionality that clears past search results after the user clicks “search”

Thoughts?

I do think this bug indicates that it would be good to revisit the logic around the useDidUpdateEffect hook. It had different logic when we first looked at adding onFoundAddress(null); to handleSearch . Related github conversation here.

[allthesignals]

This commit addresses the regression you noticed in your second point (2)

[eileen-nava]

Approved! 💯 Thanks again, @allthesignals.

[allthesignals]

Is this a sign that <InPersonLocations/> needs to be rendered from within <AddressSearch/>? Then we can remove so much context from this component and only require a callback for handleLocationSelect...

[allthesignals]

Added a test for this bug scenario. Will merge after passing CI.