From cb33c1401622fdfe8e824c99b8b2703a1638f89c Mon Sep 17 00:00:00 2001 From: Atul Varma Date: Wed, 6 May 2020 18:08:16 -0400 Subject: [PATCH] Always use our localized state label. (#1394) Due to reasons explained in #1384, we're still going to be using the English-localized version of Mapbox's API in our Spanish locale, but it would be nice if we could at least still be consistent and show the Spanish version of the state name from our own locale data. This implements that, along with some other refactorings that simplify our code a bit. --- .../lib/forms/city-and-state-form-field.tsx | 12 +++------ .../lib/forms/mapbox/city-autocomplete.tsx | 25 +++++++++++-------- frontend/lib/forms/mapbox/common.tsx | 17 +++++-------- .../mapbox/tests/city-autocomplete.test.tsx | 9 +++---- .../lib/forms/mapbox/tests/common.test.tsx | 18 +++++-------- 5 files changed, 33 insertions(+), 48 deletions(-) diff --git a/frontend/lib/forms/city-and-state-form-field.tsx b/frontend/lib/forms/city-and-state-form-field.tsx index b43b140d6..158fe2570 100644 --- a/frontend/lib/forms/city-and-state-form-field.tsx +++ b/frontend/lib/forms/city-and-state-form-field.tsx @@ -13,7 +13,6 @@ import { AppContext } from "../app-context"; import { USStateChoice, isUSStateChoice, - getUSStateChoiceLabels, } from "../../../common-data/us-state-choices"; export type CityAndStateFieldProps = { @@ -21,9 +20,9 @@ export type CityAndStateFieldProps = { stateProps: BaseFormFieldProps; }; -function safeGetUSStateChoice(state: string): USStateChoice | "" { +function safeGetUSStateChoice(state: string): USStateChoice | null { if (isUSStateChoice(state)) return state; - return ""; + return null; } const BaselineField: React.FC = (props) => ( @@ -37,14 +36,11 @@ const EnhancedField: React.FC< CityAndStateFieldProps & { pe: ProgressiveEnhancementContext } > = (props) => { const { cityProps, stateProps } = props; - const stateCode = safeGetUSStateChoice(stateProps.value); - const stateName = stateCode ? getUSStateChoiceLabels()[stateCode] : ""; const initialValue: MapboxCityItem | undefined = cityProps.value ? { city: cityProps.value, mapboxFeature: null, - stateCode, - stateName, + stateChoice: safeGetUSStateChoice(stateProps.value), } : undefined; @@ -58,7 +54,7 @@ const EnhancedField: React.FC< initialValue={initialValue} onChange={(item) => { cityProps.onChange(item.city); - stateProps.onChange(item.stateCode); + stateProps.onChange(item.stateChoice || ""); }} onNetworkError={props.pe.fallbackToBaseline} errors={cityProps.errors} diff --git a/frontend/lib/forms/mapbox/city-autocomplete.tsx b/frontend/lib/forms/mapbox/city-autocomplete.tsx index 17ca587d3..069b256eb 100644 --- a/frontend/lib/forms/mapbox/city-autocomplete.tsx +++ b/frontend/lib/forms/mapbox/city-autocomplete.tsx @@ -8,11 +8,14 @@ import { import { getGlobalAppServerInfo } from "../../app-context"; import { MapboxFeature, - MapboxStateInfo, - getMapboxStateInfo, + getMapboxStateChoice, MapboxResults, createMapboxPlacesURL, } from "./common"; +import { + USStateChoice, + getUSStateChoiceLabels, +} from "../../../../common-data/us-state-choices"; class MapboxCitySearchRequester extends SearchRequester { searchQueryToURL(query: string): string { @@ -31,7 +34,8 @@ class MapboxCitySearchRequester extends SearchRequester { export type MapboxCityItem = { city: string; mapboxFeature: MapboxFeature | null; -} & MapboxStateInfo; + stateChoice: USStateChoice | null; +}; type MapboxCityAutocompleteProps = Omit< SearchAutocompleteProps, @@ -51,24 +55,24 @@ export const mapboxCityAutocompleteHelpers: SearchAutocompleteHelpers< MapboxResults > = { itemToKey(item) { - return [item.city, item.stateCode].join("_"); + return [item.city, item.stateChoice].join("_"); }, itemToString(item) { return item - ? item.stateName - ? `${item.city}, ${item.stateName}` + ? item.stateChoice + ? `${item.city}, ${getUSStateChoiceLabels()[item.stateChoice]}` : item.city : ""; }, searchResultsToItems(results) { const items: MapboxCityItem[] = []; for (let feature of results.features) { - const stateInfo = getMapboxStateInfo(feature); - if (stateInfo) { + const stateChoice = getMapboxStateChoice(feature); + if (stateChoice) { items.push({ city: feature.text, mapboxFeature: feature, - ...stateInfo, + stateChoice, }); } } @@ -78,8 +82,7 @@ export const mapboxCityAutocompleteHelpers: SearchAutocompleteHelpers< return { city: value || "", mapboxFeature: null, - stateCode: "", - stateName: "", + stateChoice: null, }; }, createSearchRequester: (options) => new MapboxCitySearchRequester(options), diff --git a/frontend/lib/forms/mapbox/common.tsx b/frontend/lib/forms/mapbox/common.tsx index ff8902fe4..d8f89d75b 100644 --- a/frontend/lib/forms/mapbox/common.tsx +++ b/frontend/lib/forms/mapbox/common.tsx @@ -41,11 +41,6 @@ export type MapboxFeature = { context: Array & { short_code?: string }>; }; -export type MapboxStateInfo = { - stateCode: string; - stateName: string; -}; - const MAPBOX_PLACES_URL = "https://api.mapbox.com/geocoding/v5/mapbox.places"; const MAPBOX_STATE_SHORT_CODE_RE = /^US-([A-Z][A-Z])$/; @@ -75,7 +70,7 @@ function mapboxSearchOptionsToURLSearchParams( }); } -function stateCodeFromShortCode(shortCode?: string): USStateChoice | null { +function stateChoiceFromShortCode(shortCode?: string): USStateChoice | null { if (shortCode === "pr") return "PR"; const match = (shortCode || "").match(MAPBOX_STATE_SHORT_CODE_RE); const state = match ? match[1] : ""; @@ -85,13 +80,13 @@ function stateCodeFromShortCode(shortCode?: string): USStateChoice | null { return null; } -export function getMapboxStateInfo( +export function getMapboxStateChoice( feature: MapboxFeature -): MapboxStateInfo | null { +): USStateChoice | null { for (let context of feature.context) { - const stateCode = stateCodeFromShortCode(context.short_code); - if (stateCode && context.text) { - return { stateCode, stateName: context.text }; + const stateChoice = stateChoiceFromShortCode(context.short_code); + if (stateChoice) { + return stateChoice; } } return null; diff --git a/frontend/lib/forms/mapbox/tests/city-autocomplete.test.tsx b/frontend/lib/forms/mapbox/tests/city-autocomplete.test.tsx index c5712ffb4..a7339a0b2 100644 --- a/frontend/lib/forms/mapbox/tests/city-autocomplete.test.tsx +++ b/frontend/lib/forms/mapbox/tests/city-autocomplete.test.tsx @@ -7,15 +7,13 @@ import { BROOKLYN_MAPBOX_RESULTS, BROOKLYN_MAPBOX_FEATURE } from "./data"; const BROOKLYN_CITY: MapboxCityItem = { city: "Brooklyn", mapboxFeature: BROOKLYN_MAPBOX_FEATURE, - stateCode: "NY", - stateName: "New York", + stateChoice: "NY", }; const INCOMPLETE_CITY: MapboxCityItem = { city: "blargblarg", mapboxFeature: null, - stateCode: "", - stateName: "", + stateChoice: null, }; describe("mapboxCityAutocompleteHelpers", () => { @@ -39,8 +37,7 @@ describe("mapboxCityAutocompleteHelpers", () => { expect(helpers.getIncompleteItem("blarf")).toEqual({ city: "blarf", mapboxFeature: null, - stateCode: "", - stateName: "", + stateChoice: null, }); }); }); diff --git a/frontend/lib/forms/mapbox/tests/common.test.tsx b/frontend/lib/forms/mapbox/tests/common.test.tsx index 96069763b..24958c86d 100644 --- a/frontend/lib/forms/mapbox/tests/common.test.tsx +++ b/frontend/lib/forms/mapbox/tests/common.test.tsx @@ -1,23 +1,17 @@ -import { getMapboxStateInfo, createMapboxPlacesURL } from "../common"; +import { getMapboxStateChoice, createMapboxPlacesURL } from "../common"; import { BROOKLYN_MAPBOX_FEATURE, SAN_JUAN_MAPBOX_FEATURE } from "./data"; -describe("getMapboxStateInfo", () => { - it("returns state info when state is found", () => { - expect(getMapboxStateInfo(BROOKLYN_MAPBOX_FEATURE)).toEqual({ - stateCode: "NY", - stateName: "New York", - }); +describe("getMapboxStateChoice", () => { + it("returns state choice when state is found", () => { + expect(getMapboxStateChoice(BROOKLYN_MAPBOX_FEATURE)).toEqual("NY"); }); it("works with puerto rico", () => { - expect(getMapboxStateInfo(SAN_JUAN_MAPBOX_FEATURE)).toEqual({ - stateCode: "PR", - stateName: "Puerto Rico", - }); + expect(getMapboxStateChoice(SAN_JUAN_MAPBOX_FEATURE)).toEqual("PR"); }); it("returns null when no state info was found", () => { - expect(getMapboxStateInfo({ context: [] } as any)).toBe(null); + expect(getMapboxStateChoice({ context: [] } as any)).toBe(null); }); });