From 3fe68da91bc2ae596be1bbcbd24743011ee6080d Mon Sep 17 00:00:00 2001 From: "Lucas.Xu" Date: Mon, 25 Sep 2023 22:31:01 +0800 Subject: [PATCH] fix: undo failed in a nested list in a special case --- lib/src/core/document/node.dart | 41 +++++++++- lib/src/editor_state.dart | 6 ++ .../undo_redo_command_test.dart | 78 +++++++++++++++++++ 3 files changed, 123 insertions(+), 2 deletions(-) diff --git a/lib/src/core/document/node.dart b/lib/src/core/document/node.dart index 7b1491031..fa4359e96 100644 --- a/lib/src/core/document/node.dart +++ b/lib/src/core/document/node.dart @@ -1,6 +1,7 @@ import 'dart:collection'; import 'package:appflowy_editor/appflowy_editor.dart'; +import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:nanoid/non_secure.dart'; @@ -144,6 +145,7 @@ final class Node extends ChangeNotifier with LinkedListEntry { Log.editor.debug('insert Node $entry at path ${path + [index]}}'); + entry._resetRelationshipIfNeeded(); entry.parent = this; _cacheChildren = null; @@ -189,9 +191,9 @@ final class Node extends ChangeNotifier with LinkedListEntry { } @override - void unlink() { + bool unlink() { if (parent == null) { - return; + return false; } Log.editor.debug('delete Node $this from path $path'); super.unlink(); @@ -200,6 +202,17 @@ final class Node extends ChangeNotifier with LinkedListEntry { parent?.notifyListeners(); parent = null; + return true; + } + + // reset the relationship of the node before inserting it to another node + // to ensure it is not in the tree + // otherwise, it will throw a state error + // 'Bad state: LinkedNode is already in a LinkedList' + void _resetRelationshipIfNeeded() { + if (parent != null || list != null) { + unlink(); + } } @override @@ -266,6 +279,30 @@ final class Node extends ChangeNotifier with LinkedListEntry { final index = parent.children.indexOf(this); return parent._computePath([index, ...previous]); } + + /// check the integrity of the document (for DEBUG only) + void checkDocumentIntegrity() { + // skip the root node + if (path.isNotEmpty) { + // if node is rendered in the tree, its parent should not be null + final errorMessage = + '''Please submit an issue to https://github.com/AppFlowy-IO/appflowy-editor/issues if you see this error! + node = ${toJson()}'''; + assert( + parent != null, + errorMessage, + ); + // also, its parent should contain this node + assert( + parent!.children.where((element) => element.id == id).length == 1, + errorMessage, + ); + } + + for (final child in children) { + child.checkDocumentIntegrity(); + } + } } @Deprecated('Use Paragraph instead') diff --git a/lib/src/editor_state.dart b/lib/src/editor_state.dart index fc541e713..462a9b304 100644 --- a/lib/src/editor_state.dart +++ b/lib/src/editor_state.dart @@ -211,6 +211,7 @@ class EditorState { } Timer? _debouncedSealHistoryItemTimer; + final bool _enableCheckIntegrity = false; /// Apply the transaction to the state. /// @@ -231,6 +232,11 @@ class EditorState { return; } + // it's a time consuming task, only enable it if necessary. + if (_enableCheckIntegrity) { + document.root.checkDocumentIntegrity(); + } + final completer = Completer(); // broadcast to other users here, before applying the transaction diff --git a/test/new/service/shortcuts/command_shortcut_events/undo_redo_command_test.dart b/test/new/service/shortcuts/command_shortcut_events/undo_redo_command_test.dart index db8a483a4..171d86b8c 100644 --- a/test/new/service/shortcuts/command_shortcut_events/undo_redo_command_test.dart +++ b/test/new/service/shortcuts/command_shortcut_events/undo_redo_command_test.dart @@ -114,6 +114,84 @@ void main() async { await editor.dispose(); }); + + // Before + // Welcome to AppFlowy 0| <- cursor here + // Welcome to AppFlowy 0 - 1 + // + // press enter key at the cursor position + // and then, press tab key to indent the empty line + // After + // Welcome to AppFlowy 0 + // | <- cursor here + // Welcome to AppFlowy 0 - 1 + // + // execute undo command twice + // + // then it should be like this + // Welcome to AppFlowy 0| <- cursor here + // Welcome to AppFlowy 0 - 1 + // + // after that, execute redo command twice + // + // Welcome to AppFlowy 0 + // | <- cursor here + // Welcome to AppFlowy 0 - 1 + testWidgets('Undo the nested list', (tester) async { + const text0 = 'Welcome to AppFlowy 0'; + const text01 = 'Welcome to AppFlowy 0 - 1'; + + // Welcome to AppFlowy 0 + // |Welcome to AppFlowy 0 - 1 + final editor = tester.editor + ..addParagraph(initialText: text0) + ..addParagraph(initialText: text01); + + await editor.startTesting(); + await editor.updateSelection( + Selection.collapsed(Position(path: [1], offset: 0)), + ); + await editor.pressKey(key: LogicalKeyboardKey.tab); + + // Welcome to AppFlowy 0| + // Welcome to AppFlowy 0 - 1 + await editor.updateSelection( + Selection.collapsed(Position(path: [0], offset: text0.length)), + ); + await editor.pressKey(character: '\n'); + await editor.pressKey(key: LogicalKeyboardKey.tab); + await tester.pumpAndSettle(); + + expect(editor.nodeAtPath([0])!.delta!.toPlainText(), text0); + expect(editor.nodeAtPath([0, 0])!.delta!.toPlainText(), isEmpty); + expect(editor.nodeAtPath([0, 0, 0])!.delta!.toPlainText(), text01); + + // first undo + await _pressUndoCommand(editor); + expect(editor.nodeAtPath([0])!.delta!.toPlainText(), text0); + expect(editor.nodeAtPath([1])!.delta!.toPlainText(), isEmpty); + expect(editor.nodeAtPath([1, 0])!.delta!.toPlainText(), text01); + + // second undo + await _pressUndoCommand(editor); + expect(editor.nodeAtPath([0])!.delta!.toPlainText(), text0); + expect(editor.nodeAtPath([0, 0])!.delta!.toPlainText(), text01); + + // first redo + await _pressRedoCommand(editor); + expect(editor.nodeAtPath([0])!.delta!.toPlainText(), text0); + expect(editor.nodeAtPath([0])!.children, isEmpty); + expect(editor.nodeAtPath([1])!.delta!.toPlainText(), isEmpty); + expect(editor.nodeAtPath([1, 0])!.delta!.toPlainText(), text01); + + // second redo + await _pressRedoCommand(editor); + expect(editor.nodeAtPath([0])!.delta!.toPlainText(), text0); + expect(editor.nodeAtPath([1])!.delta!.toPlainText(), isEmpty); + expect(editor.nodeAtPath([1, 0])!.delta!.toPlainText(), text01); + + await editor.dispose(); + }); }); }