Conversation
acff01d to
64f0437
Compare
b9d77e1 to
778df93
Compare
1d3f155 to
1408ea9
Compare
07c1368 to
2268dab
Compare
2268dab to
d8fb125
Compare
jedef
left a comment
There was a problem hiding this comment.
Overall very good changes!
4429dd7 to
fc7d55a
Compare
jedef
left a comment
There was a problem hiding this comment.
seems good, but there's one place where I am not sure if there is a race condition.
pakb
left a comment
There was a problem hiding this comment.
discussing very minor changes
otherwise looks good!
9a71f57 to
a7df06b
Compare
| if ('addToMap' in payload && 'layerId' in payload) { | ||
| const layer = rootGetters.getActiveLayerById(payload.layerId)?.clone() | ||
| if (layer) { | ||
| layer.addToMap = payload.addToMap |
There was a problem hiding this comment.
not sure this will not raise an error / warning, we should only edit the store in the context of a mutation (actions are out of scope too)
There was a problem hiding this comment.
@pakb The store is not changed here, the layer is cloned and the clone is changed and pass to addLayer() where the store mutation happens in a mutation.
There was a problem hiding this comment.
Note we have the same code in the layers store actions, where the store value is cloned in order to make changes in the action which then trigger a mutation with the new value.
There was a problem hiding this comment.
oh right, didn't catch the .clone() up there, so all is good
There was a problem hiding this comment.
check my last review on #348 and see if it's not too hard to achieve, then we can merge this one
Store mutation in the router were notified twice because the mutations were subscribed twice instead of one.
…de automatically Thee adminId is now removed from the URL and upon adminId detection, the drawing menu is automatically opened. Re-worked the KML metadata for simplification, now it is set together with the layer in the layer store action. This simplify the url to layer store logic. Also avoid to have duplicate layer object in the store, now the active/selected kml layer for drawing is only kept in the layer store, previously it was cloned and also kept in the drawing store. The drawing menu has a copy of the layer when the drawing mode is open, but this copy is only used by the drawing module and is not in a store ! To avoid having the kml default opacity hardcoded in several places in the code the KML layer class constructor has been changed in order to have the opacity optional. So now the default opacity is only hardcoded in one place in the KMLLayer class.
…ing menu The adminId is kept in memory until next reload. Also keep the layerId in the url when inside the drawing mode in order not to loose if the page is reloaded. We also uses the last KML in the active list (independently of visible flag) as the kml to edit/copy. This is need because if the user is reloading the page while editing the layer will be invisible after reloading, so the user still have the possibility (without making the layer visible again) to copy edit his drawing.
Let Vue router to handle URL encoding and parsing without a special hack for the layers parameter. This caused troubled with the E2E tests. Moreover we should be consistent in the encoding and not use any special case for one parameter. This complicate the code and make it more error prone.
Waiting on the activelayers array size is not enough after closing the drawing mode as the drawing layer is added in invisible mode before closing and its visible flag is toggled on close.
When a KML admin ID was detected in the layer and the active layers was updated, the drawing mode was automatically open. This was expected upon parsing the KML admin ID from url, but this also happened when the active layers changed like for example toggling any layer visibility or config or re-ordering layers. Now we explicitely open the drawing mode on admin id parsed from url parsing.
Now while drawing the kml layer visible flag is kept to true, this allow the user to reload the page while drawing without loosing the visible flag. This commit also fixed the issue that a drawing copy would be added to the active layer list instead of replacing the copied drawing. Re-order the layers store actions and mutation to have logic order, this make the code easier to read/navigate.
The auto open drawing mode based on KML admin ID from Url had a possible race condition, which never occurred during testing, but to be safe this race condition is handled here.
17f99ab to
35cc121
Compare
Thee adminId is now removed from the URL and upon adminId detection, the drawing
menu is automatically opened.
Test link
Test link with legacy url kml adminId
Test link with kml adminId
Test link with kml
External WMS with mf-geoadmin3 URL syntax
External WMTS with new URL syntax
Test link