Skip to content

Commit 29f4f70

Browse files
committed
polish map location search
1 parent d80b601 commit 29f4f70

4 files changed

Lines changed: 134 additions & 82 deletions

File tree

e2e/helpers.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ type MockGeocodingFeatureOptions = {
1313
id?: string;
1414
text?: string;
1515
placeName?: string;
16+
placeType?: string[];
17+
context?: Array<{
18+
id: string;
19+
text: string;
20+
}>;
1621
center?: [number, number];
1722
countryCode?: string;
1823
};
@@ -21,6 +26,8 @@ export function createMockGeocodingFeature({
2126
id = "place.123",
2227
text = "Newtown",
2328
placeName = "Newtown, New South Wales, Australia",
29+
placeType = ["place"],
30+
context,
2431
center = [151.1781, -33.8985],
2532
countryCode = "AU",
2633
}: MockGeocodingFeatureOptions = {}) {
@@ -29,8 +36,9 @@ export function createMockGeocodingFeature({
2936
type: "Feature",
3037
text,
3138
place_name: placeName,
32-
place_type: ["place"],
33-
place_type_name: ["Place"],
39+
place_type: placeType,
40+
place_type_name: placeType,
41+
context,
3442
center,
3543
bbox: [center[0], center[1], center[0], center[1]],
3644
geometry: {

e2e/listings.spec.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -96,27 +96,42 @@ test("listing location search picks a geocoding result", async ({ page }) => {
9696
await mockMapTilerGeocoding(
9797
page,
9898
createMockGeocodingFeature({
99-
text: "Newtown",
100-
placeName: "Newtown, New South Wales, Australia",
101-
center: [151.1781, -33.8985],
99+
id: "poi.mount-kuring-gai-station",
100+
text: "Mount Kuring-gai Station",
101+
placeName:
102+
"Mount Kuring-gai Station, Mount Kuring-gai, Sydney, Australia",
103+
placeType: ["poi"],
104+
context: [
105+
{
106+
id: "neighbourhood.mount-kuring-gai",
107+
text: "Mount Kuring-gai",
108+
},
109+
{
110+
id: "place.sydney",
111+
text: "Sydney",
112+
},
113+
],
114+
center: [151.1368, -33.6546],
102115
})
103116
);
104117

105118
await expect(page.getByTestId("listing-write-form")).toBeVisible();
106119
await page.locator("#country").selectOption("AU");
107120
const searchInput = page.getByTestId("listing-location-search-input");
108-
await searchInput.fill("Newtown");
121+
await searchInput.fill("Mount Kuring-gai");
109122
await expect
110123
.poll(async () =>
111124
searchInput.evaluate((element) =>
112125
Number.parseFloat(getComputedStyle(element).fontSize)
113126
)
114127
)
115128
.toBeGreaterThanOrEqual(16);
116-
await page.getByRole("option", { name: /Newtown/ }).click();
129+
await page.getByRole("option", { name: /Mount Kuring-gai Station/ }).click();
117130

118-
await expect(page.getByRole("option", { name: /Newtown/ })).toHaveCount(0);
119-
await expect(searchInput).toHaveValue("Newtown");
131+
await expect(
132+
page.getByRole("option", { name: /Mount Kuring-gai Station/ })
133+
).toHaveCount(0);
134+
await expect(searchInput).toHaveValue("Mount Kuring-gai");
120135
await expect(page.locator(".maplibregl-canvas")).toBeVisible({
121136
timeout: 10_000,
122137
});
@@ -142,9 +157,8 @@ test("listing edit saves and restores seeded business fields", async ({
142157
: ALTERNATE_BUSINESS_DESCRIPTION;
143158
const updatedVisibility = originalVisibility === "true" ? "false" : "true";
144159

145-
await descriptionInput.fill("");
146-
await expect(descriptionInput).toHaveValue("");
147160
await descriptionInput.fill(updatedDescription);
161+
await expect(descriptionInput).toHaveValue(updatedDescription);
148162
await visibilityInput.selectOption(updatedVisibility);
149163
await delayServerActionRequests(page);
150164

@@ -167,14 +181,13 @@ test("listing edit saves and restores seeded business fields", async ({
167181
updatedVisibility
168182
);
169183

170-
await listingWriteForm.locator("#description").first().fill("");
171-
await expect(listingWriteForm.locator("#description").first()).toHaveValue(
172-
""
173-
);
174184
await listingWriteForm
175185
.locator("#description")
176186
.first()
177187
.fill(originalDescription);
188+
await expect(listingWriteForm.locator("#description").first()).toHaveValue(
189+
originalDescription
190+
);
178191
await listingWriteForm
179192
.locator("#visibility")
180193
.first()

src/components/LocationSelect/LocationSelect.tsx

Lines changed: 97 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,30 @@ type Coordinates = {
4848
longitude: number;
4949
};
5050

51+
type AreaNameFeature = {
52+
id?: string;
53+
place_name?: string;
54+
place_type?: string[];
55+
properties?: {
56+
"osm:place_type"?: string;
57+
};
58+
text?: string;
59+
"osm:place_type"?: string;
60+
};
61+
62+
type AreaNameMatch = {
63+
name: string;
64+
priority: number;
65+
};
66+
67+
const areaNameTypePriority = [
68+
"neighbourhood",
69+
"place",
70+
"municipality",
71+
"region",
72+
"country",
73+
"continental_marine",
74+
] as const;
5175
type LocationSelectProps = {
5276
listingType: string;
5377
coordinates: Coordinates | null;
@@ -62,65 +86,68 @@ type LocationSelectProps = {
6286
error?: string;
6387
};
6488

65-
async function getAreaName(
66-
longitude: number,
67-
latitude: number
68-
): Promise<string> {
69-
const coordinates = await geocoding.reverse([longitude, latitude]);
89+
function featureMatchesAreaType(feature: AreaNameFeature, type: string) {
90+
return (
91+
feature.place_type?.includes(type) ||
92+
feature.id?.startsWith(`${type}.`) ||
93+
feature.id?.startsWith(`${type}:`)
94+
);
95+
}
7096

71-
const features = coordinates.features as any[];
97+
function isUnknownOsmPlace(feature: AreaNameFeature) {
98+
return (
99+
feature.properties?.["osm:place_type"] === "unknown" ||
100+
feature["osm:place_type"] === "unknown"
101+
);
102+
}
72103

73-
if (!features || features.length === 0) {
74-
return "";
104+
function getBestAreaNameMatchFromFeatures(
105+
features: AreaNameFeature[]
106+
): AreaNameMatch | null {
107+
if (!features.length) {
108+
return null;
75109
}
76110

77-
// Helper function to find feature by place type
78-
const findFeatureByType = (features: any[], types: string[]) => {
79-
return features.find((f) =>
80-
types.some(
81-
(type) =>
82-
f.place_type?.includes(type) &&
83-
// Some places are coming up as 'storage', with a correlation to prpoerties.osm:place_type being "unknown"
84-
// E.g. see Jo's listing in Fitzroy
85-
// Skip these
86-
f.properties[`osm:place_type`] !== "unknown"
87-
)
111+
for (const [priority, type] of areaNameTypePriority.entries()) {
112+
const areaFeature = features.find(
113+
(feature) =>
114+
featureMatchesAreaType(feature, type) && !isUnknownOsmPlace(feature)
88115
);
89-
};
90116

91-
// Look for features in order of specificity
92-
const neighbourhood = findFeatureByType(features, ["neighbourhood"]);
93-
const place = findFeatureByType(features, ["place"]);
94-
const municipality = findFeatureByType(features, ["municipality"]);
95-
const region = findFeatureByType(features, ["region"]);
96-
const country = findFeatureByType(features, ["country"]);
97-
const marine = findFeatureByType(features, ["continental_marine"]);
98-
99-
let areaName = "";
100-
101-
// Build location string based on available information
102-
103-
if (place) {
104-
areaName = place.text;
105-
// } else if (place && region) {
106-
// areaName = `${place.text}, ${region.text}`;
107-
// } else if (municipality && region) {
108-
// areaName = `${municipality.text}, ${region.text}`;
109-
} else if (neighbourhood) {
110-
areaName = neighbourhood.text;
111-
} else if (municipality) {
112-
areaName = municipality.text;
113-
} else if (region) {
114-
areaName = region.text;
115-
} else if (country) {
116-
areaName = country.text;
117-
} else if (marine) {
118-
areaName = marine.text;
119-
} else {
120-
// Fallback to the most relevant feature's place name
121-
areaName = features[0].place_name || "";
117+
if (areaFeature?.text) {
118+
return {
119+
name: areaFeature.text,
120+
priority,
121+
};
122+
}
122123
}
123-
return areaName;
124+
125+
const fallbackName = features[0]?.place_name || features[0]?.text || "";
126+
127+
return fallbackName
128+
? {
129+
name: fallbackName,
130+
priority: areaNameTypePriority.length,
131+
}
132+
: null;
133+
}
134+
135+
function getSelectedFeatureAreaNameMatch(feature: GeocodingFeature) {
136+
return getBestAreaNameMatchFromFeatures([
137+
feature,
138+
...((feature.context as AreaNameFeature[] | undefined) ?? []),
139+
]);
140+
}
141+
142+
async function getAreaNameMatch(
143+
longitude: number,
144+
latitude: number
145+
): Promise<AreaNameMatch | null> {
146+
const coordinates = await geocoding.reverse([longitude, latitude]);
147+
148+
return getBestAreaNameMatchFromFeatures(
149+
coordinates.features as AreaNameFeature[]
150+
);
124151
}
125152

126153
// TODO: use this to build a custom component around the core geocoding API, using my nice own components for input and dropdown
@@ -159,7 +186,7 @@ export default function LocationSelect({
159186
const inputRef = useRef<GeocodingSearchHandle | null>(null);
160187

161188
const [mapShown, setMapShown] = useState(coordinates ? true : false);
162-
const [placeholderText, setPlaceholderText] = useState(
189+
const [placeholderText] = useState(
163190
initialPlaceholderText || t("Listings.form.locationPlaceholder")
164191
);
165192
const [searchStatusMessage, setSearchStatusMessage] = useState("");
@@ -206,9 +233,7 @@ export default function LocationSelect({
206233
const handleDragStart = useCallback(() => {
207234
inputRef.current?.blur(); // Close and blur the input if it's open
208235
console.log("handling drag start");
209-
inputRef.current?.setQuery("");
210-
setPlaceholderText(t("Listings.form.customLocation")); // Clear previous value
211-
}, [t]);
236+
}, []);
212237

213238
const handleDragEnd = useCallback(
214239
async (event: any) => {
@@ -222,12 +247,15 @@ export default function LocationSelect({
222247

223248
setCoordinates(nextCoordinates); // Unsure if this is needed. Might be helpful for form submission
224249

225-
const nextAreaName = await getAreaName(
250+
const nextAreaNameMatch = await getAreaNameMatch(
226251
nextCoordinates.longitude,
227252
nextCoordinates.latitude
228253
);
229-
setAreaName(nextAreaName);
230-
setPlaceholderText(nextAreaName);
254+
255+
if (nextAreaNameMatch) {
256+
setAreaName(nextAreaNameMatch.name);
257+
inputRef.current?.setQuery(nextAreaNameMatch.name);
258+
}
231259
},
232260
[onLocationInteract, setCoordinates, setAreaName]
233261
);
@@ -245,13 +273,17 @@ export default function LocationSelect({
245273
longitude: feature.center[0],
246274
};
247275

248-
// Get area name from coordinates
249-
const nextAreaName = await getAreaName(
250-
nextCoordinates.longitude,
251-
nextCoordinates.latitude
252-
);
276+
const selectedAreaNameMatch = getSelectedFeatureAreaNameMatch(feature);
277+
const reverseAreaNameMatch = selectedAreaNameMatch
278+
? null
279+
: await getAreaNameMatch(
280+
nextCoordinates.longitude,
281+
nextCoordinates.latitude
282+
);
283+
const nextAreaNameMatch = selectedAreaNameMatch ?? reverseAreaNameMatch;
284+
const nextAreaName = nextAreaNameMatch?.name || feature.place_name;
253285
setAreaName(nextAreaName);
254-
inputRef.current?.setQuery(nextAreaName || feature.place_name);
286+
inputRef.current?.setQuery(nextAreaName);
255287

256288
inputRef.current?.blur();
257289

src/features/map/components/GeocodingSearch.tsx

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ const ClearButton = styled.button`
100100
appearance: none;
101101
border: 0;
102102
background: transparent;
103-
color: ${theme.colors.text.secondary};
103+
color: ${theme.colors.text.ui.tertiary};
104104
cursor: pointer;
105105
display: flex;
106106
align-items: center;
@@ -114,7 +114,6 @@ const ClearButton = styled.button`
114114
border-radius: 0.7rem;
115115
116116
&:hover {
117-
color: ${theme.colors.text.primary};
118117
background: ${theme.colors.background.sunk};
119118
}
120119

0 commit comments

Comments
 (0)