RES-1863: draggable map for group location fine-tuning

app/Http/Controllers/API/GroupController.php

@@ -1207,7 +1207,7 @@
                                 ]);
     }
 
     private function validateGroupParams(Request $request, $create, ?Group $existingGroup = null): array {
         // We don't validate max lengths of other strings, to avoid duplicating the length information both here
         // and in the migrations.  If we wanted to do that we should extract the length dynamically from the
         // schema, which is possible but not trivial.
@@ -1218,6 +1218,8 @@
                                    'description' => ['required'],
                                    'website' => ['nullable', 'url', 'max:255'],
                                    'postcode' => ['nullable', 'max:32'],
+                                   'lat' => ['nullable', 'numeric', 'between:-90,90', 'required_with:lng'],
+                                   'lng' => ['nullable', 'numeric', 'between:-180,180', 'required_with:lat'],
                                ]);
         } else {
             $request->validate([
@@ -1226,6 +1228,8 @@
                                    'website' => ['nullable', 'url', 'max:255'],
                                    'archived_at' => ['nullable', 'date'],
                                    'postcode' => ['nullable', 'max:32'],
+                                   'lat' => ['nullable', 'numeric', 'between:-90,90', 'required_with:lng'],
+                                   'lng' => ['nullable', 'numeric', 'between:-180,180', 'required_with:lat'],
                                ]);
         }
 
@@ -1249,6 +1253,12 @@
             throw ValidationException::withMessages(['location ' => __('partials.validate_timezone')]);
         }
 
+        // The add/edit page shows a draggable map, so the client can send explicit
+        // coordinates that take precedence over geocoding the location text.
+        $clientLat = $request->input('lat');
+        $clientLng = $request->input('lng');
+        $hasClientCoords = $clientLat !== null && $clientLng !== null;
+
         if (!empty($location)) {
             // Skip geocoding when the location string hasn't changed — avoids unnecessary API calls
             // and prevents failures when editing other fields (e.g. group icon) in environments
@@ -1264,18 +1274,27 @@
                 $geocoded = $geocoder->geocode($location);
 
                 if (empty($geocoded)) {
-                    throw ValidationException::withMessages(['location ' => __('groups.geocode_failed')]);
-                }
-
-                $latitude = $geocoded['latitude'];
-                $longitude = $geocoded['longitude'];
+                    // With an explicit pin we can still place the group; without
+                    // one, an ungeocodable location is fatal as before.
+                    if (!$hasClientCoords) {
+                        throw ValidationException::withMessages(['location ' => __('groups.geocode_failed')]);
+                    }
+                } else {
+                    $latitude = $geocoded['latitude'];
+                    $longitude = $geocoded['longitude'];
 
-                // Note that the country returned by the geocoder is already in English, which is what we need for the
-                // value in the database.
-                $country_code = $geocoded['country_code'] ?? null;
+                    // Note that the country returned by the geocoder is already in English, which is what we need for the
+                    // value in the database.
+                    $country_code = $geocoded['country_code'] ?? null;
+                }
             }
         }
 
