From cfdd8919913f05ef6fd22c1d4f9ca1932382604d Mon Sep 17 00:00:00 2001 From: Vincent Chan Date: Tue, 13 Sep 2022 17:33:26 +0800 Subject: [PATCH] fix: redo/undo error --- .../src/operation/transaction_builder.dart | 9 ++- .../appflowy_editor/lib/src/undo_manager.dart | 24 +++--- .../test/legacy/undo_manager_test.dart | 76 +++++++++++++++++++ 3 files changed, 95 insertions(+), 14 deletions(-) create mode 100644 frontend/app_flowy/packages/appflowy_editor/test/legacy/undo_manager_test.dart diff --git a/frontend/app_flowy/packages/appflowy_editor/lib/src/operation/transaction_builder.dart b/frontend/app_flowy/packages/appflowy_editor/lib/src/operation/transaction_builder.dart index 1390b23918..41be1c8c48 100644 --- a/frontend/app_flowy/packages/appflowy_editor/lib/src/operation/transaction_builder.dart +++ b/frontend/app_flowy/packages/appflowy_editor/lib/src/operation/transaction_builder.dart @@ -10,6 +10,7 @@ import 'package:appflowy_editor/src/document/text_delta.dart'; import 'package:appflowy_editor/src/editor_state.dart'; import 'package:appflowy_editor/src/operation/operation.dart'; import 'package:appflowy_editor/src/operation/transaction.dart'; +import 'package:logging/logging.dart'; /// A [TransactionBuilder] is used to build the transaction from the state. /// It will save a snapshot of the cursor selection state automatically. @@ -193,7 +194,7 @@ class TransactionBuilder { /// /// Also, this method will transform the path of the operations /// to avoid conflicts. - add(Operation op) { + add(Operation op, {bool transform = true}) { final Operation? last = operations.isEmpty ? null : operations.last; if (last != null) { if (op is TextEditOperation && @@ -208,8 +209,10 @@ class TransactionBuilder { return; } } - for (var i = 0; i < operations.length; i++) { - op = transformOperation(operations[i], op); + if (transform) { + for (var i = 0; i < operations.length; i++) { + op = transformOperation(operations[i], op); + } } if (op is TextEditOperation && op.delta.isEmpty) { return; diff --git a/frontend/app_flowy/packages/appflowy_editor/lib/src/undo_manager.dart b/frontend/app_flowy/packages/appflowy_editor/lib/src/undo_manager.dart index cfa3f75688..737076e930 100644 --- a/frontend/app_flowy/packages/appflowy_editor/lib/src/undo_manager.dart +++ b/frontend/app_flowy/packages/appflowy_editor/lib/src/undo_manager.dart @@ -43,7 +43,7 @@ class HistoryItem extends LinkedListEntry { for (var i = operations.length - 1; i >= 0; i--) { final operation = operations[i]; final inverted = operation.invert(); - builder.add(inverted); + builder.add(inverted, transform: false); } builder.afterSelection = beforeSelection; builder.beforeSelection = afterSelection; @@ -123,11 +123,12 @@ class UndoManager { } final transaction = historyItem.toTransaction(s); s.apply( - transaction, - const ApplyOptions( - recordUndo: false, - recordRedo: true, - )); + transaction, + const ApplyOptions( + recordUndo: false, + recordRedo: true, + ), + ); } redo() { @@ -142,10 +143,11 @@ class UndoManager { } final transaction = historyItem.toTransaction(s); s.apply( - transaction, - const ApplyOptions( - recordUndo: true, - recordRedo: false, - )); + transaction, + const ApplyOptions( + recordUndo: true, + recordRedo: false, + ), + ); } } diff --git a/frontend/app_flowy/packages/appflowy_editor/test/legacy/undo_manager_test.dart b/frontend/app_flowy/packages/appflowy_editor/test/legacy/undo_manager_test.dart new file mode 100644 index 0000000000..f77e404273 --- /dev/null +++ b/frontend/app_flowy/packages/appflowy_editor/test/legacy/undo_manager_test.dart @@ -0,0 +1,76 @@ +import 'dart:collection'; +import 'package:appflowy_editor/appflowy_editor.dart'; +import 'package:appflowy_editor/src/undo_manager.dart'; +import 'package:flutter_test/flutter_test.dart'; + +void main() async { + setUpAll(() { + TestWidgetsFlutterBinding.ensureInitialized(); + }); + + Node _createEmptyEditorRoot() { + return Node( + type: 'editor', + children: LinkedList(), + attributes: {}, + ); + } + + test("HistoryItem #1", () { + final document = StateTree(root: _createEmptyEditorRoot()); + final editorState = EditorState(document: document); + + final historyItem = HistoryItem(); + historyItem.add(DeleteOperation( + [0], [TextNode(type: 'text', delta: Delta()..insert('0'))])); + historyItem.add(DeleteOperation( + [0], [TextNode(type: 'text', delta: Delta()..insert('1'))])); + historyItem.add(DeleteOperation( + [0], [TextNode(type: 'text', delta: Delta()..insert('2'))])); + + final transaction = historyItem.toTransaction(editorState); + assert(isInsertAndPathEqual(transaction.operations[0], [0], '2')); + assert(isInsertAndPathEqual(transaction.operations[1], [0], '1')); + assert(isInsertAndPathEqual(transaction.operations[2], [0], '0')); + }); + + test("HistoryItem #2", () { + final document = StateTree(root: _createEmptyEditorRoot()); + final editorState = EditorState(document: document); + + final historyItem = HistoryItem(); + historyItem.add(DeleteOperation( + [0], [TextNode(type: 'text', delta: Delta()..insert('0'))])); + historyItem + .add(UpdateOperation([0], {"subType": "number"}, {"subType": null})); + historyItem.add(DeleteOperation([0], [TextNode.empty(), TextNode.empty()])); + historyItem.add(DeleteOperation([0], [TextNode.empty()])); + + final transaction = historyItem.toTransaction(editorState); + assert(isInsertAndPathEqual(transaction.operations[0], [0])); + assert(isInsertAndPathEqual(transaction.operations[1], [0])); + assert(transaction.operations[2] is UpdateOperation); + assert(isInsertAndPathEqual(transaction.operations[3], [0], '0')); + }); +} + +bool isInsertAndPathEqual(Operation operation, Path path, [String? content]) { + if (operation is! InsertOperation) { + return false; + } + + if (!pathEquals(operation.path, path)) { + return false; + } + + final firstNode = operation.nodes[0]; + if (firstNode is! TextNode) { + return false; + } + + if (content == null) { + return true; + } + + return firstNode.delta.toRawString() == content; +}