Skip to content

Commit

Permalink
Improve search and elevation handling
Browse files Browse the repository at this point in the history
- Move search and elevation functionality to utils (and introduce
  config/fetch adapter)
- Load search results and elevation data directly in frontend where
  possible
- Set marker elevation asynchronously
- Load search results elevation asynchronously
- Fix click marker bugs
- Load click marker reverse geocode info asynchronously (fixes #25)
- Stabilize handling of 504 errors in elevation lookup
- Add migration to add elevation to existing markers
  • Loading branch information
cdauth committed Mar 24, 2024
1 parent 2cc372f commit 028cd72
Show file tree
Hide file tree
Showing 33 changed files with 923 additions and 692 deletions.
1 change: 0 additions & 1 deletion docs/src/developers/client/methods.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ Search for places. Does not persist anything on the server, simply serves as a p
* `data` (object): An object with the following properties:
* `query` (string): The query string
* `loadUrls` (boolean): Whether to return the file if `query` is a URL
* `elevation` (boolean): Whether to find out the elevation of the result(s). Will make the search significantly slower.
* **Returns:** A promise that is resolved with the following value:
* If `data.query` is a URL to a GPX/KML/OSM/GeoJSON file and `loadUrls` is `true`, a string with the content of the file.
* Otherwise an array of [SearchResults](./types.md#searchresult).
Expand Down
1 change: 1 addition & 0 deletions frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"mitt": "^3.0.1",
"osmtogeojson": "^3.0.0-beta.5",
"p-debounce": "^4.0.0",
"p-throttle": "^6.1.0",
"pluralize": "^8.0.0",
"popper-max-size-modifier": "^0.2.0",
"qrcode.vue": "^3.4.1",
Expand Down
103 changes: 65 additions & 38 deletions frontend/src/lib/components/click-marker-tab.vue
Original file line number Diff line number Diff line change
@@ -1,63 +1,90 @@
<script setup lang="ts">
import type { SearchResult } from "facilmap-types";
import { round } from "facilmap-utils";
import { find, getElevationForPoint, getFallbackLonLatResult, round } from "facilmap-utils";
import { SearchResultsLayer } from "facilmap-leaflet";
import SearchResultInfo from "./search-result-info.vue";
import { Util } from "leaflet";
import { computed, markRaw, nextTick, readonly, ref, shallowReactive, toRef, watch } from "vue";
import { computed, markRaw, nextTick, reactive, readonly, ref, toRef, watch, type Raw } from "vue";
import { useEventListener } from "../utils/utils";
import SearchBoxTab from "./search-box/search-box-tab.vue"
import { injectContextRequired, requireClientContext, requireMapContext, requireSearchBoxContext } from "./facil-map-context-provider/facil-map-context-provider.vue";
import { injectContextRequired, requireMapContext, requireSearchBoxContext } from "./facil-map-context-provider/facil-map-context-provider.vue";
import type { WritableClickMarkerTabContext } from "./facil-map-context-provider/click-marker-tab-context";
import { useToasts } from "./ui/toasts/toasts.vue";
const toasts = useToasts();
const context = injectContextRequired();
const mapContext = requireMapContext(context);
const client = requireClientContext(context);
const searchBoxContext = requireSearchBoxContext(context);
let lastClick = 0;
type Tab = {
id: number;
result: SearchResult;
layer: Raw<SearchResultsLayer>;
isLoading: boolean;
};
const activeResults = ref<SearchResult[]>([]);
const layers = shallowReactive<SearchResultsLayer[]>([]);
const tabs = ref<Tab[]>([]);
useEventListener(mapContext, "open-selection", handleOpenSelection);
const layerIds = computed(() => layers.map((layer) => Util.stamp(layer)));
const layerIds = computed(() => tabs.value.map((tab) => Util.stamp(tab.layer)));
watch(() => mapContext.value.selection, () => {
for (let i = activeResults.value.length - 1; i >= 0; i--) {
for (let i = tabs.value.length - 1; i >= 0; i--) {
if (!mapContext.value.selection.some((item) => item.type == "searchResult" && item.layerId == layerIds.value[i]))
close(activeResults.value[i]);
close(tabs.value[i]);
}
});
let idCounter = 1;
const clickMarkerTabContext = ref<WritableClickMarkerTabContext>({
async openClickMarker(point) {
const now = Date.now();
lastClick = now;
const result = reactive(getFallbackLonLatResult({ lat: point.lat, lon: point.lon, zoom: mapContext.value.zoom }));
const layer = markRaw(new SearchResultsLayer([result]).addTo(mapContext.value.components.map));
mapContext.value.components.selectionHandler.addSearchResultLayer(layer);
const results = await client.value.find({
query: `geo:${round(point.lat, 5)},${round(point.lon, 5)}?z=${mapContext.value.zoom}`,
loadUrls: false,
elevation: true
const tab = reactive<Tab>({
id: idCounter++,
result,
layer,
isLoading: true
});
if (now !== lastClick) {
// There has been another click since the one we are reacting to.
return;
}
tabs.value.push(tab);
mapContext.value.components.selectionHandler.setSelectedItems([{ type: "searchResult", result, layerId: Util.stamp(layer) }]);
if (results.length > 0) {
const layer = new SearchResultsLayer([results[0]]).addTo(mapContext.value.components.map);
mapContext.value.components.selectionHandler.addSearchResultLayer(layer);
await nextTick();
searchBoxContext.value.activateTab(`fm${context.id}-click-marker-tab-${tabs.value.length - 1}`, { expand: true });
activeResults.value.push(results[0]);
layers.push(markRaw(layer));
(async () => {
const results = await mapContext.value.runOperation(async () => await find(`geo:${round(point.lat, 5)},${round(point.lon, 5)}?z=${mapContext.value.zoom}`));
mapContext.value.components.selectionHandler.setSelectedItems([{ type: "searchResult", result: results[0], layerId: Util.stamp(layer) }]);
if (results.length > 0) {
tab.result = { ...results[0], elevation: tab.result.elevation };
}
tab.isLoading = false;
})().catch((err) => {
toasts.showErrorToast(`find-error-${tab.id}`, "Error looking up point", err);
});
(async () => {
const elevation = await getElevationForPoint(point);
if (elevation != null) {
tab.result.elevation = elevation;
}
})().catch((err) => {
console.warn("Error fetching click marker elevation", err);
});
},
await nextTick();
searchBoxContext.value.activateTab(`fm${context.id}-click-marker-tab-${activeResults.value.length - 1}`, { expand: true });
closeLastClickMarker() {
if (tabs.value.length > 0) {
close(tabs.value[tabs.value.length - 1]);
}
}
});
Expand All @@ -73,27 +100,27 @@
}
}
function close(result: SearchResult): void {
const idx = activeResults.value.indexOf(result);
function close(tab: Tab): void {
const idx = tabs.value.indexOf(tab);
if (idx == -1)
return;
mapContext.value.components.selectionHandler.removeSearchResultLayer(layers[idx]);
layers[idx].remove();
activeResults.value.splice(idx, 1);
layers.splice(idx, 1);
toasts.hideToast(`find-error-${tab.id}`);
mapContext.value.components.selectionHandler.removeSearchResultLayer(tab.layer);
tab.layer.remove();
tabs.value.splice(idx, 1);
}
</script>

<template>
<template v-for="(result, idx) in activeResults" :key="result.id">
<template v-for="(tab, idx) in tabs" :key="tab.id">
<SearchBoxTab
:id="`fm${context.id}-click-marker-tab-${idx}`"
:title="result.short_name"
:title="tab.result.short_name"
isCloseable
@close="close(result)"
@close="close(tab)"
>
<SearchResultInfo :result="result"></SearchResultInfo>
<SearchResultInfo :result="tab.result" :isLoading="tab.isLoading"></SearchResultInfo>
</SearchBoxTab>
</template>
</template>
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import type { Point } from "facilmap-types";

export interface WritableClickMarkerTabContext {
openClickMarker(point: Point): Promise<void>;
closeLastClickMarker(): void;
}

export type ClickMarkerTabContext = Readonly<WritableClickMarkerTabContext>;
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ export type MapContextData = {
components: MapComponents;
loaded: boolean;
fatalError: string | undefined;
/** Increase mapContext.loading while the given async function is running. */
runOperation: <R>(operation: () => Promise<R>) => Promise<R>;
};

export type WritableMapContext = MapContextData & Emitter<MapContextEvents>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,10 @@ function useSelectionHandler(map: Ref<Map>, context: FacilMapContext, mapContext
void context.components.clickMarkerTab?.openClickMarker({ lat: event.latlng.lat, lon: event.latlng.lng });
});

selectionHandler.on("fmLongClickAbort", () => {
context.components.clickMarkerTab?.closeLastClickMarker();
});

return selectionHandler;
},
(selectionHandler, onCleanup) => {
Expand Down Expand Up @@ -343,7 +347,15 @@ export async function useMapContext(context: FacilMapContext, mapRef: Ref<HTMLEl
overpassCustom: "",
overpassMessage: undefined,
loaded: false,
fatalError: undefined
fatalError: undefined,
runOperation: async (operation) => {
try {
mapContextWithoutComponents.loading++;
return await operation();
} finally {
mapContextWithoutComponents.loading--;
}
}
} satisfies Omit<MapContextData, 'components'>));

const mapContext: WritableMapContext = Object.assign(mapContextWithoutComponents, {
Expand Down
7 changes: 1 addition & 6 deletions frontend/src/lib/components/marker-info/marker-info.vue
Original file line number Diff line number Diff line change
Expand Up @@ -89,12 +89,7 @@
</h2>
<dl class="fm-search-box-collapse-point fm-search-box-dl">
<dt class="pos">Coordinates</dt>
<dd class="pos"><Coordinates :point="marker"></Coordinates></dd>

<template v-if="marker.ele != null">
<dt class="elevation">Elevation</dt>
<dd class="elevation">{{marker.ele}}&#x202F;m</dd>
</template>
<dd class="pos"><Coordinates :point="marker" :ele="marker.ele"></Coordinates></dd>

<template v-for="field in client.types[marker.typeId].fields" :key="field.name">
<dt>{{field.name}}</dt>
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/lib/components/save-view-dialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { useToasts } from "./ui/toasts/toasts.vue";
import { computed, ref } from "vue";
import { getUniqueId, getZodValidator, validateRequired } from "../utils/utils";
import { round } from "facilmap-utils";
import { formatCoordinates } from "facilmap-utils";
import { injectContextRequired, requireClientContext, requireMapContext } from "./facil-map-context-provider/facil-map-context-provider.vue";
import ValidatedField from "./ui/validated-form/validated-field.vue";
import { viewValidator } from "facilmap-types";
Expand Down Expand Up @@ -100,7 +100,7 @@
class="form-control-plaintext"
readonly
:id="`${id}-topleft-input`"
:value="`${round(mapContext.bounds.getNorth(), 5)}, ${round(mapContext.bounds.getWest(), 5)}`"
:value="formatCoordinates({ lat: mapContext.bounds.getNorth(), lon: mapContext.bounds.getWest() })"
/>
</div>
</div>
Expand All @@ -112,7 +112,7 @@
class="form-control-plaintext"
readonly
:id="`${id}-bottomright-input`"
:value="`${round(mapContext.bounds.getSouth(), 5)}, ${round(mapContext.bounds.getEast(), 5)}`"
:value="formatCoordinates({ lat: mapContext.bounds.getSouth(), lon: mapContext.bounds.getEast() })"
/>
</div>
</div>
Expand Down
27 changes: 22 additions & 5 deletions frontend/src/lib/components/search-form/search-form.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script setup lang="ts">
import Icon from "../ui/icon.vue";
import { isSearchId } from "facilmap-utils";
import { find, getElevationForPoint, isSearchId, parseUrlQuery } from "facilmap-utils";
import { useToasts } from "../ui/toasts/toasts.vue";
import type { FindOnMapResult, SearchResult } from "facilmap-types";
import SearchResults from "../search-results/search-results.vue";
Expand All @@ -11,7 +11,7 @@
import type { HashQuery } from "facilmap-leaflet";
import { type FileResultObject, parseFiles } from "../../utils/files";
import FileResults from "../file-results.vue";
import { computed, ref, watch } from "vue";
import { computed, reactive, ref, watch } from "vue";
import DropdownMenu from "../ui/dropdown-menu.vue";
import { injectContextRequired, requireClientContext, requireMapContext } from "../facil-map-context-provider/facil-map-context-provider.vue";
Expand Down Expand Up @@ -87,8 +87,10 @@
const query = searchString.value;
loadingSearchString.value = searchString.value;
const url = parseUrlQuery(query);
const [newSearchResults, newMapResults] = await Promise.all([
client.value.find({ query, loadUrls: true, elevation: true }),
url ? client.value.find({ query, loadUrls: true }) : mapContext.value.runOperation(async () => await find(query)),
client.value.padData ? client.value.findOnMap({ query }) : undefined
]);
Expand All @@ -106,13 +108,28 @@
if(typeof newSearchResults == "string") {
searchResults.value = undefined;
mapResults.value = undefined;
fileResult.value = await parseFiles([ newSearchResults ]);
fileResult.value = await mapContext.value.runOperation(async () => await parseFiles([ newSearchResults ]));
mapContext.value.components.searchResultsLayer.setResults(fileResult.value.features);
} else {
searchResults.value = newSearchResults;
const reactiveResults = reactive(newSearchResults);
searchResults.value = reactiveResults;
mapContext.value.components.searchResultsLayer.setResults(newSearchResults);
mapResults.value = newMapResults ?? undefined;
fileResult.value = undefined;
const points = newSearchResults.filter((res) => (res.lon && res.lat));
if(points.length > 0) {
(async () => {
const elevations = await Promise.all(points.map(async (point) => {
return await getElevationForPoint({ lat: Number(point.lat), lon: Number(point.lon) });
}));
elevations.forEach((elevation, i) => {
reactiveResults[i].elevation = elevation;
});
})().catch((err) => {
console.warn("Error fetching search result elevations", err);
});
}
}
} catch(err) {
toasts.showErrorToast(`fm${context.id}-search-form-error`, "Search error", err);
Expand Down
22 changes: 12 additions & 10 deletions frontend/src/lib/components/search-result-info.vue
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
searchResults?: SearchResult[];
/** If specified, will be passed to the route form as suggestions when using the "Use as" menu */
mapResults?: FindOnMapResult[];
isLoading?: boolean;
}>(), {
showBackButton: false
});
Expand Down Expand Up @@ -63,6 +64,11 @@
{{result.short_name}}
</h2>
<dl class="fm-search-box-collapse-point fm-search-box-dl">
<template v-if="result.lat != null && result.lon != null">
<dt class="pos">Coordinates</dt>
<dd class="pos"><Coordinates :point="result as Point" :ele="result.elevation"></Coordinates></dd>
</template>

<template v-if="result.type">
<dt>Type</dt>
<dd class="text-break">{{result.type}}</dd>
Expand All @@ -73,22 +79,18 @@
<dd class="text-break">{{result.address}}</dd>
</template>

<template v-if="result.type != 'coordinates' && result.lat != null && result.lon != null">
<dt>Coordinates</dt>
<dd><Coordinates :point="result as Point"></Coordinates></dd>
</template>

<template v-if="result.elevation != null">
<dt>Elevation</dt>
<dd>{{result.elevation}}&#x202F;m</dd>
</template>

<template v-for="(value, key) in result.extratags" :key="key">
<dt>{{key}}</dt>
<dd class="text-break" v-html="renderOsmTag(key, value)"></dd>
</template>
</dl>

<template v-if="props.isLoading">
<div class="d-flex justify-content-center mb-3">
<div class="spinner-border"></div>
</div>
</template>

<div class="btn-toolbar">
<ZoomToObjectButton
v-if="zoomDestination"
Expand Down
Loading

0 comments on commit 028cd72

Please sign in to comment.