Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ export function DesktopShell({
let lastLayerId: string | null = null;
for (const layer of importedLayers) {
lastLayerId = addGeoJsonLayer(
layerNameFromPath(layer.path),
layer.name ?? layerNameFromPath(layer.path),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

UX regression (medium confidence): addImportedVectorLayers only calls fitLayer on the last added layer. Before this PR a GPX always produced one layer, so the fit was always correct. Now it can produce up to three — the last one is always Routes (after the [Waypoints, Tracks, Routes] ordering in parseGpxTextLayers). For a file with only waypoints and tracks the view zooms to the tracks layer; for a file with all three it zooms to routes — the user may not see their waypoints at all if they're in a different region.

A simple fix is to compute a union bounding box across all newly-added layers. If mapControllerRef.current exposes a multi-layer fit method (or one can be added), calling it after the loop would resolve this. Otherwise, falling back to the first layer rather than the last would be less surprising as a convention.

layer.data,
layer.path,
);
Expand Down
81 changes: 72 additions & 9 deletions apps/geolibre-desktop/src/lib/tauri-io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { unzip } from "fflate";
import type { FeatureCollection } from "geojson";
import shp from "shpjs";
import type { DuckDbVectorFile } from "./duckdb-vector-loader";
import { parseGpxLayer } from "./gpx";

export function isTauri(): boolean {
return typeof window !== "undefined" && "__TAURI_INTERNALS__" in window;
Expand Down Expand Up @@ -96,6 +97,12 @@ interface SaveBinaryFileOptions extends SaveTextFileOptions {}

const SHAPEFILE_SIDECAR_EXTENSIONS = ["dbf", "shx", "prj", "cpg"];

export interface LoadedVectorLayer {
data: FeatureCollection;
name?: string;
path: string;
}

// Auxiliary files that accompany Shapefiles (spatial indexes, metadata, etc.)
// but are never standalone vector layers. Skipping them keeps a single such
// file from aborting an otherwise valid drag-and-drop import.
Expand Down Expand Up @@ -191,6 +198,31 @@ async function parseGeoJsonText(
return assertFeatureCollection(JSON.parse(text));
}

function parseGpxText(text: string): FeatureCollection {
const result = parseGpxLayer(text);
return mergeFeatureCollections([
result.waypoints,
result.tracks,
result.routes,
]);
}

function parseGpxTextLayers(text: string, path: string): LoadedVectorLayer[] {
const result = parseGpxLayer(text);
const baseName = pathWithoutExtension(browserSafeFileName(path)) || "GPX";
return [
{ data: result.waypoints, label: "Waypoints" },
{ data: result.tracks, label: "Tracks" },
{ data: result.routes, label: "Routes" },
]
.filter((layer) => layer.data.features.length > 0)
.map((layer) => ({
data: layer.data,
name: `${baseName} ${layer.label}`,
path,
}));
Comment on lines +210 to +223

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nits (grouped, low confidence):

  1. The intermediate objects use a label property that has no declared type and is immediately consumed in .map() to produce name. TypeScript infers this fine, but the intent can be made clearer with a typed tuple or a simple const LAYERS with an explicit type annotation.

  2. loadTauriVectorFile keeps its original return type Promise<{ data: FeatureCollection; path: string }> even though LoadedVectorLayer now exists. Structurally it's compatible (the name? field is optional), but updating the annotation would document intent and unify the codebase.

Neither is a correctness issue, just readability.

}

async function parseShapefileZip(
data: ArrayBuffer | Uint8Array,
): Promise<FeatureCollection> {
Expand Down Expand Up @@ -270,10 +302,7 @@ async function fileToDuckDbVectorFile(file: File): Promise<DuckDbVectorFile> {
async function loadBrowserVectorFile(
file: File,
siblingFiles: DuckDbVectorFile[] = [],
): Promise<{
data: FeatureCollection;
path: string;
}> {
): Promise<LoadedVectorLayer> {
const extension = fileExtension(file.name);
if (extension === "geojson" || extension === "json") {
try {
Expand Down Expand Up @@ -305,6 +334,13 @@ async function loadBrowserVectorFile(
};
}

if (extension === "gpx") {
return {
data: parseGpxText(await file.text()),
path: file.name,
};
}

return {
data: await loadDuckDbVector({
name: file.name,
Expand Down Expand Up @@ -391,6 +427,18 @@ async function loadTauriVectorFile(path: string): Promise<{
}
}

if (extension === "gpx") {
try {
return {
data: parseGpxText(await readTextFile(path)),
path,
};
} catch (error) {
const detail = error instanceof Error ? error.message : "Unknown error";
throw new Error(`Could not read this GPX file. ${detail}`);
}
}

try {
const siblingFiles =
extension === "shp" ? await readShapefileSiblings(path) : [];
Expand Down Expand Up @@ -884,7 +932,7 @@ export async function openVectorFileWithFallback(): Promise<{

export async function loadDroppedVectorFiles(
droppedFiles: FileList | File[],
): Promise<Array<{ data: FeatureCollection; path: string }>> {
): Promise<LoadedVectorLayer[]> {
const droppedFileArray = Array.from(droppedFiles);
const files = droppedFileArray.filter((file) =>
isVectorFileName(file.name),
Expand All @@ -900,11 +948,16 @@ export async function loadDroppedVectorFiles(
]);
}

const layers: Array<{ data: FeatureCollection; path: string }> = [];
const layers: LoadedVectorLayer[] = [];
for (const file of files) {
const extension = fileExtension(file.name);
if (SHAPEFILE_SIDECAR_EXTENSIONS.includes(extension)) continue;

if (extension === "gpx") {
layers.push(...parseGpxTextLayers(await file.text(), file.name));
continue;
}
Comment on lines +956 to +959

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent error handling (medium confidence): The Tauri loadDroppedVectorPaths path (added in this same PR) wraps GPX parse errors with "Could not read this GPX file. ${detail}" before rethrowing. The browser path here does not — a parse error from parseGpxLayer (e.g. "The GPX file is not valid XML." or "No valid GPX waypoints, routes, or tracks were found.") propagates raw to whatever catch block the caller has.

For parity you could wrap the same way:

Suggested change
if (extension === "gpx") {
layers.push(...parseGpxTextLayers(await file.text(), file.name));
continue;
}
if (extension === "gpx") {
try {
layers.push(...parseGpxTextLayers(await file.text(), file.name));
} catch (error) {
const detail = error instanceof Error ? error.message : "Unknown error";
throw new Error(`Could not read this GPX file. ${detail}`);
}
continue;
}


const siblingFiles =
extension === "shp"
? await Promise.all(
Expand All @@ -928,13 +981,23 @@ export async function loadDroppedVectorFiles(

export async function loadDroppedVectorPaths(
paths: string[],
): Promise<Array<{ data: FeatureCollection; path: string }>> {
): Promise<LoadedVectorLayer[]> {
const vectorPaths = paths.filter(isVectorFileName);
if (!vectorPaths.length) return [];

const layers: Array<{ data: FeatureCollection; path: string }> = [];
const layers: LoadedVectorLayer[] = [];
for (const path of vectorPaths) {
if (SHAPEFILE_SIDECAR_EXTENSIONS.includes(fileExtension(path))) continue;
const extension = fileExtension(path);
if (SHAPEFILE_SIDECAR_EXTENSIONS.includes(extension)) continue;
if (extension === "gpx") {
try {
layers.push(...parseGpxTextLayers(await readTextFile(path), path));
} catch (error) {
const detail = error instanceof Error ? error.message : "Unknown error";
throw new Error(`Could not read this GPX file. ${detail}`);
}
continue;
}
layers.push(await loadTauriVectorFile(path));
}

Expand Down