+        if ($hasClientCoords) {
+            $latitude = (float) $clientLat;
+            $longitude = (float) $clientLng;
+        }
+
         return [
             $name,
             $area,

lang/en/partials.php

@@ -84,6 +84,7 @@
   'notification_greeting' => 'Hello!',
   'confirm' => 'Confirm',
   'validate_timezone' => 'Please select a valid timezone.',
+  'dragmap' => 'Pin not quite in the right place?  Drag it to the right spot.',
   'share_this' => 'Share this',
   'download' => 'Download',
   'share_modal_title' => 'Shareable Statistics',

lang/fr-BE/partials.php

@@ -87,6 +87,7 @@
   'notification_greeting' => 'Bonjour !',
   'notification_footer' => 'Si vous souhaitez ne plus recevoir ces courriels, veuillez consulter <a href=":url">vos préférences</a> sur votre compte.',
   'validate_timezone' => 'Veuillez sélectionner un fuseau horaire valide.',
+  'dragmap' => 'Le repère n\'est pas tout à fait au bon endroit ? Faites-le glisser pour le corriger.',
   'share_this' => 'Partager ceci',
   'download' => 'Télécharger',
   'share_modal_title' => 'Statistiques à partager',

lang/fr/partials.php

@@ -87,6 +87,7 @@
   'notification_greeting' => 'Bonjour !',
   'notification_footer' => 'Si vous souhaitez ne plus recevoir ces courriels, veuillez consulter <a href=":url">vos préférences</a> sur votre compte.',
   'validate_timezone' => 'Veuillez sélectionner un fuseau horaire valide.',
+  'dragmap' => 'Le repère n\'est pas tout à fait au bon endroit ? Faites-le glisser pour le corriger.',
   'share_this' => 'Partager ceci',
   'download' => 'Télécharger',
   'share_modal_title' => 'Statistiques à partager',

resources/js/components/GroupAddEdit.vue

@@ -58,11 +58,10 @@
           ref="location"
       />
       <GroupLocationMap
-          :lat="lat"
-          :lng="lng"
+          :lat.sync="lat"
+          :lng.sync="lng"
           class="group-locationmap"
           ref="locationmap"
-          :id="lat + ',' + lng"
           v-if="lat || lng"
       />
       <GroupTimeZone
@@ -482,7 +481,11 @@ export default {
               timezone: this.timezone,
               phone: this.phone,
               image: this.image,
-              network_data: JSON.stringify(this.networkData)
+              network_data: JSON.stringify(this.networkData),
+              // Strings, not numbers: the store skips falsy values when
+              // building FormData, which would drop a 0 coordinate.
+              lat: this.lat !== null ? String(this.lat) : null,
+              lng: this.lng !== null ? String(this.lng) : null,
             }
             const id = await this.$store.dispatch('groups/create', payload)
 
@@ -515,6 +518,8 @@ export default {
                 tags: JSON.stringify((this.tagList || []).map(n => n.id)),
                 network_data: JSON.stringify(this.networkData),
                 archived_at: this.archived_at,
+                lat: this.lat !== null ? String(this.lat) : null,
+                lng: this.lng !== null ? String(this.lng) : null,
               }
 
               console.log('Edit', JSON.stringify(payload))

resources/js/components/GroupLocationMap.test.js

@@ -0,0 +1,91 @@
+import Vue from 'vue'
+import { BootstrapVue } from 'bootstrap-vue'
+Vue.use(BootstrapVue)
+
+import { mount } from '@vue/test-utils'
+import LangMixin from 'resources/js/mixins/lang.js'
+import GroupLocationMap from './GroupLocationMap.vue'
+
+const LMapStub = {
+  name: 'l-map',
+  props: ['center', 'zoom'],
+  template: '<div class="lmap-stub"><slot /></div>'
+}
+
+const LMarkerStub = {
+  name: 'l-marker',
+  props: ['latLng', 'draggable'],
+  template: '<div class="lmarker-stub" />'
+}
+
+function mountMap (props = {}) {
+  return mount(GroupLocationMap, {
+    mixins: [LangMixin],
+    propsData: { lat: 51.5, lng: -0.12, ...props },
+    stubs: {
+      'l-map': LMapStub,
+      'l-tile-layer': true,
+      'l-marker': LMarkerStub
+    }
+  })
+}
+
+// What Leaflet passes to a marker's dragend handler.
+function dragMarkerTo (wrapper, lat, lng) {
+  wrapper.findComponent(LMarkerStub).vm.$emit('dragend', { target: { getLatLng: () => ({ lat, lng }) } })
+}
+
+test('shows the drag hint', () => {
+  const wrapper = mountMap()
+  expect(wrapper.text()).toContain('partials.dragmap')
+})
+
+test('the marker is draggable and starts zoomed in on the location', () => {
+  const wrapper = mountMap()
+  expect(wrapper.findComponent(LMarkerStub).props('draggable')).toBe(true)
+  expect(wrapper.findComponent(LMapStub).props('zoom')).toBeGreaterThanOrEqual(15)
+  expect(wrapper.findComponent(LMapStub).props('center')).toEqual([51.5, -0.12])
+})
+
+test('dragging the marker emits updated lat/lng', async () => {
+  const wrapper = mountMap()
+
+  dragMarkerTo(wrapper, 52.1, 1.3)
+  await wrapper.vm.$nextTick()
+
+  expect(wrapper.emitted('update:lat').pop()).toEqual([52.1])
+  expect(wrapper.emitted('update:lng').pop()).toEqual([1.3])
+  expect(wrapper.findComponent(LMarkerStub).props('latLng')).toEqual([52.1, 1.3])
+})
+
+test('panning or zooming the map does not move the marker', async () => {
+  const wrapper = mountMap()
+
+  wrapper.findComponent(LMapStub).vm.$emit('update:center', { lat: 48.85, lng: 2.35 })
+  wrapper.findComponent(LMapStub).vm.$emit('update:zoom', 12)
+  await wrapper.vm.$nextTick()
+
+  expect(wrapper.emitted('update:lat')).toBeUndefined()
+  expect(wrapper.findComponent(LMarkerStub).props('latLng')).toEqual([51.5, -0.12])
+})
+
+test('an external lat/lng change (new geocode) moves the marker and the map', async () => {
+  const wrapper = mountMap()
+
+  await wrapper.setProps({ lat: 40.7, lng: -74.0 })
+
+  expect(wrapper.findComponent(LMarkerStub).props('latLng')).toEqual([40.7, -74.0])
+  expect(wrapper.findComponent(LMapStub).props('center')).toEqual([40.7, -74.0])
+})
+
+test('prop echo of a drag does not recentre the map', async () => {
+  const wrapper = mountMap()
+
+  dragMarkerTo(wrapper, 52.1, 1.3)
+  await wrapper.vm.$nextTick()
+
+  // Parent syncs the emitted values straight back down.
+  await wrapper.setProps({ lat: 52.1, lng: 1.3 })
+
+  expect(wrapper.findComponent(LMapStub).props('center')).toEqual([51.5, -0.12])
+})

resources/js/components/GroupLocationMap.vue

@@ -1,18 +1,25 @@
 <template>
-  <l-map
-      class="map"
-      ref="group-map"
-      :zoom="11"
-      :center="[lat, lng]"
-      :style="'width: 100%; height: 200px'"
-  >
-    <l-tile-layer :url="tiles" :attribution="attribution" />
-    <l-marker :lat-lng="[lat, lng]" :interactive="false" />
-  </l-map>
+  <div>
+    <l-map
+        class="map"
+        ref="group-map"
+        :zoom="16"
+        :center="mapCenter"
+        :style="'width: 100%; height: 200px'"
+    >
+      <l-tile-layer :url="tiles" :attribution="attribution" />
+      <l-marker :lat-lng="marker" :draggable="true" @dragend="markerDragged" />
+    </l-map>
+    <small class="text-muted">{{ __('partials.dragmap') }}</small>
+  </div>
 </template>
 <script>
 import map from '../mixins/map'
 
+// Below this difference two coordinates are "the same place" — stops the
+// parent syncing our own drag emission back down and re-centring the map.
+const EPSILON = 1e-7
+
 export default {
   mixins: [ map ],
   props: {
@@ -27,26 +34,39 @@ export default {
       default: null
     },
   },
+  data () {
+    return {
+      // The marker is the group's location; the map can be panned and zoomed
+      // independently without moving it.
+      marker: [this.lat, this.lng],
+      mapCenter: [this.lat, this.lng]
+    }
+  },
   watch: {
-    lat(newLat) {
-      this.updateMapCenter()
+    lat() {
+      this.relocate()
     },
-    lng(newLng) {
-      this.updateMapCenter()
+    lng() {
+      this.relocate()
     }
   },
   methods: {
-    updateMapCenter() {
-      if (this.lat !== null && this.lng !== null && this.$refs['group-map'] && this.$refs['group-map'].mapObject) {
-        this.$refs['group-map'].mapObject.setView([this.lat, this.lng], this.$refs['group-map'].mapObject.getZoom())
+    markerDragged(e) {
+      const pos = e.target.getLatLng()
+      this.marker = [pos.lat, pos.lng]
+      this.$emit('update:lat', pos.lat)
+      this.$emit('update:lng', pos.lng)
+    },
+    relocate() {
+      // A genuinely new position (e.g. a fresh geocode from the location
+      // field) moves the marker and the map; the echo of our own drag does not.
+      if (this.lat !== null && this.lng !== null &&
+          (Math.abs(this.lat - this.marker[0]) > EPSILON ||
+           Math.abs(this.lng - this.marker[1]) > EPSILON)) {
+        this.marker = [this.lat, this.lng]
+        this.mapCenter = [this.lat, this.lng]
       }
     }
-  },
-  mounted() {
-    // Ensure map is centered correctly on initial mount
-    this.$nextTick(() => {
-      this.updateMapCenter()
-    })
   }
 }
-</script>
+</script>