web mailing list archives

web@odoo-community.org

Avatar

Re: [OCA/web] [12.0][ADD] web_widget_map (#1447)

by Sylvain LE GAL <notifications@github.com> - 13/01/2020 12:36:37

@legalsylvain requested changes on this pull request.

Question about design inline.

regards.


In web_widget_map/README.rst:

> +    :alt: Try me on Runbot
+
+|badge1| |badge2| |badge3| |badge4| |badge5| 
+
+This module add the possibility to insert a map into Odoo form views using
+the OpenLayers library.
+
+**Table of contents**
+
+.. contents::
+   :local:
+
+Usage
+=====
+
+To insert a Bokeh chart in a view proceed as follows:

Bokeh chart ? I think there is an error in a copy / paste. Don't you think ?


In web_widget_map/README.rst:

> +    <widget name='map_widget'/>
+
+
+When the widget is set in a view, the user will see a button, clicking on
+the button will render the map.
+
+.. figure:: https://raw.githubusercontent.com/OCA/web/12.0/web_widget_map/static/src/img/view_map_button.png
+    :align: center
+    :width: 600 px
+
+.. figure:: https://raw.githubusercontent.com/OCA/web/12.0/web_widget_map/static/src/img/view_map.png
+    :align: center
+    :width: 600 px
+
+By default the center of the map is set to latitude 0 and longitude 0.
+Refer to the module hr_attendance_geolocation_map to see an example on how

greats to have an exemple, without refering on a another module.
or at least, set an url to find easily this module. (https://github.com/OCA/hr-attendance)

regards.


In web_widget_map/static/src/js/web_widget_map.js:

> +    var Widget = require('web.Widget');
+
+    var MapWidget = Widget.extend({
+        template: 'Map',
+        events: {
+            'click #loadMap': '_initializeMap',
+        },
+        init: function () {
+            this._super.apply(this, arguments);
+            this.mapLayers = [new ol.layer.Tile({
+                source: new ol.source.OSM()
+            })];
+            this.centerMap = [0, 0];
+            this.markers = [];
+        },
+        _initializeMap: function () {

I'm not sure about the design.
you are calling _initializeMap and in the hr module, it makes an rpc call.
https://github.com/OCA/hr/pull/731/files#diff-4f79c3ed43c3d1ec84dd1097f991c34aR17

but if you have two modules that depends on this widget, I think that the call will be done for both model, and generate errors.
I think that refactoring is needed. don't you think ?


You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.