That fix does not really fix the original issue, but it makes it
impactless, and I think it's safer anyway to have upsert idempotent.
The pattern to reproduce is:
- peer A create a synced map, add a datalayer, save it
- peer B loads the map, click on edit
- at this time, peer B have twice the datalayer data, once from the
server AND once from the sync
So a better fix would be to make that peer B send a meaningfull HLC to
peer A I guess.
For this we may save the last HLC is the map properties, or maybe try to
merge the "reference_version" and the HLC.
I first tried to handle this on Leaflet.Editable side, to make it fire
the "editable:edited" event we use to trigger the sync, but deciding
what to do with a feature on escape needs some decisions that seems hard
to implement in a generic way in Leaflet.Editable.
We call stopDrawing, which then calls cancelDrawing, so here one need to
decide if cancelDrawing should keep the already drawn line (but cancel
the point being drawn) or cancel everything.
This is why I end up making this change in uMap itself.
To reproduce:
- create a map
- saved it
- change the "syncEnabled" setting to on
- save again
- open another tab with this map
- switch on edit mode
In this case, the second client will try to authenticate:
- once switch on edit mode
- and once receiving the operation message from peer A about the
syncEnabled (which calls render, which calls initSyncEngine in in this
case)
I think we want to keep render to call initSyncEngine, so if a map owner
switch off the syncEnabled setting, this will (should) disconnect the
other peers.
That fix does not really fix the original issue, but it make it impactless.
The pattern is:
- peer A create a synced map, add a datalayer, save it
- peer B loads the map, click on edit
- at this time, peer B have twice the datalayer data, once from the server
AND once from the sync
So a better fix would be to make that peer B send a meaningfull HLC to
peer A I guess.
For this we may save the last HLC is the map properties, or maybe try
to merge the "reference_version" and the HLC.
I first tried to handle this on Leaflet.Editable side, to make
it fire the "editable:edited" event we use to trigger the sync,
but deciding what to do with a feature on escape needs some
decisions that seems hard to implement in a generic way in
Leaflet.Editable.
We call stopDrawing, which then calls cancelDrawing, so here
one need to decide if cancelDrawing should keep the already
drawn line (but cancel the point being drawn) or cancel
everything.
This is why I end up making this change in uMap itself.
To reproduce:
- create a map
- saved it
- change the "syncEnabled" setting to on
- save again
- open another tab with this map
- switch on edit mode
In this case, the second client will try to authenticate:
- once switch on edit mode
- and once receiving the operation message from peer A about the
syncEnabled (which calls render, which calls initSyncEngine in
in this case)
I think we want to keep render to call initSyncEngine, so if a map
owner switch off the syncEnabled setting, this will (should) disconnect
the other peers.
When a peer save the map, other peers in dirty state should not need to
save the map anymore.
That implementation uses the lastKnownHLC as a reference, but it changes
all dirty states at once. Another impementation could be to have each
object sync its dirty state, but in this case we do not have a HLC per
object as reference, and it also creates more messages.
ping @almet if you're around and wanna have a fresh look. :)
And remove Leaflet.Toolbar dependency.
This also teach ContextMenu how to display icons instead of
text and how to render in horizontal orientation instead of
vertical.
Until now, it was displayed on the path (line or polygon) "center". So
when zoomed in, if the center is not on the screen, the tooltip will not
be visible.
Until now, it was displayed on the path (line or polygon)
"center". So when zoomed in, if the center is not on the
screen, the tooltip will not be visible.
I've chosen to keep the one in the "lint" step, instead of a
dedicated step, so to prevent one more container to be built at
each run (did I say "to save trees" ?) :p
