From aafb4cb4a0ee46e1b6e76a0890818299f8284418 Mon Sep 17 00:00:00 2001 From: Roman Priesol Date: Wed, 9 Sep 2026 11:39:36 +0200 Subject: [PATCH] Consolidate note-drawing into Measure; add edit/delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measure already draws distance lines and area polygons by hand — a second click-to-draw UI in the notes tool (Point/Area mode, from the previous commit) duplicated that. Replaced it with a "Save as note" button on the Measure panel that saves whatever shape is currently drawn there; NotesControl goes back to being just a one-click point pin. Both share a single NoteSaveForm (notes.js) for the note-text + target-layer-file UI instead of two copies of it. gpx.js: the "lines" source now also renders a note-flagged LineString (dashed, popup-on-click) alongside the Polygon fill/outline from before — the plain track-line layer excludes category:"Note" features so a saved distance note doesn't also get the track hover-summary/ profile-panel treatment. Notes are now individually editable/deletable from their own popup (logged-in only): Edit swaps the popup content for an inline textarea, Delete confirms and removes. Both target the feature by a new client-generated properties.id rather than array position (new GpxStore#update_feature/#delete_feature, PUT/DELETE /api/gpx/notes), since neither array index nor the note's own text (the thing being edited) is a stable identifier. Verified live: Measure's Save button (disabled/enabled state, saves the exact on-screen shape), the popup's Edit/Delete buttons and inline edit form. Full test suite: 115 runs, 0 failures (new coverage for update_feature/delete_feature and the PUT/DELETE routes). --- TODO.adoc | 6 +- app.rb | 39 +++++ lib/maptool/gpx_store.rb | 32 ++++ public/css/style.css | 74 ++++++++- public/js/gpx.js | 126 ++++++++++++++-- public/js/measure.js | 52 +++++-- public/js/notes.js | 316 ++++++++++++++------------------------- test/test_app.rb | 55 ++++++- test/test_gpx_store.rb | 38 ++++- 9 files changed, 499 insertions(+), 239 deletions(-) diff --git a/TODO.adoc b/TODO.adoc index bd4736d..0621844 100644 --- a/TODO.adoc +++ b/TODO.adoc @@ -40,8 +40,10 @@ independence from connectivity. == Custom layers & data * [x] GPX/GeoJSON drag-and-drop import (Wikiloc, Komoot, own GPX tracks — also FIT), auto-styled by category with a generated elevation profile — KML not supported (only GPX/FIT/GeoJSON/.zip) -* [x] User overlay: offline drawing of custom points *and areas* with notes (e.g. "turning space for a long vehicle", "narrow gate", "good swimming spot") — Point/Area mode toggle with a live drawing preview, saved into the same GpxStore the Tracks panel already reads (so visibility/popup come for free); gpx.js's `_addLayers` now renders Polygon features (fill + outline) alongside points/lines, not just points -** [ ] The Unigrid Matrix category filter only re-slices the *points* source (`_applyCategoryFilter`) — toggling the "Note" category off hides point notes but not polygon ones sharing that category +* [x] User overlay: offline drawing of custom points with notes (e.g. "turning space for a long vehicle", "narrow gate", "good swimming spot") — one click on the map, pick or name a `.geojson` layer, saved into the same GpxStore the Tracks panel already reads (so visibility/popup come for free) +** [x] Save a distance/area shape as a note too — Measure already draws these by hand, so a "Save" button there (not a second click-to-draw UI in the notes tool) reuses that instead of duplicating it; both share one `NoteSaveForm` (notes.js). `gpx.js`'s `_addLayers` now renders Polygon and note-flagged LineString features (fill/outline, dashed line), not just points +** [x] Edit/delete individual notes from their popup (logged-in only) — targets a feature by a client-generated `properties.id` (`GpxStore#update_feature`/`#delete_feature`), not array position +** [ ] The Unigrid Matrix category filter only re-slices the *points* source (`_applyCategoryFilter`) — toggling the "Note" category off hides point notes but not polygon/line ones sharing that category * [ ] Offline satellite tile caching (e.g. ESRI) for a selected route area — key for judging surface quality and obstacles when wild camping == Nice to have (later) diff --git a/app.rb b/app.rb index 1f67a05..d81b06a 100644 --- a/app.rb +++ b/app.rb @@ -254,6 +254,45 @@ module Maptool json path: path, tree: GPX_STORE.tree end + # Editing/deleting one note (see the popup's Edit/Delete actions, + # gpx.js) — targets a feature by properties["id"], not array position + # (see GpxStore#update_feature/#delete_feature for why). + put "/api/gpx/notes" do + require_login! + begin + feature = JSON.parse(params[:feature].to_s) + rescue JSON::ParserError + halt 400, json(error: "invalid feature") + end + halt 400, json(error: "invalid feature") unless feature.is_a?(Hash) && feature["geometry"] + halt 400, json(error: "path and id are required") if params[:path].to_s.empty? || params[:id].to_s.empty? + + begin + GPX_STORE.update_feature(path: params[:path], id: params[:id], feature: feature) + rescue GpxStore::InvalidPath + halt 400, json(error: "invalid path") + rescue Errno::ENOENT + halt 404, json(error: "note not found") + end + + json ok: true + end + + delete "/api/gpx/notes" do + require_login! + halt 400, json(error: "path and id are required") if params[:path].to_s.empty? || params[:id].to_s.empty? + + begin + GPX_STORE.delete_feature(path: params[:path], id: params[:id]) + rescue GpxStore::InvalidPath + halt 400, json(error: "invalid path") + rescue Errno::ENOENT + halt 404, json(error: "note not found") + end + + json ok: true + end + # ?download=geojson exports the file's GeoJSON as an attachment instead # of rendering it on the map — the only way to get a .gpx/.fit's # converted form out of the app, since GpxStore#geojson's cache is diff --git a/lib/maptool/gpx_store.rb b/lib/maptool/gpx_store.rb index 914a750..1864337 100644 --- a/lib/maptool/gpx_store.rb +++ b/lib/maptool/gpx_store.rb @@ -154,6 +154,38 @@ module Maptool path end + # Replaces the feature whose properties["id"] matches `id` (see + # append_feature/notes.js — every hand-drawn note gets a client- + # generated id specifically so it can be found again later, since nothing + # else here is a stable identifier: array position shifts as other + # features are added/removed, and a note's own text is exactly the thing + # being edited). Raises Errno::ENOENT for a missing file *or* a missing + # feature — same "nothing to act on" story either way. + def update_feature(path:, id:, feature:) + file = resolve_file(path) + raise Errno::ENOENT, file unless File.file?(file) + + collection = JSON.parse(File.read(file)) + index = (collection["features"] || []).find_index { |f| f.dig("properties", "id") == id } + raise Errno::ENOENT, "#{path}##{id}" unless index + + collection["features"][index] = feature + File.write(file, JSON.generate(collection)) + end + + # Same id-matching story as update_feature above. + def delete_feature(path:, id:) + file = resolve_file(path) + raise Errno::ENOENT, file unless File.file?(file) + + collection = JSON.parse(File.read(file)) + before = (collection["features"] || []).length + collection["features"] = (collection["features"] || []).reject { |f| f.dig("properties", "id") == id } + raise Errno::ENOENT, "#{path}##{id}" if collection["features"].length == before + + File.write(file, JSON.generate(collection)) + end + private def resolve_dir(folder) diff --git a/public/css/style.css b/public/css/style.css index 6f09f2e..3223bcd 100644 --- a/public/css/style.css +++ b/public/css/style.css @@ -673,9 +673,6 @@ html, body { left: 100%; margin-left: 8px; z-index: 20; - display: flex; - align-items: center; - gap: 6px; min-width: max-content; background: #fff; border: 1px solid #ddd; @@ -689,6 +686,41 @@ html, body { display: none; } +.measure-panel__controls { + display: flex; + align-items: center; + gap: 6px; +} + +/* Same "author display wins over the UA [hidden] rule" gotcha as + .gpx-panel__action[hidden]/.notes-panel__form[hidden] elsewhere in this + file — this sets its own "display" too. */ +.measure-panel__controls[hidden] { + display: none; +} + +.measure-panel .measure-panel__save { + width: auto; + height: auto; + display: inline-block; + border: none; + border-radius: 4px; + background: none; + font-size: 14px; + line-height: 1; + cursor: pointer; + padding: 2px 4px; +} + +.measure-panel__save:disabled { + opacity: 0.3; + cursor: default; +} + +.measure-panel .notes-panel__form { + width: 200px; +} + /* ".measure-panel " prefix (not just ".measure-panel__mode") to outrank ".maplibregl-ctrl-group button" (higher specificity, since these buttons sit inside a maplibregl-ctrl-group container for the ruler icon's own @@ -990,6 +1022,42 @@ html, body { font-size: 12px; } +.gpx-popup__note-actions, +.gpx-popup__edit-actions { + display: flex; + gap: 6px; + margin-top: 8px; + padding-top: 6px; + border-top: 1px solid #eee; +} + +.gpx-popup__note-actions button, +.gpx-popup__edit-actions button { + border: none; + border-radius: 4px; + background: #f0f0f0; + padding: 3px 8px; + font: inherit; + font-size: 12px; + cursor: pointer; +} + +.gpx-popup__edit-actions .gpx-popup__edit-save { + background: #1a5fb4; + color: #fff; +} + +.gpx-popup__edit-text { + width: 100%; + box-sizing: border-box; + font: inherit; + font-size: 13px; + padding: 4px 6px; + border: 1px solid #ddd; + border-radius: 4px; + resize: vertical; +} + .gnss-control__status { color: #888; white-space: nowrap; diff --git a/public/js/gpx.js b/public/js/gpx.js index 473483a..766c66a 100644 --- a/public/js/gpx.js +++ b/public/js/gpx.js @@ -12,7 +12,7 @@ import { POI_ICONS, PARK4NIGHT_TAGS, } from "/js/icons.js"; -import { notifySidePanelOpen, onlyOneSidePanelOpen, onGpxStoreChanged } from "/js/panels.js"; +import { notifySidePanelOpen, onlyOneSidePanelOpen, onGpxStoreChanged, notifyGpxStoreChanged } from "/js/panels.js"; import { formatActivityType } from "/js/track_profile.js"; import { isLoggedIn } from "/js/auth.js"; @@ -531,22 +531,27 @@ export class GpxPanelControl { } this._map.addSource(linesId, { type: "geojson", data: lines }); + // Excludes category:"Note" — a saved Measure-tool distance line gets + // its own layer below (popup-on-click, not this hover-summary/profile- + // panel track behavior), not this one. this._map.addLayer({ id: `${linesId}-line`, type: "line", source: linesId, + filter: ["any", ["!has", "category"], ["!=", ["get", "category"], "Note"]], layout: { "line-cap": "round", "line-join": "round" }, paint: { "line-color": TRACK_COLOR, "line-width": 3 }, }); this._registerLineInteractivity(`${linesId}-line`); - // Polygon notes (see notes.js) — the same "lines" source already only - // renders LineString geometry through the line layer above (MapLibre's - // own "line" layer type silently ignores Polygon features), so this is - // just two more layers reading the same source, not a new bucket. - // Popup-on-click like a waypoint (_registerPointInteractivity), not the - // track line's hover-summary/profile-panel behavior above — a note - // polygon has no elevation profile to open. + // Polygon/line notes (see notes.js and measure.js's "Save as note") — + // the same "lines" source already only renders LineString geometry + // through the line layer above, and Polygon geometry isn't drawn by a + // "line"-type layer at all (MapLibre only draws LineString for that + // type), so these are just more layers reading the same source, not a + // new bucket. Popup-on-click like a waypoint (_registerPointInteractivity), + // not the track line's hover-summary/profile-panel behavior — a note + // has no elevation profile to open. this._map.addLayer({ id: `${linesId}-polygon-fill`, type: "fill", @@ -561,7 +566,17 @@ export class GpxPanelControl { filter: ["==", ["geometry-type"], "Polygon"], paint: { "line-color": TRACK_COLOR, "line-width": 2 }, }); - this._registerPointInteractivity(`${linesId}-polygon-fill`); + this._registerPointInteractivity(`${linesId}-polygon-fill`, path); + + this._map.addLayer({ + id: `${linesId}-note-line`, + type: "line", + source: linesId, + filter: ["all", ["==", ["geometry-type"], "LineString"], ["==", ["get", "category"], "Note"]], + layout: { "line-cap": "round", "line-join": "round" }, + paint: { "line-color": TRACK_COLOR, "line-width": 3, "line-dasharray": [2, 1] }, + }); + this._registerPointInteractivity(`${linesId}-note-line`, path); // clusterRadius/clusterMaxZoom are harmless when there's nothing dense // enough to cluster (a handful of waypoints just render individually). @@ -570,7 +585,7 @@ export class GpxPanelControl { await Promise.all([this._iconsReady, this._registerBadgeColors(badgeColors)]); if (!this._map.getSource(pointsId)) return true; // a style.load (basemap switch) tore it down while icons were loading - this._addClusterLayers(pointsId); + this._addClusterLayers(pointsId, path); if (points.features.length > 0) this._pointsByPath.set(path, points); this._updateLegend(); @@ -584,7 +599,7 @@ export class GpxPanelControl { // each waypoint's own category color (see Maptool::Gpx#waypoint_color), // falling back to the plain track color for waypoints with no category // (e.g. a hand-placed GNSS pin, not from a POI export). - _addClusterLayers(pointsId) { + _addClusterLayers(pointsId, path) { this._map.addLayer({ id: `${pointsId}-clusters`, type: "circle", @@ -643,7 +658,7 @@ export class GpxPanelControl { }); this._registerClusterInteractivity(pointsId); - this._registerPointInteractivity(`${pointsId}-unclustered`); + this._registerPointInteractivity(`${pointsId}-unclustered`, path); } // Clicking a cluster zooms in just far enough to break it apart, the @@ -671,17 +686,22 @@ export class GpxPanelControl { // delegated listener that lives on the map itself and keeps matching // that layer id after setStyle() removes and (via _addLayers) re-adds // it, so re-registering on every re-add would stack duplicate popups. - _registerPointInteractivity(layerId) { + // `path` (which file this layer's features came from) is only used for + // a "Note" feature's Edit/Delete actions — plain GPX/FIT/park4night + // waypoints don't get those, so it's otherwise unused. + _registerPointInteractivity(layerId, path) { if (this._interactiveLayers.has(layerId)) return; this._interactiveLayers.add(layerId); this._map.on("click", layerId, (event) => { const feature = event.features[0]; if (!feature) return; - new Popup({ closeButton: true, maxWidth: "260px" }) + const editable = feature.properties.category === "Note" && isLoggedIn(); + const popup = new Popup({ closeButton: true, maxWidth: "260px" }) .setLngLat(event.lngLat) - .setHTML(popupHtml(feature.properties)) + .setHTML(popupHtml(feature.properties, { editable })) .addTo(this._map); + if (editable) this._wireNoteActions(popup, path, feature); }); this._map.on("mouseenter", layerId, () => { this._map.getCanvas().style.cursor = "pointer"; @@ -691,6 +711,70 @@ export class GpxPanelControl { }); } + // Edit swaps the popup's content for a plain textarea + Save/Cancel (the + // note's location doesn't change, just its text, so there's no need for + // NoteSaveForm's fuller target-file picker here). Delete just confirms + // and removes. Both end by reloading the file's layers from the server — + // simplest way to keep the map, the category legend, and this popup's + // own feature data all in sync with what the edit/delete actually wrote, + // rather than hand-patching three different client-side copies of it. + _wireNoteActions(popup, path, feature) { + const element = popup.getElement(); + const id = feature.properties.id; + + element.querySelector(".gpx-popup__delete")?.addEventListener("click", async () => { + if (!window.confirm("Delete this note?")) return; + try { + const response = await fetch(`/api/gpx/notes?${new URLSearchParams({ path, id })}`, { method: "DELETE" }); + if (!response.ok) throw new Error(`delete failed: ${response.status}`); + popup.remove(); + notifyGpxStoreChanged(); + await this._reloadFile(path); + } catch (err) { + window.alert("Could not delete the note — check your connection and try again."); + } + }); + + element.querySelector(".gpx-popup__edit")?.addEventListener("click", () => { + const content = popup.getElement().querySelector(".maplibregl-popup-content"); + content.innerHTML = ` +
+ +
+ + +
+
+ `; + const textarea = content.querySelector(".gpx-popup__edit-text"); + textarea.focus(); + content.querySelector(".gpx-popup__edit-cancel").addEventListener("click", () => { + content.innerHTML = popupHtml(feature.properties, { editable: true }); + this._wireNoteActions(popup, path, feature); + }); + content.querySelector(".gpx-popup__edit-save").addEventListener("click", async () => { + const text = textarea.value.trim(); + if (!text) return; + const updated = { ...feature, properties: { ...feature.properties, name: text } }; + try { + const body = new URLSearchParams({ path, id, feature: JSON.stringify(updated) }); + const response = await fetch("/api/gpx/notes", { method: "PUT", body }); + if (!response.ok) throw new Error(`save failed: ${response.status}`); + popup.remove(); + notifyGpxStoreChanged(); + await this._reloadFile(path); + } catch (err) { + window.alert("Could not save the note — check your connection and try again."); + } + }); + }); + } + + async _reloadFile(path) { + this._removeLayers(path); + await this._addLayers(path); + } + // Hover shows a quick summary (unlike a waypoint popup, which is // click-triggered — a track's summary describes the whole line, not // whatever point along it happens to get hovered, so there's no single @@ -729,6 +813,7 @@ export class GpxPanelControl { `${linesId}-line`, `${linesId}-polygon-fill`, `${linesId}-polygon-outline`, + `${linesId}-note-line`, `${pointsId}-clusters`, `${pointsId}-cluster-count`, `${pointsId}-unclustered`, @@ -985,7 +1070,11 @@ function gpxFileUrl(path) { // properties come straight from an uploaded GPX file — untrusted input — // so every value is escaped before landing in the popup's HTML. -function popupHtml({ name, description, time, category, photos, icon }) { +// editable (only ever true for a "Note" feature the viewer is logged in +// for — see _registerPointInteractivity) adds Edit/Delete buttons; the +// click handling for them lives in _wireNoteActions, not here, since this +// only builds markup, it has no map/store access of its own. +function popupHtml({ name, description, time, category, photos, icon }, { editable = false } = {}) { const parts = []; if (name) { const glyph = icon && POI_ICONS[icon] ? `${POI_ICONS[icon]}` : ""; @@ -1000,6 +1089,11 @@ function popupHtml({ name, description, time, category, photos, icon }) { parts.push(`

