Changelog: 1598 fragment

changelog.d/1598-shortcuts-modifier-keys.md

@@ -0,0 +1,5 @@
+- Keep single-key app shortcuts off Cmd, Ctrl and Alt combinations, so Cmd+F,
+  Cmd+C and Cmd+R reach the browser instead of hiding the layer panel or
+  switching layers on. `F` now collapses the layer panel the way its button
+  does, instead of hiding the panel and the button that brings it back
+  (asturksever, #1598).

changelog.d/1603-mcp-keyless-geocode.md

@@ -1,3 +0,0 @@
-- MCP tools resolve place names through the keyless Photon geocoder when the
-  server has no geocoder configured, instead of answering "No place matched"
-  for every name (asturksever, #1603).

scripts/package-boundaries.json

@@ -5081,8 +5081,7 @@
       "src/layers/alpr/records.js",
       "src/layers/alpr/tileRecords.js",
       "src/sources/infrastructureData.js",
-      "src/sources/eventPacks.js",
+      "src/sources/eventPacks.js"
-      "src/keylessGeocoder.js"
     ],
     "external": ["@mapbox/vector-tile", "pbf"]
   },

src/tools/places.js

@@ -24,7 +24,6 @@ export function createGeocodePlaceService({
   cacheMs = 10 * 60_000,
   timeoutMs = 15_000,
   now = () => Date.now(),
-  keylessGeocoder = null,
 } = {}) {
   // Recent answers by name, so tools that combine others resolve a place to
   // the same point once instead of once per section. A pending lookup is
@@ -34,17 +33,8 @@ export function createGeocodePlaceService({
   async function lookup(name, signal) {
     const query = new URLSearchParams({ q: name });
     const response = await fetchImpl(`/api/geocode?${query}`, { signal });
-    const payload = await response.json().catch(() => null);
-    // No server geocoder (no Google key, no Nominatim): ask the keyless one
-    // the search box uses, rather than reporting every name as unmatched.
-    if (
-      keylessGeocoder &&
-      (payload?.configured === false || payload?.code === 'NOMINATIM_DISABLED')
-    ) {
-      const { place } = await keylessGeocoder.geocode(name, { signal });
-      return place ? placeFromKeylessResult(place) : null;
-    }
     if (!response.ok) throw new Error(`Geocode HTTP ${response.status}`);
+    const payload = await response.json();
     return placeFromGeocodeResult(payload?.results?.[0]);
   }
   function start(key, name) {
@@ -110,14 +100,6 @@ export function createGeocodePlaceService({
 }
 
 /** Convert one geocode result to a place with bounds, or null. */
-/** A keyless (Photon) answer in the same shape as a geocode result's. */
-function placeFromKeylessResult({ lat, lng, label, name, viewport }) {
-  return placeFromGeocodeResult({
-    formatted_address: label || name || null,
-    geometry: { location: { lat, lng }, ...(viewport ? { viewport } : {}) },
-  });
-}
-
 export function placeFromGeocodeResult(result) {
   const location = result?.geometry?.location;
   if (!Number.isFinite(location?.lat) || !Number.isFinite(location?.lng))

src/tools/places.test.mjs

@@ -93,49 +93,3 @@ test('a shared lookup outlives one cancelled caller, and times out on its own',
   await places.resolve('Slow').catch(() => {});
   assert.equal(signals.length, 3, 'a timed-out lookup is not cached');
 });
-
-test('without a server geocoder, names resolve through the keyless geocoder', async () => {
-  const asked = [];
-  const places = createGeocodePlaceService({
-    fetchImpl: async () =>
-      Response.json(
-        {
-          error: 'Place search is not configured',
-          code: 'NOMINATIM_DISABLED',
-          retryable: false,
-        },
-        { status: 503 },
-      ),
-    keylessGeocoder: {
-      geocode: async (name) => {
-        asked.push(name);
-        return name === 'Paris'
-          ? {
-              place: {
-                lat: 48.8566,
-                lng: 2.3522,
-                name: 'Paris',
-                label: 'Paris, France',
-                viewport: {
-                  southwest: { lat: 48.81, lng: 2.22 },
-                  northeast: { lat: 48.9, lng: 2.47 },
-                },
-              },
-              answered: true,
-            }
-          : { place: null, answered: true };
-      },
-    },
-  });
-  const paris = await places.resolve('Paris');
-  assert.equal(paris.name, 'Paris, France');
-  assert.deepEqual(paris.point, { lat: 48.8566, lon: 2.3522 });
-  assert.deepEqual(paris.bounds, {
-    west: 2.22,
-    south: 48.81,
-    east: 2.47,
-    north: 48.9,
-  });
-  assert.equal(await places.resolve('Nowhere-at-all'), null);
-  assert.deepEqual(asked, ['Paris', 'Nowhere-at-all']);
-});

src/tools/services.js

@@ -35,7 +35,6 @@ import {
   createVesselSource,
   createFlightSource,
 } from '../sources/live/standalone.js';
-import { createPhotonGeocoder } from '../keylessGeocoder.js';
 import {
   createGeocodePlaceService,
   createPlaceSearchService,
@@ -87,10 +86,7 @@ export function createToolServices({ fetchImpl, appUrl, panelKey }) {
       fetchImpl,
       tileFetchImpl: fetchImpl,
     }),
-    places: createGeocodePlaceService({
+    places: createGeocodePlaceService({ fetchImpl }),
-      fetchImpl,
-      keylessGeocoder: createPhotonGeocoder({ fetchImpl }),
-    }),
   };
 }
 

src/ui/applicationShell.js

@@ -542,6 +542,7 @@ export class StyleManager extends ShellFacade {
         _toggleOrbit: (...args) => this._toggleOrbit(...args),
         toggleCleanView: (...args) => this.toggleCleanView(...args),
         _toggleCctvEnabled: (...args) => this._toggleCctvEnabled(...args),
+        _togglePanel: (panelId) => this._panelChrome._togglePanel(panelId),
         _setBloomEnabled: (...args) => this._setBloomEnabled(...args),
         _setBloomIntensity: (...args) => this._setBloomIntensity(...args),
         _setSharpenEnabled: (...args) => this._setSharpenEnabled(...args),

src/ui/applicationShortcuts.js

@@ -27,6 +27,11 @@ export function bindApplicationShortcuts({
       event.target?.matches?.('select, input, textarea') ||
       event.target === searchInput;
     if (isFormControl && event.key !== 'Escape') return;
+    // Browser and OS shortcuts (Cmd+C, Ctrl+F, Alt+1…) are not ours.
+    if (event.metaKey || event.ctrlKey || event.altKey) {
+      if (event.key === 'Escape') actions.dismissSearch();
+      return;
+    }
 
     if (STYLE_KEYS[event.key]) actions.setStyle(STYLE_KEYS[event.key]);
     if (event.key === 'Escape') actions.dismissSearch();

src/ui/displayBindings.js

@@ -64,8 +64,8 @@ export class DisplayBindings {
         },
         toggleOrbit: () => this._toggleOrbit(),
         toggleCleanView: () => this.toggleCleanView(),
-        toggleLayers: () =>
+        // Same collapse as the panel's own button, so its rail entry stays.
-          document.getElementById('data-panel').classList.toggle('active'),
+        toggleLayers: () => this._togglePanel('data-panel'),
         cycleDetection: () => {
           this.shareLinkManager?.claimRestoreLane?.('visual');
           this.claimDetection();

src/ui/visualInput.test.mjs

@@ -117,9 +117,19 @@ test('the supplied search target retains editing even without form markup', () =
   assert.deepEqual(f.calls, [['dismissSearch']]);
 });
 
-test('shortcut extraction does not change repeat or modifier policy', () => {
+test('browser shortcuts with Cmd, Ctrl or Alt never reach app shortcuts', () => {
   const f = shortcuts();
-  f.press('h', new Element(), { repeat: true, ctrlKey: true });
+  for (const modifier of ['metaKey', 'ctrlKey', 'altKey'])
+    for (const key of ['1', 'h', 'o', 'v', 'f', 'd', 'c'])
+      f.press(key, new Element(), { [modifier]: true });
+  assert.deepEqual(f.calls, []);
+  f.press('Escape', new Element(), { metaKey: true });
+  assert.deepEqual(f.calls, [['dismissSearch']]);
+});
+
+test('a held key still repeats its shortcut', () => {
+  const f = shortcuts();
+  f.press('h', new Element(), { repeat: true });
   assert.deepEqual(f.calls, [['toggleHud']]);
 });