diff --git a/README.md b/README.md index 5a2dea9c..53cd7da2 100644 --- a/README.md +++ b/README.md @@ -1203,8 +1203,16 @@ ytext.toString() // => 'abc'
constructor(scope:Y.AbstractType|Array<Y.AbstractType> - [, {captureTimeout:number,trackedOrigins:Set<any>,deleteFilter:function(item):boolean}]) + [, {captureTimeout:number,trackedOrigins:Set<any>,deleteFilter:function(item):boolean,ignoreRemoteAttributeChanges:boolean}])
Accepts either single type as scope or an array of types.
+ ignoreRemoteAttributeChanges:boolean +
+By default, the UndoManager will never overwrite remote changes. In some cases +this is not the expected behavior. When set to true, undo/redo may +overwrite remote attribute changes (e.g. on Y.Map keys or +Y.XmlElement / Y.XmlText attributes). Defaults to +false. (previously named ignoreRemoteMapChanges) +
undo()
redo() diff --git a/src/utils/UndoManager.js b/src/utils/UndoManager.js index 5b94f87e..27af08ca 100644 --- a/src/utils/UndoManager.js +++ b/src/utils/UndoManager.js @@ -92,7 +92,7 @@ const popStackItem = (undoManager, stack, eventType) => { } }) itemsToRedo.forEach(struct => { - performedChange = redoItem(transaction, struct, itemsToRedo, stackItem.inserts, undoManager.ignoreRemoteMapChanges, undoManager) !== null || performedChange + performedChange = redoItem(transaction, struct, itemsToRedo, stackItem.inserts, undoManager.ignoreRemoteAttributeChanges, undoManager) !== null || performedChange }) // We want to delete in reverse order so that children are deleted before // parents, so we have more information available when items are filtered. @@ -131,7 +131,7 @@ const popStackItem = (undoManager, stack, eventType) => { * filter returns false, the type/item won't be deleted even it is in the * undo/redo scope. * @property {Set} [UndoManagerOptions.trackedOrigins=new Set([null])] - * @property {boolean} [ignoreRemoteMapChanges] Experimental. By default, the UndoManager will never overwrite remote changes. Enable this property to enable overwriting remote changes on key-value changes (Y.Map, properties on Y.Xml, etc..). + * @property {boolean} [ignoreRemoteAttributeChanges] By default, the UndoManager will never overwrite remote changes. In some cases this might be the expected behavior. This property enables overwriting remote changes on attribute changes. (previously named `ignoreRemoteMapChanges`) * @property {Doc} [doc] The document that this UndoManager operates on. Only needed if typeScope is empty. */ @@ -162,7 +162,7 @@ export class UndoManager extends ObservableV2 { captureTransaction = _tr => true, deleteFilter = () => true, trackedOrigins = new Set([null]), - ignoreRemoteMapChanges = false, + ignoreRemoteAttributeChanges = false, doc = /** @type {Doc} */ (array.isArray(typeScope) ? typeScope[0].doc : typeScope instanceof Doc ? typeScope : typeScope.doc) } = {}) { super() @@ -198,7 +198,7 @@ export class UndoManager extends ObservableV2 { */ this.currStackItem = null this.lastChange = 0 - this.ignoreRemoteMapChanges = ignoreRemoteMapChanges + this.ignoreRemoteAttributeChanges = ignoreRemoteAttributeChanges this.captureTimeout = captureTimeout /** * @param {Transaction} transaction @@ -415,14 +415,14 @@ const isDeletedByUndoStack = (stack, id) => array.some(stack, /** @param {StackI * @param {Item} item * @param {Set} redoitems * @param {IdSet} itemsToDelete - * @param {boolean} ignoreRemoteMapChanges + * @param {boolean} ignoreRemoteAttributeChanges * @param {import('../utils/UndoManager.js').UndoManager} um * * @return {Item|null} * * @private */ -export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemoteMapChanges, um) => { +export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemoteAttributeChanges, um) => { const doc = transaction.doc const store = doc.store const ownClientID = doc.clientID @@ -442,7 +442,7 @@ export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemo // make sure that parent is redone if (parentItem !== null && parentItem.deleted === true) { // try to undo parent if it will be undone anyway - if (parentItem.redone === null && (!redoitems.has(parentItem) || redoItem(transaction, parentItem, redoitems, itemsToDelete, ignoreRemoteMapChanges, um) === null)) { + if (parentItem.redone === null && (!redoitems.has(parentItem) || redoItem(transaction, parentItem, redoitems, itemsToDelete, ignoreRemoteAttributeChanges, um) === null)) { return null } while (parentItem.redone !== null) { @@ -491,7 +491,7 @@ export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemo } } else { right = null - if (item.right && !ignoreRemoteMapChanges) { + if (item.right && !ignoreRemoteAttributeChanges) { left = item // Iterate right while right is in itemsToDelete // If it is intended to delete right while item is redone, we can expect that item should replace right. @@ -508,6 +508,9 @@ export const redoItem = (transaction, item, redoitems, itemsToDelete, ignoreRemo } else { left = parentType._map.get(item.parentSub) || null } + if (left !== null && /** @type {YType} */ (left.parent)._item !== parentItem) { + left = parentType._map.get(item.parentSub) || null + } } const nextClock = store.getClock(ownClientID) const nextId = createID(ownClientID, nextClock) diff --git a/tests/undo-redo.tests.js b/tests/undo-redo.tests.js index 161b3af3..0c34107e 100644 --- a/tests/undo-redo.tests.js +++ b/tests/undo-redo.tests.js @@ -234,6 +234,38 @@ export const testUndoMap = tc => { t.assert(map0.getAttr('b') === 'initial') } +/** + * Regression: undoing a deletion of an embedded type together with an attribute + * change on it must restore the attribute consistently for remote peers. The + * redone attribute item used to keep an origin from the original (deleted) + * parent, so the value was correct locally but dropped after sync. + * + * @param {t.TestCase} tc + */ +export const testUndoEmbeddedTypeAttribute = tc => { + const { testConnector, text0, text1 } = init(tc, { users: 2 }) + const button = new Y.Type() + button.setAttr('type', 'button') + button.setAttr('test', true) + button.insert(0, 'Click me') + text0.insert(0, [button]) + testConnector.syncAll() + + const undoManager = new Y.UndoManager(text0) + undoManager.stopCapturing() + // change an attribute, then delete the embedded type + button.setAttr('type', 'paragraph') + text0.delete(0, 1) + // undo both operations at once + undoManager.undo() + + const expected = { type: 'button', test: true } + t.compare(text0.get(0).getAttrs(), expected) + testConnector.syncAll() + // remote peer must converge to the same attributes + t.compare(text1.get(0).getAttrs(), expected) +} + /** * @param {t.TestCase} tc */ @@ -685,14 +717,14 @@ export const testUndoDeleteTextFormat = _tc => { * @see https://github.com/yjs/yjs/issues/392 * @param {t.TestCase} _tc */ -export const testBehaviorOfIgnoreremotemapchangesProperty = _tc => { +export const testBehaviorOfIgnoreRemoteAttributeChangesProperty = _tc => { const doc = new Y.Doc() const doc2 = new Y.Doc() doc.on('update', update => Y.applyUpdate(doc2, update, doc)) doc2.on('update', update => Y.applyUpdate(doc, update, doc2)) const map1 = doc.get() const map2 = doc2.get() - const um1 = new Y.UndoManager(map1, { ignoreRemoteMapChanges: true }) + const um1 = new Y.UndoManager(map1, { ignoreRemoteAttributeChanges: true }) map1.setAttr('x', 1) map2.setAttr('x', 2) map1.setAttr('x', 3)