From 5171d6edc913a10a0170367259927615f8ee1758 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 9 Apr 2022 23:38:11 +0200 Subject: [PATCH 1/7] feat: read-only geolocation (GDE-86) - render map into display_area - hide draw controls if read-only - remove useless refresh_button --- .../js/frappe/form/controls/geolocation.js | 112 ++++++++---------- 1 file changed, 50 insertions(+), 62 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index 688e7da3e0..008f90bc72 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -6,69 +6,69 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f async make() { await frappe.require(this.required_libs); super.make(); + $(this.input_area).addClass("hidden"); } - make_wrapper() { + set_disp_area(value) { // Create the elements for map area - super.make_wrapper(); - - let $input_wrapper = this.$wrapper.find('.control-input-wrapper'); + if (!this.disp_area) return; + this.map_id = frappe.dom.get_unique_id(); this.map_area = $( `
` ); - this.map_area.prependTo($input_wrapper); - this.$wrapper.find('.control-input').addClass("hidden"); + + $(this.disp_area).html(this.map_area); + $(this.disp_area).removeClass("like-disabled-input"); + $(this.disp_area).css("display", "block"); if (this.frm) { - this.make_map(); + this.make_map(value); } else { $(document).on('frappe.ui.Dialog:shown', () => { - this.make_map(); + this.make_map(value); }); } } - make_map() { + make_map(value) { this.bind_leaflet_map(); this.bind_leaflet_draw_control(); + this.bind_leaflet_event_listeners(); this.bind_leaflet_locate_control(); - this.bind_leaflet_refresh_button(); + this.bind_leaflet_data(value); } - format_for_input(value) { - if (!this.map) return; - // render raw value from db into map + bind_leaflet_data(value) { + /* render raw value from db into map */ + if (!this.map || !value) return; this.clear_editable_layers(); - if(value) { - var data_layers = new L.FeatureGroup() - .addLayer(L.geoJson(JSON.parse(value),{ - pointToLayer: function(geoJsonPoint, latlng) { - if (geoJsonPoint.properties.point_type == "circle"){ - return L.circle(latlng, {radius: geoJsonPoint.properties.radius}); - } else if (geoJsonPoint.properties.point_type == "circlemarker") { - return L.circleMarker(latlng, {radius: geoJsonPoint.properties.radius}); - } - else { - return L.marker(latlng); - } + + var data_layers = new L.FeatureGroup() + .addLayer(L.geoJson(JSON.parse(value),{ + pointToLayer: function(geoJsonPoint, latlng) { + if (geoJsonPoint.properties.point_type == "circle"){ + return L.circle(latlng, {radius: geoJsonPoint.properties.radius}); + } else if (geoJsonPoint.properties.point_type == "circlemarker") { + return L.circleMarker(latlng, {radius: geoJsonPoint.properties.radius}); } - })); - this.add_non_group_layers(data_layers, this.editableLayers); - try { - this.map.fitBounds(this.editableLayers.getBounds(), { - padding: [50,50] - }); - } - catch(err) { - // suppress error if layer has a point. - } - this.editableLayers.addTo(this.map); - } else { - this.map.setView(frappe.utils.map_defaults.center, frappe.utils.map_defaults.zoom); + else { + return L.marker(latlng); + } + } + })); + this.add_non_group_layers(data_layers, this.editableLayers); + try { + this.map.fitBounds(this.editableLayers.getBounds(), { + padding: [50,50] + }); } + catch(err) { + // suppress error if layer has a point. + } + this.editableLayers.addTo(this.map); this.map.invalidateSize(); } @@ -98,9 +98,12 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f L.Icon.Default.imagePath = '/assets/frappe/images/leaflet/'; this.map = L.map(this.map_id); + this.map.setView(frappe.utils.map_defaults.center, frappe.utils.map_defaults.zoom); L.tileLayer(frappe.utils.map_defaults.tiles, frappe.utils.map_defaults.options).addTo(this.map); + + this.editableLayers = new L.FeatureGroup(); } bind_leaflet_locate_control() { @@ -110,9 +113,13 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f } bind_leaflet_draw_control() { - this.editableLayers = new L.FeatureGroup(); + if (!frappe.perm.has_perm(this.doctype, this.df.permlevel, 'write', this.doc)) return; - var options = { + this.map.addControl(this.get_leaflet_controls()); + } + + get_leaflet_controls() { + return new L.Control.Draw({ position: 'topleft', draw: { polyline: { @@ -142,12 +149,10 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f featureGroup: this.editableLayers, //REQUIRED!! remove: true } - }; - - // create control and add to map - this.drawControl = new L.Control.Draw(options); - this.map.addControl(this.drawControl); + }); + } + bind_leaflet_event_listeners() { this.map.on('draw:created', (e) => { var type = e.layerType, layer = e.layer; @@ -165,23 +170,6 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f }); } - bind_leaflet_refresh_button() { - L.easyButton({ - id: 'refresh-map-'+this.df.fieldname, - position: 'topright', - type: 'replace', - leafletClasses: true, - states:[{ - stateName: 'refresh-map', - onClick: function(button, map){ - map._onResize(); - }, - title: 'Refresh map', - icon: 'fa fa-refresh' - }] - }).addTo(this.map); - } - add_non_group_layers(source_layer, target_group) { // https://gis.stackexchange.com/a/203773 // Would benefit from https://github.com/Leaflet/Leaflet/issues/4461 From 892150eb7397bf229d132b3621fafa81f568bc61 Mon Sep 17 00:00:00 2001 From: marination Date: Fri, 19 May 2023 16:56:54 +0530 Subject: [PATCH 2/7] fix: Load map libraries at build time to avoid async issues during geolocation render - When gelocation control is built, it awaits loading of libraries - Withi the control everything is await but a layer above where multiple controls are sequentially built this breaks - Form render goes ahead without waiting for the gelocation control - The `onload_post_render` in `Location` cannot find the map wrapper as the control is still being built - Therefore, on a hard load, the control does not show up and appears on soft reload. --- .../public/js/frappe/form/controls/geolocation.js | 14 -------------- .../lib/leaflet_control_locate/L.Control.Locate.js | 2 +- frappe/public/js/libs.bundle.js | 4 ++++ frappe/public/scss/desk.bundle.scss | 5 +++++ 4 files changed, 10 insertions(+), 15 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index c2ce336266..026457b856 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -4,7 +4,6 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f static horizontal = false; async make() { - await frappe.require(this.required_libs); super.make(); $(this.input_area).addClass("hidden"); } @@ -200,17 +199,4 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f this.editableLayers.removeLayer(l); }); } - - get required_libs() { - return [ - "assets/frappe/js/lib/leaflet_easy_button/easy-button.css", - "assets/frappe/js/lib/leaflet_control_locate/L.Control.Locate.css", - "assets/frappe/js/lib/leaflet_draw/leaflet.draw.css", - "assets/frappe/js/lib/leaflet/leaflet.css", - "assets/frappe/js/lib/leaflet/leaflet.js", - "assets/frappe/js/lib/leaflet_easy_button/easy-button.js", - "assets/frappe/js/lib/leaflet_draw/leaflet.draw.js", - "assets/frappe/js/lib/leaflet_control_locate/L.Control.Locate.js", - ]; - } }; diff --git a/frappe/public/js/lib/leaflet_control_locate/L.Control.Locate.js b/frappe/public/js/lib/leaflet_control_locate/L.Control.Locate.js index 8544e17a04..8ea44ce00c 100644 --- a/frappe/public/js/lib/leaflet_control_locate/L.Control.Locate.js +++ b/frappe/public/js/lib/leaflet_control_locate/L.Control.Locate.js @@ -17,7 +17,7 @@ You can find the project at: https://github.com/domoritz/leaflet-locatecontrol if (typeof window !== 'undefined' && window.L) { module.exports = factory(L); } else { - module.exports = factory(require('leaflet')); + module.exports = factory(require('../leaflet/leaflet.js')); } } diff --git a/frappe/public/js/libs.bundle.js b/frappe/public/js/libs.bundle.js index e4e172c1b4..77704bb173 100644 --- a/frappe/public/js/libs.bundle.js +++ b/frappe/public/js/libs.bundle.js @@ -1,5 +1,9 @@ import "./jquery-bootstrap"; import "./lib/moment"; +import "../js/lib/leaflet/leaflet.js"; +import "../js/lib/leaflet_easy_button/easy-button.js"; +import "../js/lib/leaflet_draw/leaflet.draw.js"; +import "../js/lib/leaflet_control_locate/L.Control.Locate.js"; import Sortable from "sortablejs"; window.SetVueGlobals = (app) => { diff --git a/frappe/public/scss/desk.bundle.scss b/frappe/public/scss/desk.bundle.scss index 10fd116d6c..6b192bb3ff 100644 --- a/frappe/public/scss/desk.bundle.scss +++ b/frappe/public/scss/desk.bundle.scss @@ -4,3 +4,8 @@ @import "~frappe-charts/dist/frappe-charts.min"; @import "~plyr/dist/plyr"; @import "./desk/index"; + +@import "frappe/public/js/lib/leaflet/leaflet.css"; +@import "frappe/public/js/lib/leaflet_easy_button/easy-button.css"; +@import "frappe/public/js/lib/leaflet_control_locate/L.Control.Locate.css"; +@import "frappe/public/js/lib/leaflet_draw/leaflet.draw.css"; \ No newline at end of file From d6edc1530ec8705b41e57318f3e8ed719542dc2b Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 22 May 2023 15:08:26 +0200 Subject: [PATCH 3/7] refactor: const instead of var, indentation --- .../js/frappe/form/controls/geolocation.js | 63 ++++++++++--------- 1 file changed, 33 insertions(+), 30 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index 026457b856..c886864059 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -10,14 +10,14 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f set_disp_area(value) { // Create the elements for map area - if (!this.disp_area) return; - + if (!this.disp_area) { + return; + } + this.map_id = frappe.dom.get_unique_id(); this.map_area = $( `
-
+
` ); @@ -46,7 +46,7 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f this.map.zoomControl.remove(); } else { this.bind_leaflet_draw_control(); - this.bind_leaflet_event_listeners(); + this.bind_leaflet_event_listeners(); this.bind_leaflet_locate_control(); this.bind_leaflet_data(value); } @@ -54,29 +54,30 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f bind_leaflet_data(value) { /* render raw value from db into map */ - if (!this.map || !value) return; + if (!this.map || !value) { + return; + } this.clear_editable_layers(); - var data_layers = new L.FeatureGroup() - .addLayer(L.geoJson(JSON.parse(value),{ - pointToLayer: function(geoJsonPoint, latlng) { - if (geoJsonPoint.properties.point_type == "circle"){ - return L.circle(latlng, {radius: geoJsonPoint.properties.radius}); + const data_layers = new L.FeatureGroup().addLayer( + L.geoJson(JSON.parse(value), { + pointToLayer: function (geoJsonPoint, latlng) { + if (geoJsonPoint.properties.point_type == "circle") { + return L.circle(latlng, { radius: geoJsonPoint.properties.radius }); } else if (geoJsonPoint.properties.point_type == "circlemarker") { - return L.circleMarker(latlng, {radius: geoJsonPoint.properties.radius}); - } - else { + return L.circleMarker(latlng, { radius: geoJsonPoint.properties.radius }); + } else { return L.marker(latlng); } - } - })); + }, + }) + ); this.add_non_group_layers(data_layers, this.editableLayers); try { this.map.fitBounds(this.editableLayers.getBounds(), { - padding: [50,50] + padding: [50, 50], }); - } - catch(err) { + } catch (err) { // suppress error if layer has a point. } this.editableLayers.addTo(this.map); @@ -84,10 +85,10 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f } bind_leaflet_map() { - var circleToGeoJSON = L.Circle.prototype.toGeoJSON; + const circleToGeoJSON = L.Circle.prototype.toGeoJSON; L.Circle.include({ toGeoJSON: function () { - var feature = circleToGeoJSON.call(this); + const feature = circleToGeoJSON.call(this); feature.properties = { point_type: "circle", radius: this.getRadius(), @@ -98,7 +99,7 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f L.CircleMarker.include({ toGeoJSON: function () { - var feature = circleToGeoJSON.call(this); + const feature = circleToGeoJSON.call(this); feature.properties = { point_type: "circlemarker", radius: this.getRadius(), @@ -114,8 +115,8 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f L.tileLayer(frappe.utils.map_defaults.tiles, frappe.utils.map_defaults.options).addTo( this.map ); - - this.editableLayers = new L.FeatureGroup(); + + this.editableLayers = new L.FeatureGroup(); } bind_leaflet_locate_control() { @@ -125,7 +126,9 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f } bind_leaflet_draw_control() { - if (!frappe.perm.has_perm(this.doctype, this.df.permlevel, 'write', this.doc)) return; + if (!frappe.perm.has_perm(this.doctype, this.df.permlevel, "write", this.doc)) { + return; + } this.map.addControl(this.get_leaflet_controls()); } @@ -159,13 +162,13 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f }, edit: { featureGroup: this.editableLayers, //REQUIRED!! - remove: true - } + remove: true, + }, }); } bind_leaflet_event_listeners() { - this.map.on('draw:created', (e) => { + this.map.on("draw:created", (e) => { var type = e.layerType, layer = e.layer; if (type === "marker") { @@ -176,7 +179,7 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f }); this.map.on("draw:deleted draw:edited", (e) => { - var layer = e.layer; + const { layer } = e; this.editableLayers.removeLayer(layer); this.set_value(JSON.stringify(this.editableLayers.toGeoJSON())); }); From 50b15be1c26dee5fa65bde64863323ede2460051 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Mon, 22 May 2023 15:09:01 +0200 Subject: [PATCH 4/7] fix: handle read only property --- frappe/public/js/frappe/form/controls/geolocation.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index c886864059..37d799e96d 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -126,7 +126,10 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f } bind_leaflet_draw_control() { - if (!frappe.perm.has_perm(this.doctype, this.df.permlevel, "write", this.doc)) { + if ( + !frappe.perm.has_perm(this.doctype, this.df.permlevel, "write", this.doc) || + this.df.read_only + ) { return; } From ba2251219141cc57b714bd3d6aece682d419de1c Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 3 Jun 2023 14:16:43 +0200 Subject: [PATCH 5/7] refactor: call control's `on_section_collapse()` This way, the logic can stay in the control itself. Each control can decide what it needs to do on section collapse/expand. --- frappe/public/js/frappe/form/controls/signature.js | 3 +++ frappe/public/js/frappe/form/section.js | 7 +------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/signature.js b/frappe/public/js/frappe/form/controls/signature.js index 0cbc1f3c26..6ab96012f1 100644 --- a/frappe/public/js/frappe/form/controls/signature.js +++ b/frappe/public/js/frappe/form/controls/signature.js @@ -133,4 +133,7 @@ frappe.ui.form.ControlSignature = class ControlSignature extends frappe.ui.form. this.set_my_value(base64_img); this.set_image(this.get_value()); } + on_section_collapse() { + this.refresh(); + } }; diff --git a/frappe/public/js/frappe/form/section.js b/frappe/public/js/frappe/form/section.js index a692cbac0d..b4908e749a 100644 --- a/frappe/public/js/frappe/form/section.js +++ b/frappe/public/js/frappe/form/section.js @@ -110,12 +110,7 @@ export default class Section { this.set_icon(hide); - // refresh signature fields - this.fields_list.forEach((f) => { - if (f.df.fieldtype == "Signature") { - f.refresh(); - } - }); + this.fields_list.forEach((f) => f.on_section_collapse && f.on_section_collapse(hide)); // save state for next reload ('' is falsy) if (this.df.css_class) From 79aaf072bde8646d11cc7a5e80d73a0ffa74f2a6 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 3 Jun 2023 14:17:59 +0200 Subject: [PATCH 6/7] fix: fit and recenter map when section is expanded --- .../js/frappe/form/controls/geolocation.js | 25 +++++++++++++------ 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index 37d799e96d..29d36539ec 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -73,15 +73,8 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f }) ); this.add_non_group_layers(data_layers, this.editableLayers); - try { - this.map.fitBounds(this.editableLayers.getBounds(), { - padding: [50, 50], - }); - } catch (err) { - // suppress error if layer has a point. - } this.editableLayers.addTo(this.map); - this.map.invalidateSize(); + this.fit_and_recenter_map(); } bind_leaflet_map() { @@ -205,4 +198,20 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f this.editableLayers.removeLayer(l); }); } + + fit_and_recenter_map() { + // Spread map across the wrapper, recenter and zoom w.r.t bounds + try { + this.map.invalidateSize(); + this.map.fitBounds(this.editableLayers.getBounds(), { + padding: [50, 50], + }); + } catch (err) { + // suppress error if layer has a point. + } + } + + on_section_collapse(hide) { + !hide && this.fit_and_recenter_map(); + } }; From f9251f0f5b2a051def991bd1e7828ba83e425a11 Mon Sep 17 00:00:00 2001 From: barredterra <14891507+barredterra@users.noreply.github.com> Date: Sat, 3 Jun 2023 14:38:14 +0200 Subject: [PATCH 7/7] refactor: move pointToLayer into a separate method For better customizability --- .../js/frappe/form/controls/geolocation.js | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/frappe/public/js/frappe/form/controls/geolocation.js b/frappe/public/js/frappe/form/controls/geolocation.js index 29d36539ec..ada729fd66 100644 --- a/frappe/public/js/frappe/form/controls/geolocation.js +++ b/frappe/public/js/frappe/form/controls/geolocation.js @@ -60,23 +60,31 @@ frappe.ui.form.ControlGeolocation = class ControlGeolocation extends frappe.ui.f this.clear_editable_layers(); const data_layers = new L.FeatureGroup().addLayer( - L.geoJson(JSON.parse(value), { - pointToLayer: function (geoJsonPoint, latlng) { - if (geoJsonPoint.properties.point_type == "circle") { - return L.circle(latlng, { radius: geoJsonPoint.properties.radius }); - } else if (geoJsonPoint.properties.point_type == "circlemarker") { - return L.circleMarker(latlng, { radius: geoJsonPoint.properties.radius }); - } else { - return L.marker(latlng); - } - }, - }) + L.geoJson(JSON.parse(value), { pointToLayer: this.point_to_layer }) ); this.add_non_group_layers(data_layers, this.editableLayers); this.editableLayers.addTo(this.map); this.fit_and_recenter_map(); } + /** + * Defines custom rules for how geoJSON data is rendered on the map. + * + * @param {Object} geoJsonPoint - The geoJSON object to be rendered on the map. + * @param {Object} latlng - The latitude and longitude where the geoJSON data should be rendered on the map. + * @returns {Object} - Returns the Leaflet layer object to be rendered on the map. + */ + point_to_layer(geoJsonPoint, latlng) { + // Custom rules for how geojson data is rendered on the map + if (geoJsonPoint.properties.point_type == "circle") { + return L.circle(latlng, { radius: geoJsonPoint.properties.radius }); + } else if (geoJsonPoint.properties.point_type == "circlemarker") { + return L.circleMarker(latlng, { radius: geoJsonPoint.properties.radius }); + } else { + return L.marker(latlng); + } + } + bind_leaflet_map() { const circleToGeoJSON = L.Circle.prototype.toGeoJSON; L.Circle.include({