${escapeHtml(parsed.toLocaleString())}

`); } } + if (editable) { + parts.push( + `
`, + ); + } return parts.length > 0 ? parts.join("") : "No details"; } diff --git a/public/js/measure.js b/public/js/measure.js index 40086b8..3c47add 100644 --- a/public/js/measure.js +++ b/public/js/measure.js @@ -1,4 +1,5 @@ import { RULER_ICON } from "/js/icons.js"; +import { NoteSaveForm } from "/js/notes.js"; const SOURCE_ID = "measure"; const LINE_LAYER_ID = "measure-line"; @@ -29,24 +30,38 @@ export class MeasureControl { this._container.innerHTML = ` `; this._button = this._container.querySelector(".measure-ctrl__button"); this._panel = this._container.querySelector(".measure-panel"); + this._controls = this._container.querySelector(".measure-panel__controls"); this._result = this._container.querySelector(".measure-panel__result"); + this._saveButton = this._container.querySelector(".measure-panel__save"); + + this._noteForm = new NoteSaveForm(this._panel, { + onSaved: () => { + this._controls.hidden = false; + this._clear(); + }, + onCancel: () => (this._controls.hidden = false), + }); this._button.addEventListener("click", () => this._setActive(!this._active)); this._panel.querySelectorAll(".measure-panel__mode").forEach((button) => { button.addEventListener("click", () => this._setMode(button.dataset.mode)); }); + this._saveButton.addEventListener("click", () => this._saveAsNote()); this._panel.querySelector(".measure-panel__clear").addEventListener("click", () => this._clear()); this._onClick = (event) => { - if (!this._active) return; + if (!this._active || this._noteForm.isOpen) return; this._points.push([event.lngLat.lng, event.lngLat.lat]); this._render(); }; @@ -79,10 +94,30 @@ export class MeasureControl { if (active) { this._addLayers(); } else { + this._noteForm.close(); + this._controls.hidden = false; this._clear(); } } + // The same shape _render() draws — a distance line (2+ points) or a + // closed area polygon (3+ points), or null while there isn't yet enough + // to measure — extracted so "Save as note" saves exactly what's on + // screen rather than recomputing it slightly differently. + _currentShapeGeometry() { + if (this._mode === "area") { + return this._points.length >= 3 ? { type: "Polygon", coordinates: [[...this._points, this._points[0]]] } : null; + } + return this._points.length >= 2 ? { type: "LineString", coordinates: this._points } : null; + } + + _saveAsNote() { + const geometry = this._currentShapeGeometry(); + if (!geometry) return; + this._controls.hidden = true; + this._noteForm.open(geometry); + } + _setMode(mode) { this._mode = mode; this._panel.querySelectorAll(".measure-panel__mode").forEach((button) => { @@ -165,10 +200,9 @@ export class MeasureControl { properties: { role: "vertex" }, geometry: { type: "Point", coordinates: coord }, })); - const shapeFeature = - this._mode === "area" - ? this._points.length >= 3 && { type: "Feature", geometry: { type: "Polygon", coordinates: [[...this._points, this._points[0]]] } } - : this._points.length >= 2 && { type: "Feature", geometry: { type: "LineString", coordinates: this._points } }; + const geometry = this._currentShapeGeometry(); + const shapeFeature = geometry && { type: "Feature", geometry }; + this._saveButton.disabled = !geometry; // Area mode has no equivalent yet — only asked for distance's own // per-leg labels, and a polygon's edges read differently (perimeter // segments, not a path) if this ever extends there. diff --git a/public/js/notes.js b/public/js/notes.js index c440a2f..93eb141 100644 --- a/public/js/notes.js +++ b/public/js/notes.js @@ -2,222 +2,73 @@ import { NOTE_PIN_ICON } from "/js/icons.js"; import { notifyGpxStoreChanged } from "/js/panels.js"; const NEW_LAYER_VALUE = "__new__"; -const PREVIEW_SOURCE_ID = "notes-preview"; -const PREVIEW_FILL_LAYER_ID = "notes-preview-fill"; -const PREVIEW_LINE_LAYER_ID = "notes-preview-line"; -const PREVIEW_POINT_LAYER_ID = "notes-preview-points"; -const PREVIEW_COLOR = "#c0392b"; -// A quick "pin it with a note" tool — click the map (Point mode) or click -// out a few corners and Finish (Area mode), write a short note, pick which -// .geojson "layer" file it joins (or start a new one). Saved server-side -// (POST /api/gpx/notes) into the same GpxStore everything else in the -// Tracks panel already reads from, so a note shows up there with no -// separate rendering path of its own: same tree, same checkbox visibility, -// same popup — public/js/gpx.js's _addLayers renders a Polygon feature the -// same way it already renders points/lines, no separate code path here. -export class NotesControl { - constructor() { - this._active = false; - this._mode = "point"; // "point" | "area" - this._areaPoints = []; // vertices while drawing an area, before Finish - this._pendingGeometry = null; // the finished Point/Polygon geometry, once placed - } +// Every hand-drawn note (from here or measure.js's "Save as note") gets one +// of these — the only stable way to find a specific feature again for +// editing/deleting later (see GpxStore#update_feature/#delete_feature): +// array position shifts as other notes are added/removed, and the note's +// own text is exactly the thing an edit changes. +export function generateNoteId() { + return typeof crypto !== "undefined" && crypto.randomUUID ? crypto.randomUUID() : `${Date.now()}-${Math.random().toString(36).slice(2)}`; +} - onAdd(map) { - this._map = map; +// The "note text + which .geojson layer" form, shared by NotesControl (a +// single click) and measure.js's "Save as note" button (its current +// distance/area shape) — extracted here rather than duplicated in both, +// since the two differ only in *what* the note attaches a location to, +// not in how it's captured or where it's saved. Renders into whatever +// container it's given (each caller owns where that sits in its own panel); +// hidden until open() is called. +export class NoteSaveForm { + constructor(container, { onSaved, onCancel } = {}) { + this._onSaved = onSaved; + this._onCancel = onCancel; - this._container = document.createElement("div"); - this._container.className = "maplibregl-ctrl maplibregl-ctrl-group notes-ctrl"; - this._container.innerHTML = ` - -