GSLUX-705: Modify feature geometry #151
Conversation
|
GitHub Pages links: * Luxembourg-geoportail: https://geoportail-luxembourg.github.io/luxembourg-geoportail/GSLUX-705-modify-feature/ |
and adapt style when feature become uneditable
There was a problem hiding this comment.
Thanks,
I appreciate a lot the refactoring with moving the draw parts from ol.composable into draw.coposable
But then I am not sre what ol.composable really is. It is rather ol.layer.composable whereas draw.composable would be ol.draw.composable ??
I have a remark on the use of editPoint vs. drawPoint, which might be a bit confusing.
Furthermore I have observed one (minor) bug, but this would not prevent merging in my point of view:
- if edit mode is active and a feature is deleted, it is only hidden from the map when another item is selectec
- if the feature is not in edit mode and is deleted, it disappears at once.
src/stores/draw.store.ts
Outdated
| if (drawStateActive.value === newState) { | ||
| function toggleDrawActiveState(newState: DrawStateActive) { | ||
| // allow draw of different geom type after edit, but not same type | ||
| if (editStateActive.value?.slice(4) === newState?.slice(4)) { |
There was a problem hiding this comment.
I find this use of string type enums a bit risky.
What if a refactoring changed 'draw' into 'paint' and 'draw' and 'edit' do not have both a 4 Letter prefix.
At firest I thought, the 4 letter offset was shared between draw and edit
I would suggest 2 levels of fix:
- basic fix: replace slice by editStateActive.value?.replace('edit', '') and newState?.replace('draw', '') => this might be a bit more readable
- sophisticated fix: create a class with 2 arguments: feature type (point, label, circle, etc.) and feature context (draw, edit, select)
There was a problem hiding this comment.
Thanks @mki-c2c, I really like your first idea. I just stumbled on a switch case where an object would have caused a long if else chain, so I stuck with your basic fix for now.
| :active="drawStateActive === 'drawPoint'" | ||
| @click="() => toggleActiveState('drawPoint')" | ||
| :active=" | ||
| drawStateActive === 'drawPoint' || editStateActive === 'editPoint' |
There was a problem hiding this comment.
maybe this can be simplified if the feature state has 2 parts, so we only have to check one type
- feature type
- feature context (draw, edit, etc.)
There was a problem hiding this comment.
Indeed, here an object would be more elegant.
this prevents once edited feature geometries to stay editable when another feature geometry should only be editable
tkohr
force-pushed
the
GSLUX-705-modify-feature
branch
from
f82feaf to
39a3bfe
Compare
|
Thanks for the review @mki-c2c ! Originally I thought it was a good idea to move all the ol draw and modify stuff to the ol composable, but I had the impression that it made the code much more verbose (just to get the right layer for example) since these functionalities are so closely tight with openlayers due to the interactions. This is why I finally decided to concentrate everything in the |
JIRA issue
https://jira.camptocamp.com/browse/GSLUX-705
Description
PR to handle modification of feature geometries.