Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: undo failed in a nested list in a special case #503

Merged
merged 1 commit into from
Sep 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions lib/src/core/document/node.dart
Original file line number Diff line number Diff line change
@@ -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';

Expand Down Expand Up @@ -144,6 +145,7 @@

Log.editor.debug('insert Node $entry at path ${path + [index]}}');

entry._resetRelationshipIfNeeded();
entry.parent = this;

_cacheChildren = null;
Expand Down Expand Up @@ -189,9 +191,9 @@
}

@override
void unlink() {
bool unlink() {
if (parent == null) {
return;
return false;
}
Log.editor.debug('delete Node $this from path $path');
super.unlink();
Expand All @@ -200,6 +202,17 @@

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
Expand Down Expand Up @@ -266,6 +279,30 @@
final index = parent.children.indexOf(this);
return parent._computePath([index, ...previous]);
}

/// check the integrity of the document (for DEBUG only)
void checkDocumentIntegrity() {

Check warning on line 284 in lib/src/core/document/node.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/core/document/node.dart#L284

Added line #L284 was not covered by tests
// skip the root node
if (path.isNotEmpty) {

Check warning on line 286 in lib/src/core/document/node.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/core/document/node.dart#L286

Added line #L286 was not covered by tests
// 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()}''';

Check warning on line 290 in lib/src/core/document/node.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/core/document/node.dart#L290

Added line #L290 was not covered by tests
assert(
parent != null,

Check warning on line 292 in lib/src/core/document/node.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/core/document/node.dart#L292

Added line #L292 was not covered by tests
errorMessage,
);
// also, its parent should contain this node
assert(
parent!.children.where((element) => element.id == id).length == 1,

Check warning on line 297 in lib/src/core/document/node.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/core/document/node.dart#L297

Added line #L297 was not covered by tests
errorMessage,
);
}

for (final child in children) {
child.checkDocumentIntegrity();

Check warning on line 303 in lib/src/core/document/node.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/core/document/node.dart#L302-L303

Added lines #L302 - L303 were not covered by tests
}
}
}

@Deprecated('Use Paragraph instead')
Expand Down
6 changes: 6 additions & 0 deletions lib/src/editor_state.dart
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@
}

Timer? _debouncedSealHistoryItemTimer;
final bool _enableCheckIntegrity = false;

/// Apply the transaction to the state.
///
Expand All @@ -231,6 +232,11 @@
return;
}

// it's a time consuming task, only enable it if necessary.
if (_enableCheckIntegrity) {
document.root.checkDocumentIntegrity();

Check warning on line 237 in lib/src/editor_state.dart

View check run for this annotation

Codecov / codecov/patch

lib/src/editor_state.dart#L237

Added line #L237 was not covered by tests
}

final completer = Completer<void>();

// broadcast to other users here, before applying the transaction
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
});
});
}

Expand Down
Loading