Consolidate original/GeoJSON download into one menu button

The file name doubling as a GeoJSON-only download link was easy to
miss as a link at all, and only ever covered half of what's actually
downloadable. One button next to delete now opens a small menu
(Original file / GeoJSON) instead -- costs a click when downloading
several files in a row, but both formats are reachable from one,
discoverable place. Only one menu open at a time, closed on any
outside click.

Verified in a real browser: opens with both items, closes on outside
click, no console errors.
This commit is contained in:
randy committed 2026-09-08 00:10:02 +02:00
1 parent 45567fc520
commit 1b4463a3c1
3 files changed
+149 -62

No files matched your search

+27 -24
View File
@@ -342,30 +342,33 @@ Once a `.gpx`/`.fit` is cached, the tree shows it under its converted
name instead — `"ride.gpx"` reads as `"ride.gpx.geojson"`, matching name instead — `"ride.gpx"` reads as `"ride.gpx.geojson"`, matching
the real cache file sitting next to it on disk — since the GeoJSON, the real cache file sitting next to it on disk — since the GeoJSON,
not the original, is what's actually rendered on the map from that not the original, is what's actually rendered on the map from that
point on. The original is never deleted or re-parsed once cached, just point on. Plain text now, not a link — see below. The original is
not re-shown as itself, so both forms need a way out of the app as an never deleted or re-parsed once cached, just not re-shown as itself,
actual file: the file name itself (`GET /api/gpx/file/<path>?download=geojson`) so both forms need a way out of the app as an actual file: one
is a real link, not a `<span>` — clicking it downloads the GeoJSON download button, next to delete, opens a small menu ("Original file" /
rather than also toggling the row's checkbox, since a native "GeoJSON", `_downloadButton`/`_downloadMenuItem`) rather than the name
`<a>`/`<button>` nested inside a `<label>` claims its own click doubling as a GeoJSON-only download link the way it briefly did — that
instead of the label forwarding it to the associated control — and a was easy to miss as a link at all, and only ever covered half of
second button next to delete (`?download=original`) downloads the what's downloadable. Only one menu open at a time
untouched, as-uploaded file, since the name alone only covers half of (`_toggleDownloadMenu`/`_closeDownloadMenu`), closed by a
what's now downloadable and isn't an obvious click target regardless. document-level click listener the same way an outside click closes a
A directly-uploaded `.geojson` keeps its own name and both buttons `sidebar-flyout` dropdown, just scoped to this one control rather than
download the same (identical) content — redundant there, not wrong. threaded through `onlyOneSidePanelOpen`. A directly-uploaded `.geojson`
Both kinds of download require login, same as upload/move/delete/ keeps its own name and both menu items download the same (identical)
create-folder — `GET /api/gpx/file` itself stays open to anyone content — redundant there, not wrong. Both downloads (`GET
(viewing a file on the map isn't a write action), just not /api/gpx/file/<path>?download=original`/`?download=geojson`) require
`?download=...` on it. Since both links are real `<a href>`s (so login, same as upload/move/delete/create-folder, so the whole button —
right-click/middle-click still do something sensible), a logged-out not just its menu items — is left out of the DOM when logged out
click can't just be a plain navigation the way delete/upload already rather than shown and failing on click (see "Authentication" below);
aren't — `_download` (`gpx.js`) fetches instead, checks for a 401 the `GET /api/gpx/file` itself stays open to anyone otherwise (viewing a
same way `redirectIfUnauthorized` already does for every other write file on the map isn't a write action), just not `?download=...` on it.
action, and only then turns the response into a blob and clicks a Each menu item is a real `<a href>` (so right-click/middle-click still
throwaway `<a download>` to trigger the actual save — a plain do something sensible), but a plain navigation would land a logged-out
navigation would otherwise land on a raw JSON error page instead of click on a raw JSON error page instead of being sent to log in —
being sent to log in. `Gpx` parses the GPX with REXML: `<wpt>` `_download` (`gpx.js`) fetches instead, checks for a 401 the same way
`redirectIfUnauthorized` already does for every other write action,
and only then turns the response into a blob and clicks a throwaway
`<a download>` to trigger the actual save. `Gpx` parses the GPX with REXML: `<wpt>`
waypoints become `Point` features (with `name`, `desc` and `time` waypoints become `Point` features (with `name`, `desc` and `time`
carried through as properties), each `<trk>` becomes one carried through as properties), each `<trk>` becomes one
`MultiLineString` feature (one line per `<trkseg>`), and each `<rte>` `MultiLineString` feature (one line per `<trkseg>`), and each `<rte>`
+41
View File
@@ -379,11 +379,20 @@ html, body {
height: 14px; height: 14px;
} }
.gpx-tree__download-wrapper {
position: relative;
flex-shrink: 0;
display: flex;
}
.gpx-tree__download { .gpx-tree__download {
flex-shrink: 0; flex-shrink: 0;
display: flex; display: flex;
border: none;
background: none;
color: #999; color: #999;
padding: 0 4px; padding: 0 4px;
cursor: pointer;
} }
.gpx-tree__download svg { .gpx-tree__download svg {
@@ -395,6 +404,38 @@ html, body {
color: #333; color: #333;
} }
.gpx-tree__download-menu {
position: absolute;
top: 100%;
right: 0;
z-index: 20;
min-width: max-content;
background: #fff;
border: 1px solid #ddd;
border-radius: 6px;
box-shadow: 0 2px 8px rgba(0, 0, 0, 0.15);
padding: 4px;
font-size: 13px;
}
.gpx-tree__download-menu[hidden] {
display: none;
}
.gpx-tree__download-menu-item {
display: block;
padding: 6px 10px;
border-radius: 4px;
color: inherit;
text-decoration: none;
white-space: nowrap;
cursor: pointer;
}
.gpx-tree__download-menu-item:hover {
background: #f0f0f0;
}
.gpx-tree__delete { .gpx-tree__delete {
flex-shrink: 0; flex-shrink: 0;
border: none; border: none;
+81 -38
View File
@@ -53,6 +53,7 @@ export class GpxPanelControl {
this._visible = new Set(); // file paths currently shown on the map this._visible = new Set(); // file paths currently shown on the map
this._interactiveLayers = new Set(); // point layer ids with click/hover wired up this._interactiveLayers = new Set(); // point layer ids with click/hover wired up
this._pointsByPath = new Map(); // path -> its waypoints FeatureCollection, for the category legend this._pointsByPath = new Map(); // path -> its waypoints FeatureCollection, for the category legend
this._openDownloadMenu = null; // the one gpx-tree__download-menu currently open, if any
// setStyle() (switching the Online/Offline base map) tears down all // setStyle() (switching the Online/Offline base map) tears down all
// custom sources/layers/images — re-register the category glyphs before // custom sources/layers/images — re-register the category glyphs before
@@ -130,6 +131,14 @@ export class GpxPanelControl {
onlyOneSidePanelOpen(this._panel, () => this._icon.classList.remove("active")); onlyOneSidePanelOpen(this._panel, () => this._icon.classList.remove("active"));
// Closes whichever per-file download menu is open on any click that
// isn't the toggle button that opened it (that one calls
// stopPropagation so this never fights it) — same "outside click
// closes it" behavior sidebar-flyout dropdowns already have, just not
// wired through onlyOneSidePanelOpen since these aren't full side
// panels.
document.addEventListener("click", () => this._closeDownloadMenu());
this._setupToolbarActions(); this._setupToolbarActions();
this._setupRootDropTarget(); this._setupRootDropTarget();
this._setupMapDropTarget(); this._setupMapDropTarget();
@@ -292,32 +301,15 @@ export class GpxPanelControl {
// name reflects that (matching the real "<path>.geojson" cache file on // name reflects that (matching the real "<path>.geojson" cache file on
// disk) rather than still reading as a plain, un-converted .gpx/.fit. // disk) rather than still reading as a plain, un-converted .gpx/.fit.
// A direct .geojson upload already ends in .geojson, so it's left // A direct .geojson upload already ends in .geojson, so it's left
// alone. // alone. Plain text, not a link — downloading is the download button's
// job now (see below), one obvious place instead of two differently-
// shaped ones (the name used to double as a GeoJSON-only download
// link, easy to miss and only half the story).
const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name; const displayName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
const loggedIn = isLoggedIn(); const loggedIn = isLoggedIn();
// Logged in: an <a>, not a <span> — clicking it downloads that GeoJSON, const name = document.createElement("span");
// the only way to get it out of the app, since the original .gpx/.fit
// is never deleted but also never re-shown once converted. An <a>
// nested in a <label> doesn't also toggle the checkbox on click (unlike
// a plain element), so this doesn't fight the row's own
// toggle-on-map behavior. href kept pointing at the real endpoint (so
// hover/right-click/middle-click still do something sensible) but the
// actual click goes through _download, which redirects to /login on a
// 401 rather than navigating to a raw JSON error page (this endpoint
// requires login the same as upload/move/delete, see app.rb).
// Logged out: plain text — nothing here would actually work, so it
// isn't shown as if it would (same reasoning as the buttons below).
const name = document.createElement(loggedIn ? "a" : "span");
name.className = "gpx-tree__file-name"; name.className = "gpx-tree__file-name";
name.textContent = displayName; name.textContent = displayName;
if (loggedIn) {
name.href = `${gpxFileUrl(file.path)}?download=geojson`;
name.title = `Download ${displayName}`;
name.addEventListener("click", (event) => {
event.preventDefault();
this._download(name.href, displayName);
});
}
label.appendChild(checkbox); label.appendChild(checkbox);
(file.kinds || []).forEach((kind) => label.appendChild(this._kindIcon(kind))); (file.kinds || []).forEach((kind) => label.appendChild(this._kindIcon(kind)));
if (!file.cached) label.appendChild(this._pendingIcon()); if (!file.cached) label.appendChild(this._pendingIcon());
@@ -357,24 +349,75 @@ export class GpxPanelControl {
return icon; return icon;
} }
// The file's name already downloads its GeoJSON (?download=geojson, // One button, one obvious place for both download kinds — clicking it
// matching what's displayed) — this covers the other half: the // opens a small menu (Original / GeoJSON) instead of splitting them
// untouched, as-uploaded file, which otherwise has no way out of the // across two differently-shaped click targets (the file name used to be
// app once it stops being shown as itself. For a native .geojson // a GeoJSON-only download link; easy to miss it was a link at all, and
// upload the two downloads are identical (nothing to convert), so this // it only ever covered half the story). Costs a click for the common
// isn't hidden for those — just redundant, not wrong. // case of downloading several files in a row, in exchange for both
// formats actually being reachable from one, discoverable place.
_downloadButton(file) { _downloadButton(file) {
const link = document.createElement("a"); const wrapper = document.createElement("span");
link.className = "gpx-tree__download"; wrapper.className = "gpx-tree__download-wrapper";
link.href = `${gpxFileUrl(file.path)}?download=original`;
link.title = "Download original file"; const button = document.createElement("button");
link.setAttribute("aria-label", "Download original file"); button.type = "button";
link.innerHTML = DOWNLOAD_ICON; button.className = "gpx-tree__download";
link.addEventListener("click", (event) => { button.title = "Download";
event.preventDefault(); button.setAttribute("aria-label", "Download");
this._download(link.href, file.name); button.innerHTML = DOWNLOAD_ICON;
const menu = document.createElement("div");
menu.className = "gpx-tree__download-menu";
menu.hidden = true;
const geojsonName = file.cached && !file.name.toLowerCase().endsWith(".geojson") ? `${file.name}.geojson` : file.name;
menu.appendChild(this._downloadMenuItem("Original file", `${gpxFileUrl(file.path)}?download=original`, file.name));
menu.appendChild(this._downloadMenuItem("GeoJSON", `${gpxFileUrl(file.path)}?download=geojson`, geojsonName));
// stopPropagation so this click doesn't immediately reach the
// document-level listener (mount()) that closes whichever menu is
// open on an outside click — otherwise it would close right back up
// the instant it opened.
button.addEventListener("click", (event) => {
event.stopPropagation();
this._toggleDownloadMenu(menu);
}); });
return link;
wrapper.appendChild(button);
wrapper.appendChild(menu);
return wrapper;
}
_downloadMenuItem(label, url, filename) {
const item = document.createElement("a");
item.className = "gpx-tree__download-menu-item";
item.href = url;
item.textContent = label;
item.addEventListener("click", (event) => {
event.preventDefault();
this._closeDownloadMenu();
this._download(url, filename);
});
return item;
}
// Only one menu open at a time — opening a second one closes whichever
// was already open, same as onlyOneSidePanelOpen does for the sidebar's
// own panels (panels.js), just scoped to this one control instead of
// shared across the page.
_toggleDownloadMenu(menu) {
if (this._openDownloadMenu === menu) {
this._closeDownloadMenu();
return;
}
this._closeDownloadMenu();
menu.hidden = false;
this._openDownloadMenu = menu;
}
_closeDownloadMenu() {
if (this._openDownloadMenu) this._openDownloadMenu.hidden = true;
this._openDownloadMenu = null;
} }
// fetch-based rather than a plain navigation so a 401 (not logged in — // fetch-based rather than a plain navigation so a 401 (not logged in —