From cc2625bfacdb20937af77a79eefc4b57da73b4bd Mon Sep 17 00:00:00 2001 From: Yohan Boniface Date: Thu, 13 Mar 2025 11:39:49 +0100 Subject: [PATCH] wip: undo redo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Alexis Métaireau --- umap/static/umap/css/bar.css | 12 +- umap/static/umap/css/icon.css | 6 +- umap/static/umap/img/16-white.svg | 12 +- umap/static/umap/img/16.svg | 2 +- umap/static/umap/img/source/16-white.svg | 14 +- umap/static/umap/img/source/16.svg | 2 +- umap/static/umap/js/modules/data/features.js | 16 +- umap/static/umap/js/modules/data/layer.js | 9 +- umap/static/umap/js/modules/form/builder.js | 3 +- umap/static/umap/js/modules/rendering/ui.js | 14 +- umap/static/umap/js/modules/sync/engine.js | 32 ++- umap/static/umap/js/modules/sync/undo.js | 71 ++++++ umap/static/umap/js/modules/sync/updaters.js | 16 +- umap/static/umap/js/modules/ui/bar.js | 19 +- umap/static/umap/js/modules/umap.js | 9 + umap/static/umap/js/umap.controls.js | 15 +- umap/static/umap/map.css | 2 +- umap/tests/base.py | 2 +- umap/tests/integration/test_undo_redo.py | 231 +++++++++++++++++++ 19 files changed, 447 insertions(+), 40 deletions(-) create mode 100644 umap/static/umap/js/modules/sync/undo.js create mode 100644 umap/tests/integration/test_undo_redo.py diff --git a/umap/static/umap/css/bar.css b/umap/static/umap/css/bar.css index cf71e2c1..07356939 100644 --- a/umap/static/umap/css/bar.css +++ b/umap/static/umap/css/bar.css @@ -14,7 +14,8 @@ background-color: inherit; } .leaflet-container .edit-save, -.leaflet-container .edit-cancel, +.leaflet-container .edit-undo, +.leaflet-container .edit-redo, .leaflet-container .edit-disable, .leaflet-container .connected-peers { @@ -39,7 +40,8 @@ color: var(--color-darkGray); } -.leaflet-container .edit-cancel:hover, +.leaflet-container .edit-undo:hover, +.leaflet-container .edit-redo:hover, .leaflet-container .edit-disable:hover { border: 0.5px solid rgba(153, 153, 153, 0.80); text-decoration: none; @@ -76,14 +78,16 @@ background: rgba(66, 236, 230, 0.10); } .leaflet-container .edit-save, -.leaflet-container .edit-cancel, +.leaflet-container .edit-undo, +.leaflet-container .edit-redo, .leaflet-container .edit-disable, .umap-edit-enabled .edit-enable { display: none; } .umap-edit-enabled .edit-save, .umap-edit-enabled .edit-disable, -.umap-edit-enabled.umap-is-dirty .edit-cancel { +.umap-edit-enabled.umap-is-dirty .edit-undo, +.umap-edit-enabled.umap-is-dirty .edit-redo { display: inline-block; } .umap-is-dirty .edit-disable { diff --git a/umap/static/umap/css/icon.css b/umap/static/umap/css/icon.css index dfcffa72..30d5ce3b 100644 --- a/umap/static/umap/css/icon.css +++ b/umap/static/umap/css/icon.css @@ -167,11 +167,15 @@ html[dir="rtl"] .icon { .icon-profile { background-position: 0 calc(var(--tile) * 4); } +.icon-redo { + background-position: calc(var(--tile) * 3) calc(var(--tile) * 7); +} .icon-resize { background-position: calc(var(--tile) * 3) calc(var(--tile) * 6); } +.icon-undo, .icon-restore { - background-position: calc(var(--tile) * 5) calc(var(--tile) * 3); + background-position: calc(var(--tile) * 2) calc(var(--tile) * 7); } .expanded .icon-resize { background-position: calc(var(--tile) * 2) calc(var(--tile) * 6); diff --git a/umap/static/umap/img/16-white.svg b/umap/static/umap/img/16-white.svg index f61c1682..eae286ae 100644 --- a/umap/static/umap/img/16-white.svg +++ b/umap/static/umap/img/16-white.svg @@ -18,6 +18,9 @@ + + + @@ -67,9 +70,12 @@ - + + + + @@ -143,6 +149,10 @@ + + + + diff --git a/umap/static/umap/img/16.svg b/umap/static/umap/img/16.svg index e33dc876..eb32402f 100644 --- a/umap/static/umap/img/16.svg +++ b/umap/static/umap/img/16.svg @@ -1 +1 @@ -image/svg+xml   +image/svg+xml   diff --git a/umap/static/umap/img/source/16-white.svg b/umap/static/umap/img/source/16-white.svg index 7bb8097b..34efb72f 100644 --- a/umap/static/umap/img/source/16-white.svg +++ b/umap/static/umap/img/source/16-white.svg @@ -21,8 +21,11 @@ + + + - + @@ -78,9 +81,12 @@ - + + + + @@ -154,6 +160,10 @@ + + + + diff --git a/umap/static/umap/img/source/16.svg b/umap/static/umap/img/source/16.svg index 454d2fad..d264c313 100644 --- a/umap/static/umap/img/source/16.svg +++ b/umap/static/umap/img/source/16.svg @@ -1,4 +1,4 @@ -image/svg+xml   +image/svg+xml   diff --git a/umap/static/umap/js/modules/data/features.js b/umap/static/umap/js/modules/data/features.js index 8ccfe9ba..8278d498 100644 --- a/umap/static/umap/js/modules/data/features.js +++ b/umap/static/umap/js/modules/data/features.js @@ -91,6 +91,7 @@ class Feature { } set geometry(value) { + this._geometry_bk = Utils.CopyJSON(this._geometry) this._geometry = value this.pushGeometry() } @@ -104,13 +105,17 @@ class Feature { } pullGeometry(sync = true) { + const oldGeometry = Utils.CopyJSON(this._geometry) this.fromLatLngs(this._getLatLngs()) if (sync) { - this.sync.update('geometry', this.geometry) + console.log('sync geometry') + this.sync.update('geometry', this.geometry, oldGeometry) } } fromLatLngs(latlngs) { + console.log('fromLatLngs', latlngs) + this._geometry_bk = Utils.CopyJSON(this._geometry) this._geometry = this.convertLatLngs(latlngs) } @@ -145,8 +150,15 @@ class Feature { onCommit() { // When the layer is a remote layer, we don't want to sync the creation of the // points via the websocket, as the other peers will get them themselves. + const oldGeoJSON = this._just_married ? null : Utils.CopyJSON(this.toGeoJSON()) + this.pullGeometry(false) if (this.datalayer?.isRemoteLayer()) return - this.sync.upsert(this.toGeoJSON()) + if (this._just_married) { + this.sync.upsert(this.toGeoJSON(), null) + this._just_married = false + } else { + this.sync.update('geometry', this.geometry, this._geometry_bk) + } } isReadOnly() { diff --git a/umap/static/umap/js/modules/data/layer.js b/umap/static/umap/js/modules/data/layer.js index 88ab9667..a645e11b 100644 --- a/umap/static/umap/js/modules/data/layer.js +++ b/umap/static/umap/js/modules/data/layer.js @@ -417,7 +417,11 @@ export class DataLayer extends ServerStored { removeFeature(feature, sync) { const id = stamp(feature) - if (sync !== false) feature.sync.delete() + if (sync !== false) { + const oldValue = feature.toGeoJSON() + console.log('oldValue in removeFeature', oldValue) + feature.sync.delete(oldValue) + } this.hideFeature(feature) delete this._umap.featuresIndex[feature.getSlug()] feature.disconnectFromDataLayer(this) @@ -596,10 +600,11 @@ export class DataLayer extends ServerStored { } del(sync = true) { + const oldValue = Utils.CopyJSON(this.umapGeoJSON()) this.erase() if (sync) { this.isDeleted = true - this.sync.delete() + this.sync.delete(oldValue) } } diff --git a/umap/static/umap/js/modules/form/builder.js b/umap/static/umap/js/modules/form/builder.js index b505c817..d20d1770 100644 --- a/umap/static/umap/js/modules/form/builder.js +++ b/umap/static/umap/js/modules/form/builder.js @@ -190,13 +190,14 @@ export class MutatingForm extends Form { } setter(field, value) { + const oldValue = this.getter(field) super.setter(field, value) this.obj.isDirty = true if ('render' in this.obj) { this.obj.render([field], this) } if ('sync' in this.obj) { - this.obj.sync.update(field, value) + this.obj.sync.update(field, value, oldValue) } } diff --git a/umap/static/umap/js/modules/rendering/ui.js b/umap/static/umap/js/modules/rendering/ui.js index 1755c046..6f18bbda 100644 --- a/umap/static/umap/js/modules/rendering/ui.js +++ b/umap/static/umap/js/modules/rendering/ui.js @@ -97,7 +97,6 @@ const FeatureMixin = { }, onCommit: function () { - this.feature.pullGeometry(false) this.feature.onCommit() }, } @@ -112,7 +111,7 @@ const PointMixin = { this.on('dragend', (event) => { this.isDirty = true this.feature.edit(event) - this.feature.pullGeometry(false) + // this.feature.pullGeometry(false) }) if (!this.feature.isReadOnly()) this.on('mouseover', this._enableDragging) this.on('mouseout', this._onMouseOut) @@ -303,13 +302,13 @@ const PathMixin = { this._container = null FeatureMixin.onAdd.call(this, map) this.setStyle() - if (this.editing?.enabled()) this.editing.addHooks() + if (this.editor?.enabled()) this.editor.addHooks() this.resetTooltip() this._path.dataset.feature = this.feature.id }, onRemove: function (map) { - if (this.editing?.enabled()) this.editing.removeHooks() + if (this.editor?.enabled()) this.editor.removeHooks() FeatureMixin.onRemove.call(this, map) }, @@ -362,6 +361,13 @@ const PathMixin = { isOnScreen: function (bounds) { return bounds.overlaps(this.getBounds()) }, + + _setLatLngs: function (latlngs) { + this.parentClass.prototype._setLatLngs.call(this, latlngs) + if (this.editor?.enabled()) { + this.editor.reset() + } + }, } export const LeafletPolyline = Polyline.extend({ diff --git a/umap/static/umap/js/modules/sync/engine.js b/umap/static/umap/js/modules/sync/engine.js index 63af120e..3e524b67 100644 --- a/umap/static/umap/js/modules/sync/engine.js +++ b/umap/static/umap/js/modules/sync/engine.js @@ -1,6 +1,7 @@ import * as SaveManager from '../saving.js' import * as Utils from '../utils.js' import { HybridLogicalClock } from './hlc.js' +import { UndoManager } from './undo.js' import { DataLayerUpdater, FeatureUpdater, MapUpdater } from './updaters.js' import { WebSocketTransport } from './websocket.js' @@ -64,6 +65,7 @@ export class SyncEngine { this.websocketConnected = false this.closeRequested = false this.peerId = Utils.generateId() + this._undoManager = new UndoManager(this.updaters, this) } get isOpen() { @@ -122,16 +124,38 @@ export class SyncEngine { await this.authenticate() }, this._reconnectDelay) } - upsert(subject, metadata, value) { + upsert(subject, metadata, value, oldValue) { + this._undoManager.add({ + verb: 'upsert', + subject, + metadata, + oldValue: oldValue, + newValue: value, + }) this._send({ verb: 'upsert', subject, metadata, value }) } - update(subject, metadata, key, value) { + update(subject, metadata, key, value, oldValue) { + this._undoManager.add({ + verb: 'update', + subject, + metadata, + key, + oldValue: oldValue, + newValue: value, + }) this._send({ verb: 'update', subject, metadata, key, value }) } - delete(subject, metadata, key) { - this._send({ verb: 'delete', subject, metadata, key }) + delete(subject, metadata, oldValue) { + console.log('oldValue', oldValue) + this._undoManager.add({ + verb: 'delete', + subject, + metadata, + oldValue: oldValue, + }) + this._send({ verb: 'delete', subject, metadata }) } saved() { diff --git a/umap/static/umap/js/modules/sync/undo.js b/umap/static/umap/js/modules/sync/undo.js new file mode 100644 index 00000000..52c61e06 --- /dev/null +++ b/umap/static/umap/js/modules/sync/undo.js @@ -0,0 +1,71 @@ +import * as Utils from '../utils.js' +import { DataLayerUpdater, FeatureUpdater, MapUpdater } from './updaters.js' + +export class UndoManager { + constructor(updaters, syncEngine) { + this._syncEngine = syncEngine + this.updaters = updaters + this._undoStack = [] + this._redoStack = [] + } + + toggleState() { + document.querySelector('.edit-undo').disabled = !this._undoStack.length + document.querySelector('.edit-redo').disabled = !this._redoStack.length + } + + add(operation) { + console.debug('New entry in undo stack', operation) + this._redoStack = [] + this._undoStack.push(operation) + this.toggleState() + } + + undo(redo = false) { + const fromStack = redo ? this._redoStack : this._undoStack + const toStack = redo ? this._undoStack : this._redoStack + const operation = fromStack.pop() + if (!operation) return + const syncOperation = Utils.CopyJSON(operation) + console.log('old/new', syncOperation.oldValue, syncOperation.newValue) + delete syncOperation.oldValue + delete syncOperation.newValue + syncOperation.value = redo ? operation.newValue : operation.oldValue + this.applyOperation(syncOperation) + toStack.push(operation) + this.toggleState() + } + + redo() { + this.undo(true) + } + + applyOperation(syncOperation) { + const updater = this._getUpdater(syncOperation.subject, syncOperation.metadata) + switch (syncOperation.verb) { + case 'update': + updater.update(syncOperation) + this._syncEngine._send(syncOperation) + break + case 'delete': + case 'upsert': + console.log('undo upsert/delete', syncOperation.value) + if (syncOperation.value === null || syncOperation.value === undefined) { + console.log('case delete') + updater.delete(syncOperation) + } else { + console.log('case upsert') + updater.upsert(syncOperation) + } + this._syncEngine._send(syncOperation) + break + } + } + + _getUpdater(subject, metadata) { + if (Object.keys(this.updaters).includes(subject)) { + return this.updaters[subject] + } + throw new Error(`Unknown updater ${subject}, ${metadata}`) + } +} diff --git a/umap/static/umap/js/modules/sync/updaters.js b/umap/static/umap/js/modules/sync/updaters.js index e6055091..0ea66d2e 100644 --- a/umap/static/umap/js/modules/sync/updaters.js +++ b/umap/static/umap/js/modules/sync/updaters.js @@ -43,6 +43,7 @@ class BaseUpdater { export class MapUpdater extends BaseUpdater { update({ key, value }) { + console.log('updating', key, value) if (fieldInSchema(key)) { this.updateObjectValue(this._umap, key, value) } @@ -56,9 +57,17 @@ export class DataLayerUpdater extends BaseUpdater { upsert({ value }) { // Upsert only happens when a new datalayer is created. try { - this.getDataLayerFromID(value.id) + console.log( + 'found datalayer with id', + value.id, + this.getDataLayerFromID(value.id) + ) } catch { - this._umap.createDataLayer(value, false) + console.log('we are the fucking catch', value) + const datalayer = this._umap.createDataLayer(value._umap_options || value, false) + if (value.features) { + datalayer.addData(value) + } } } @@ -92,11 +101,14 @@ export class FeatureUpdater extends BaseUpdater { // Create or update an object at a specific position upsert({ metadata, value }) { + console.log('updater.upsert for', metadata, value) const { id, layerId } = metadata const datalayer = this.getDataLayerFromID(layerId) const feature = this.getFeatureFromMetadata(metadata) + console.log('feature', feature) if (feature) { + console.log('changing feature geometry') feature.geometry = value.geometry } else { datalayer.makeFeature(value, false) diff --git a/umap/static/umap/js/modules/ui/bar.js b/umap/static/umap/js/modules/ui/bar.js index f38616fa..484197fc 100644 --- a/umap/static/umap/js/modules/ui/bar.js +++ b/umap/static/umap/js/modules/ui/bar.js @@ -22,9 +22,13 @@ const TOP_BAR_TEMPLATE = ` - +