From 9997a8a8c06280266003a8ed51a4f081f1da11cb Mon Sep 17 00:00:00 2001 From: Roman Priesol Date: Wed, 9 Sep 2026 15:11:50 +0200 Subject: [PATCH] Fix note UX: unreachable wiki link, white Save hover, stale icon preview, invisible new points MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - "Nearby on Wikipedia" linked to KIWIX_URL, a Docker-internal hostname (http://kiwix:8080) no real browser can resolve. Split it: KIWIX_URL stays server-side-only for the reachability check, and the link itself is built from KIWIX_PUBLIC_PORT (kiwix-serve's newly published host port) plus whatever host the visitor already used to reach maptool — works regardless of which hostname/IP that is. - The Save button went white on hover in GTK-themed Chrome: the browser's native button skin was painting over the custom background because appearance wasn't reset. - The map preview marker for a pending note always showed a plain red pin no matter which icon was picked in the form. NoteSaveForm now reports icon changes via onIconChange so NotesControl can update the marker's glyph live. - Saving a note (or a Measure "Save as note") left the new/grown .geojson file unchecked and invisible until manually toggled on. notifyGpxStoreChanged now carries the affected path so GpxPanelControl auto-shows it. --- app.rb | 23 ++++++++++++++++++++--- docker-compose.yaml | 12 ++++++++++++ docs/development.adoc | 15 ++++++++++----- public/css/style.css | 28 ++++++++++++++++++++++++++++ public/js/gpx.js | 13 +++++++++++-- public/js/notes.js | 18 +++++++++++++++--- public/js/panels.js | 11 +++++++---- 7 files changed, 103 insertions(+), 17 deletions(-) diff --git a/app.rb b/app.rb index e6f6f51..7e91819 100644 --- a/app.rb +++ b/app.rb @@ -73,9 +73,21 @@ module Maptool # An external kiwix-serve instance (see docs/development.adoc), not # something this app runs itself — blank disables the "Nearby on # Wikipedia" popup link the same way the other optional-integration - # blanks above do. + # blanks above do. This is reached server-side only (the reachability + # check below, same-Docker-network service name in production, e.g. + # "http://kiwix:8080") — it is NOT what gets handed to the browser, see + # KIWIX_PUBLIC_PORT. KIWIX_URL = ENV.fetch("KIWIX_URL", "") + # The port kiwix-serve is published on for browsers to reach directly + # (docker-compose.yaml's kiwix service ports: mapping) — KIWIX_URL above + # is a Docker-network-internal hostname a real browser can't resolve at + # all, so the "Nearby on Wikipedia" link is instead built from whatever + # host the visitor already used to reach maptool itself + # (request.host) plus this port. Blank disables the link, same as a + # blank KIWIX_URL. + KIWIX_PUBLIC_PORT = ENV.fetch("KIWIX_PUBLIC_PORT", "") + # Login delegates to the Gitea instance the user already has an account # on, rather than maintaining a local username/password — see # docs/architecture.adoc. Only users listed in ALLOWED_GITEA_USERS may @@ -208,9 +220,14 @@ module Maptool # Pinged once per page load (public/js/map.js) — url is handed back so # the frontend can build the geo-search link itself; this server never - # proxies to kiwix-serve, just says whether it's there. + # proxies to kiwix-serve, just says whether it's there. url is built + # from request.host (whatever hostname/IP the visitor already used to + # reach maptool itself) + KIWIX_PUBLIC_PORT, not KIWIX_URL — that's a + # Docker-network-internal address only this server can resolve. get "/api/kiwix" do - json available: Kiwix.reachable?(KIWIX_URL), url: KIWIX_URL + reachable = Kiwix.reachable?(KIWIX_URL) + url = KIWIX_PUBLIC_PORT.empty? ? "" : "http://#{request.host}:#{KIWIX_PUBLIC_PORT}" + json available: reachable && !url.empty?, url: url end get "/api/gpx/tree" do diff --git a/docker-compose.yaml b/docker-compose.yaml index 0439b36..0718206 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -37,7 +37,14 @@ services: # actually being present: GET /api/kiwix pings it and just reports # unavailable if the container isn't up/reachable, same story as # every other optional-local-file feature (offline.pmtiles, etc). + # Server-side reachability check only — a browser on the LAN can't + # resolve this hostname, see KIWIX_PUBLIC_PORT below. KIWIX_URL: "http://kiwix:8080" + # The host port kiwix's own ports: mapping below publishes it on — + # app.rb builds the browser-facing "Nearby on Wikipedia" link from + # whatever host the visitor used to reach maptool itself plus this + # port, since KIWIX_URL above is Docker-internal only. + KIWIX_PUBLIC_PORT: "8090" volumes: # Uploaded GPX files and the offline PMTiles extract — kept outside # the image so a rebuild/redeploy doesn't wipe them (see @@ -87,5 +94,10 @@ services: # passing it again produced "Unexpected argument: --port=8080, # --port=8080, /data/wikipedia.zim" and kiwix-serve refused to start. command: ["/data/wikipedia.zim"] + # Published so browsers on the LAN can reach kiwix-serve's own search + # UI directly (see maptool's KIWIX_PUBLIC_PORT above) — 8090, not 8080, + # since 8080 is already taken by another container on this host. + ports: + - "8090:8080" volumes: - ./data/kiwix/wikipedia.zim:/data/wikipedia.zim diff --git a/docs/development.adoc b/docs/development.adoc index 72337f2..d1d6375 100644 --- a/docs/development.adoc +++ b/docs/development.adoc @@ -77,11 +77,16 @@ outline rather than a single point). A point popup's "Nearby on Wikipedia" link needs https://github.com/kiwix/kiwix-tools[kiwix-serve] running with a Wikipedia https://library.kiwix.org[ZIM] loaded — set `KIWIX_URL` to -wherever it's reachable (`docker-compose.yaml` already runs a `kiwix` -service pointed at `data/kiwix/wikipedia.zim` and wires `KIWIX_URL` to -it; blank/unset disables the link, same as every other optional -integration here). Not part of the repo — a ZIM is a large, -self-contained archive, obtain one locally: +wherever *the server* can reach it (`docker-compose.yaml` already runs +a `kiwix` service pointed at `data/kiwix/wikipedia.zim` and wires +`KIWIX_URL` to it via its compose service name; blank/unset disables +the link, same as every other optional integration here). This is +used only for the server-side reachability check — a browser can't +resolve a Docker-internal hostname like `kiwix`, so the link itself is +built from `KIWIX_PUBLIC_PORT` (the host port kiwix-serve is +`ports:`-published on, `8090` in `docker-compose.yaml`) plus whatever +host the visitor already used to reach maptool itself. Not part of the +repo — a ZIM is a large, self-contained archive, obtain one locally: * https://library.kiwix.org/#lang=eng&category=wikipedia[Kiwix's own library] has pre-built Wikipedia ZIMs — a `_maxi` variant per diff --git a/public/css/style.css b/public/css/style.css index 55a945a..4422f68 100644 --- a/public/css/style.css +++ b/public/css/style.css @@ -930,6 +930,29 @@ html, body { background: #dbe9ff; } +/* The pending-note marker shown on the map while the save form is open — + updates live to whatever icon is picked (see notes.js's onIconChange). A + plain round badge (not a drop-pin shape) since MapLibre anchors a custom + marker element at its center, and a pin's usual tip-points-at-the-spot + shape needs a bottom anchor to line up correctly. */ +.notes-panel__preview-marker { + width: 26px; + height: 26px; + border-radius: 50%; + background: #c0392b; + border: 2px solid #fff; + box-shadow: 0 1px 4px rgba(0, 0, 0, 0.4); + display: flex; + align-items: center; + justify-content: center; + color: #fff; +} + +.notes-panel__preview-marker svg { + width: 15px; + height: 15px; +} + .notes-panel__target, .notes-panel__new-name { width: 100%; @@ -948,6 +971,11 @@ html, body { } .notes-panel__actions button { + /* Without this, some browsers (GTK-themed Chrome on Linux in + particular) paint their own native hover/active highlight over a + button's background instead of leaving a custom one alone — the + Save button visibly went white on hover because of exactly this. */ + appearance: none; width: auto; height: auto; padding: 4px 10px; diff --git a/public/js/gpx.js b/public/js/gpx.js index 8560458..8b8d152 100644 --- a/public/js/gpx.js +++ b/public/js/gpx.js @@ -60,8 +60,17 @@ export class GpxPanelControl { // notes.js writes directly into the same GPX store (a hand-drawn note // saved into a .geojson layer) — refresh so a newly created/grown file - // shows up in the tree without a manual reopen. - onGpxStoreChanged(() => this._loadTree()); + // shows up in the tree without a manual reopen, and auto-show it (check + // + render) since a note the user just placed being invisible until + // they find and check its layer by hand defeats the point of dropping + // a pin right there on the map. + onGpxStoreChanged((path) => { + if (path) { + this._visible.add(path); + this._reloadFile(path); + } + this._loadTree(); + }); } // Registered once, always white, as plain (non-SDF) images: these used to diff --git a/public/js/notes.js b/public/js/notes.js index c54dc86..42bd372 100644 --- a/public/js/notes.js +++ b/public/js/notes.js @@ -45,9 +45,10 @@ export function generateNoteId() { // 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 } = {}) { + constructor(container, { onSaved, onCancel, onIconChange } = {}) { this._onSaved = onSaved; this._onCancel = onCancel; + this._onIconChange = onIconChange; this._el = document.createElement("div"); this._el.className = "notes-panel__form"; @@ -85,6 +86,7 @@ export class NoteSaveForm { button.addEventListener("click", () => { this._selectedIcon = button.dataset.icon; this._iconPicker.querySelectorAll(".notes-panel__icon-option").forEach((b) => b.classList.toggle("active", b === button)); + this._onIconChange?.(this._selectedIcon); }); }); this._target.addEventListener("change", () => { @@ -106,6 +108,7 @@ export class NoteSaveForm { this._body.value = ""; this._selectedIcon = ""; this._iconPicker.querySelectorAll(".notes-panel__icon-option").forEach((b, i) => b.classList.toggle("active", i === 0)); + this._onIconChange?.(this._selectedIcon); // An icon only makes sense on a single point — Measure's "Save as // note" can hand this a LineString/Polygon, which has no one spot for // a badge glyph to sit at. @@ -173,7 +176,8 @@ export class NoteSaveForm { try { const response = await fetch("/api/gpx/notes", { method: "POST", body: requestBody }); if (!response.ok) throw new Error(`save failed: ${response.status}`); - notifyGpxStoreChanged(); + const { path } = await response.json(); + notifyGpxStoreChanged(path); this.close(); this._onSaved?.(); } catch (err) { @@ -219,6 +223,11 @@ export class NotesControl { this._hint.hidden = false; this._removeMarker(); }, + // Keeps the map preview in sync with the icon picker — it used to + // always show a plain red pin no matter which icon got picked. + onIconChange: (icon) => { + if (this._marker) this._marker.getElement().innerHTML = icon && POI_ICONS[icon] ? POI_ICONS[icon] : NOTE_PIN_ICON; + }, }); this._button.addEventListener("click", () => this._setActive(!this._active)); @@ -226,7 +235,10 @@ export class NotesControl { this._onClick = (event) => { if (!this._active || this._form.isOpen) return; this._hint.hidden = true; - this._marker = new Marker({ color: "#c0392b" }).setLngLat(event.lngLat).addTo(this._map); + const el = document.createElement("div"); + el.className = "notes-panel__preview-marker"; + el.innerHTML = NOTE_PIN_ICON; + this._marker = new Marker({ element: el }).setLngLat(event.lngLat).addTo(this._map); this._form.open({ type: "Point", coordinates: [event.lngLat.lng, event.lngLat.lat] }); }; map.on("click", this._onClick); diff --git a/public/js/panels.js b/public/js/panels.js index 6e4552c..d812173 100644 --- a/public/js/panels.js +++ b/public/js/panels.js @@ -18,13 +18,16 @@ export function onlyOneSidePanelOpen(panel, onClosedByOther) { // Lets a control that writes into the GPX store (e.g. notes.js saving a // drawn point) tell GpxPanelControl its tree is stale, without either one -// holding a reference to the other. +// holding a reference to the other. path (the .geojson file just written +// to) lets the listener also auto-show that file instead of just refreshing +// the tree — the point/shape just saved is otherwise invisible until the +// user finds and checks it by hand. const STORE_CHANGED_EVENT = "gpx-store-changed"; -export function notifyGpxStoreChanged() { - document.dispatchEvent(new CustomEvent(STORE_CHANGED_EVENT)); +export function notifyGpxStoreChanged(path) { + document.dispatchEvent(new CustomEvent(STORE_CHANGED_EVENT, { detail: path })); } export function onGpxStoreChanged(callback) { - document.addEventListener(STORE_CHANGED_EVENT, callback); + document.addEventListener(STORE_CHANGED_EVENT, (event) => callback(event.detail)); }