-
Notifications
You must be signed in to change notification settings - Fork 0
[fix] correct city names in address autocomplete dropdown #29
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
25b94ec
a75a722
12fb4fb
ad1bc74
8345432
e21853c
ba82f4e
23c02d6
62c5192
d79c99b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,12 +22,14 @@ export function AddressSearch({ | |
| portalRoot, | ||
| }: AddressSearchProps) { | ||
| const places = useMapsLibrary("places"); | ||
| const addressValidation = useMapsLibrary("addressValidation"); | ||
| const token = useRef<google.maps.places.AutocompleteSessionToken | null>( | ||
| null, | ||
| ); | ||
| const placesRef = useRef< | ||
| Record<string, google.maps.places.AutocompleteSuggestion | undefined> | ||
| >({}); | ||
| const correctedTextRef = useRef<Record<string, string>>({}); | ||
| const [inputValue, setInputValue] = useState<string>(""); | ||
| const searchQuery = inputValue.trim(); | ||
| const [cache, setCache] = useState< | ||
|
|
@@ -41,7 +43,7 @@ export function AddressSearch({ | |
| >([]); | ||
|
|
||
| useEffect(() => { | ||
| if (!places) return; | ||
| if (!places || !addressValidation) return; | ||
|
|
||
| // Create new token if not exists | ||
| if (!token.current) { | ||
|
|
@@ -63,19 +65,80 @@ export function AddressSearch({ | |
| // region: "US", // Don't restrict to US -- this changes the way the formatted address is returned | ||
| language: "en", | ||
| includedPrimaryTypes: ["street_address"], | ||
| }).then(({ suggestions }) => { | ||
| }).then(async ({ suggestions }) => { | ||
| suggestions.forEach((suggestion) => { | ||
| if (!suggestion.placePrediction?.placeId) return; | ||
| placesRef.current[suggestion.placePrediction.placeId] = | ||
| suggestion; | ||
| }); | ||
|
|
||
| // Use Address Validation API to get correct USPS city names. | ||
| // Autocomplete secondaryText returns CDPs like "Briarcliff" | ||
| // instead of the postal city like "Austin". | ||
| await Promise.all( | ||
| suggestions.map(async (suggestion) => { | ||
| const placeId = suggestion.placePrediction?.placeId; | ||
| const streetAddress = | ||
| suggestion.placePrediction?.mainText?.text; | ||
| if ( | ||
| !placeId || | ||
| !streetAddress || | ||
| correctedTextRef.current[placeId] | ||
| ) | ||
| return; | ||
|
|
||
| // Use whichever has more info: user's input (may include | ||
| // city/state) or the autocomplete street address | ||
| const addressInput = | ||
| searchQuery.length > streetAddress.length | ||
| ? searchQuery | ||
| : streetAddress; | ||
|
Comment on lines
+90
to
+95
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we think this is better / more accurate? longer length doesnt represent more information necessarily right?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should always honor / respect user's input. |
||
|
|
||
| try { | ||
| const validation = | ||
| await addressValidation.AddressValidation.fetchAddressValidation( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is gonna get extremely expensive. Rough estimate is probably at least 10x our cost. We should find a better solution here or only fetch validation after selection. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack, @divazbozz looks like has a better solution. If so, i'll be happy to close this one. ref: |
||
| { | ||
| address: { | ||
| addressLines: [addressInput], | ||
| regionCode: "US", | ||
| }, | ||
| uspsCASSEnabled: true, | ||
| }, | ||
| ); | ||
|
|
||
| const uspsCity = | ||
| validation.uspsData?.standardizedAddress?.city; | ||
| const postalAddress = validation.address?.postalAddress; | ||
| const rawCity = uspsCity || postalAddress?.locality || ""; | ||
| // USPS city is uppercase (e.g. "AUSTIN"), title-case it | ||
| const city = rawCity | ||
| .toLowerCase() | ||
| .replace(/\b\w/g, (c: string) => c.toUpperCase()); | ||
| const state = postalAddress?.administrativeArea || ""; | ||
| const country = | ||
| postalAddress?.regionCode === "US" | ||
| ? "USA" | ||
| : (postalAddress?.regionCode ?? ""); | ||
|
|
||
| if (city) { | ||
| correctedTextRef.current[placeId] = [city, state, country] | ||
| .filter(Boolean) | ||
| .join(", "); | ||
| } | ||
| } catch { | ||
| // Fall back to default secondaryText on error | ||
| } | ||
| }), | ||
| ); | ||
|
ruchirx marked this conversation as resolved.
|
||
|
|
||
| return suggestions; | ||
|
Comment on lines
+68
to
134
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔴 Autocomplete suggestions are blocked until all Address Validation API calls complete The promise stored in the Prompt for agentsWas this helpful? React with 👍 or 👎 to provide feedback.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this ⏫ , this component is pretty important in terms of how snappy it feels, can we double check latency / user experience (can we show a video or sth to see how it feels)
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have tried this on npm run dev, and it feels fast and snappy enough. unclear whats ask here, Do you want me to record a video and upload somewhere as evidence? |
||
| }), | ||
| }; | ||
| }); | ||
| }, [places, searchQuery]); | ||
| }, [places, addressValidation, searchQuery]); | ||
|
|
||
| useEffect(() => { | ||
| let stale = false; | ||
| if (!searchQuery) { | ||
| setPlacesResult([]); | ||
| return; | ||
|
|
@@ -84,9 +147,12 @@ export function AddressSearch({ | |
| const cached = cache[searchQuery]; | ||
| if (cached) { | ||
| cached.then((suggestions) => { | ||
| setPlacesResult(suggestions); | ||
| if (!stale) setPlacesResult(suggestions); | ||
| }); | ||
| } | ||
| return () => { | ||
| stale = true; | ||
| }; | ||
| }, [cache, searchQuery]); | ||
|
|
||
| const handleSelect = useCallback( | ||
|
|
@@ -97,23 +163,43 @@ export function AddressSearch({ | |
| setInputValue( | ||
| [ | ||
| place.placePrediction?.mainText?.text, | ||
| place.placePrediction?.secondaryText?.text, | ||
| correctedTextRef.current[result.id] || | ||
| place.placePrediction?.secondaryText?.text, | ||
| ] | ||
| .filter(Boolean) | ||
| .join(", "), | ||
| ); | ||
| // Save corrected city before refs are cleared | ||
| const corrected = correctedTextRef.current[result.id]; | ||
| await place.placePrediction | ||
| ?.toPlace() | ||
| .fetchFields({ | ||
| fields: ["location", "formattedAddress", "addressComponents"], | ||
| }) | ||
| .then(({ place }) => { | ||
| return onSelect?.({ selection: parseAddress(place) }); | ||
| const selection = parseAddress(place); | ||
| // Override city and formattedAddress with USPS-validated city | ||
| if (selection && corrected) { | ||
| const correctedCity = corrected.split(",")[0].trim(); | ||
| if (correctedCity) { | ||
| const originalCity = selection.address.city; | ||
| selection.address.city = correctedCity; | ||
| if (originalCity) { | ||
| selection.formattedAddress = | ||
| selection.formattedAddress.replace( | ||
| originalCity, | ||
| correctedCity, | ||
| ); | ||
| } | ||
| } | ||
|
Comment on lines
+183
to
+194
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why only overwrite city? why not use the entire corrected address?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. b/c the only discrepancy I have noticed is with city. Have we seen any other discrepancy / should we prefer if we swap everything? |
||
| } | ||
| return onSelect?.({ selection }); | ||
| }); | ||
|
|
||
| // Clear cached values now that our selection is complete -- the token is only valid until the first toPlace() call | ||
| setCache({}); | ||
| placesRef.current = {}; | ||
| correctedTextRef.current = {}; | ||
| token.current = null; | ||
| }, | ||
| [onSelect], | ||
|
|
@@ -124,7 +210,10 @@ export function AddressSearch({ | |
| (suggestion) => | ||
| ({ | ||
| mainText: suggestion.placePrediction?.mainText?.text, | ||
| secondaryText: suggestion.placePrediction?.secondaryText?.text, | ||
| secondaryText: | ||
| correctedTextRef.current[ | ||
| suggestion.placePrediction?.placeId || "" | ||
| ] || suggestion.placePrediction?.secondaryText?.text, | ||
| id: suggestion.placePrediction?.placeId, | ||
| }) as Result, | ||
| ); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔴 Autocomplete completely blocked until
addressValidationlibrary finishes loadingAt line 46, the guard
if (!places || !addressValidation) return;prevents any autocomplete requests from firing until theaddressValidationlibrary has loaded. Previously, autocomplete only neededplacesto be available. SinceaddressValidationis a separate library loaded asynchronously viauseMapsLibrary(src/utils/useMapsLibrary.ts:27-30), this adds latency before any suggestions can appear. Worse, if theaddressValidationlibrary fails to load (e.g., not enabled on the API key, network error), autocomplete will be permanently non-functional.The address validation calls are an enhancement (correcting city names) and should degrade gracefully — the autocomplete suggestions should still be shown even if validation hasn't loaded yet.
Prompt for agents
Was this helpful? React with 👍 or 👎 to provide feedback.