[16.0][ADD] base_geoengine: geoengine view in edit mode #338
Conversation
… returned by fields_view_get
- separate view and widgets in 2 files - change js models using 'odoo.define'
…in a mixin abstract model
…isplay values of selection field instead of technical value in info map box in geoengine view
The feature to set a symbol with symbol_url or symbol_binary on vector layers was broken. This replace those fields by a new object vector symbol on which you can specify which symbol to use depending on a field value.
geo_contains calls St_Contains SQL search. Plus fix search to set SRID to avoid mixin srid issue.
And extract Swisstopo to move it in a new module
Otherwise partial value is interpreted as context and it will fails on Odoo loading
ged-odoo
left a comment
ged-odoo
left a comment
There was a problem hiding this comment.
as I was pinged, I just did a quick review. I hope that's ok. This is only my comments after a quick look. This is way too big for me to have a serious look in 30min.
However, my overall impression is that it feels like a pretty good work. It involves so many different parts of the JS framework, and it looks like they are all used properly. So congratulation to everyone who worked on this.
| @@ -0,0 +1,36 @@ | |||
| /** @odoo-module */ | |||
| const {reactive} = owl; | |||
There was a problem hiding this comment.
random remark: you can import owl from @odoo/owl, and it is now the recommended way to do it.
import { reactive } from "@odoo/owl"| @@ -0,0 +1,402 @@ | |||
| /* OpenLayers Style */ | |||
There was a problem hiding this comment.
in odoo, we decided to remove the /css, /js and /xml subfolders. The new guideline is to put everything in /static/src, and locate related code together (so, typically, the xml/js/scss for a component should be located at the same place
| return this.rasters.length; | ||
| } | ||
| } | ||
| export const rasterLayersStore = reactive(new RasterLayersStore()); |
There was a problem hiding this comment.
looks good to me. The reactive function opened up a lot of new ways of organizing the code.
| const activeFields = {}; | ||
| const geoengineAttr = {}; | ||
|
|
||
| this.visitXML(xmlDoc, (node) => { |
There was a problem hiding this comment.
i am not a fan of our archparser system. it feels way too complicated. sorry for that.
|
|
||
| export class LayersPanel extends Component { | ||
| setup() { | ||
| super.setup(); |
There was a problem hiding this comment.
you don't have to call super.setup if you inherit from Component. Owl guarantees that the setup function of Component is always empty
| } | ||
|
|
||
| .ol-scale-bar-inner { | ||
| display: flex; |
There was a problem hiding this comment.
the guideline in odoo is now to prefer using boostrap css class above custom css. It helped a lot to reduce repetitive common css properties, such as display flex. So, for example, here, you could simply set d-flex on the element. (note that it may not work here, if you don't own the code that render this css class). Just saying that if you want, you can probably replace some of your css with bootstrap.
| this.id = `map_${Date.now()}`; | ||
| this.orm = useService("orm"); | ||
|
|
||
| onWillStart(() => Promise.all([this.loadJsFiles()])); |
There was a problem hiding this comment.
you can now create a bundle and just load the bundle here
| } | ||
| } | ||
|
|
||
| registry.category("fields").add("domain", DomainFieldExtend); |
There was a problem hiding this comment.
you can use the force: true option, and not have to remove it from the registry before
| @@ -0,0 +1,34 @@ | |||
| /** @odoo-module **/ | |||
|
|
|||
| import {DomainField} from "@web/views/fields/domain/domain_field"; | |||
There was a problem hiding this comment.
be aware that we changed the domain selector a lot in master, so sadly, there will be some migration work to migrate this addon to 17.0 in the future
| }); | ||
| } | ||
|
|
||
| async loadJsFiles() { |
There was a problem hiding this comment.
you can just put all your js/css files in a new bundle, and load that bundle at once
test-requirements
kouffsamuel
force-pushed
the
16.0-add_editable_geoengine_view
branch
from
3ca04d8 to
f965e55
Compare
|
@ged-odoo I'm so grateful for your review. Thank you!!!! |
base_geoengine/static/src/js/views/geoengine/layers_panel/layers_panel.esm.js
Outdated
Show resolved
Hide resolved
kouffsamuel
force-pushed
the
16.0-add_editable_geoengine_view
branch
from
37b0295 to
a55af4f
Compare
|
ocabot merge nobump |
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
@lmignon The merge process could not be finalized, because command |
[FIX] base_geoengine: Fix documentation format
|
/ocabot merge nobump |
|
This PR looks fantastic, let's merge it! |
|
Congratulations, your PR was merged at c1cd92d. Thanks a lot for contributing to OCA. ❤️ |
20 participants
This PR will allow users to modify or create new records from the geoengine view.